Refactor creation of web intents for inline disposition up into the picker controller.

TBR=estade@chromium.org,sail@chromium.org,pkasting@chromium.org
R=smckay@chromium.org
BUG=None


Review URL: https://chromiumcodereview.appspot.com/11088042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@161244 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 33 insertions, 61 deletions

diff --git a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
index f29036d..1c39da4 100644
--- a/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
+++ b/chrome/browser/ui/cocoa/web_intent_picker_cocoa.mm
@@ -159,10 +159,10 @@ void WebIntentPickerCocoa::OnExtensionIconChanged(
 
 void WebIntentPickerCocoa::OnInlineDisposition(const string16& title,
                                                const GURL& url) {
-  content::WebContents* web_contents = content::WebContents::Create(
-      tab_contents_->profile(),
-      tab_util::GetSiteInstanceForNewTab(tab_contents_->profile(), url),
-      MSG_ROUTING_NONE, NULL);
+
+  content::WebContents* web_contents =
+      delegate_->CreateWebContentsForInlineDisposition(
+          tab_contents_->profile(), url);
   inline_disposition_tab_contents_.reset(
       TabContents::Factory::CreateTabContents(web_contents));
   Browser* browser = browser::FindBrowserWithWebContents(
@@ -170,10 +170,6 @@ void WebIntentPickerCocoa::OnInlineDisposition(const string16& title,
   inline_disposition_delegate_.reset(
       new WebIntentInlineDispositionDelegate(this, web_contents, browser));
 
-  // Must call this immediately after WebContents creation to avoid race
-  // with load.
-  delegate_->OnInlineDispositionWebContentsCreated(web_contents);
-
   inline_disposition_tab_contents_->web_contents()->GetController().LoadURL(
       url,
       content::Referrer(),
diff --git a/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm b/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
index 6a80cd0..5c7e5c9 100644
--- a/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
+++ b/chrome/browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm
@@ -9,31 +9,11 @@
 #include "chrome/browser/ui/cocoa/web_intent_picker_cocoa.h"
 #import "chrome/browser/ui/cocoa/web_intent_sheet_controller.h"
 #include "chrome/browser/ui/intents/web_intent_picker_delegate.h"
+#include "chrome/browser/ui/intents/web_intent_picker_delegate_mock.h"
 #include "chrome/browser/ui/intents/web_intent_picker_model.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "webkit/glue/web_intent_service_data.h"
 
-class MockIntentPickerDelegate : public WebIntentPickerDelegate {
- public:
-  MockIntentPickerDelegate() {}
-  virtual ~MockIntentPickerDelegate() {}
-
-  MOCK_METHOD2(OnServiceChosen, void(
-      const GURL& url,
-      webkit_glue::WebIntentServiceData::Disposition disposition));
-  MOCK_METHOD2(OnExtensionLinkClicked, void(
-      const std::string& id,
-      WindowOpenDisposition disposition));
-  MOCK_METHOD1(OnInlineDispositionWebContentsCreated,
-      void(content::WebContents* web_contents));
-  MOCK_METHOD1(OnExtensionInstallRequested, void(const std::string& id));
-  MOCK_METHOD1(OnSuggestionsLinkClicked,
-      void(WindowOpenDisposition disposition));
-  MOCK_METHOD0(OnUserCancelledPickerDialog, void());
-  MOCK_METHOD0(OnChooseAnotherService, void());
-  MOCK_METHOD0(OnClosing, void());
-};
-
 class WebIntentSheetControllerBrowserTest : public InProcessBrowserTest {
  public:
   void CreateBubble(TabContents* tab_contents);
@@ -42,7 +22,7 @@ class WebIntentSheetControllerBrowserTest : public InProcessBrowserTest {
   WebIntentPickerSheetController* controller_;  // Weak, owns self.
   NSWindow* window_;  // Weak, owned by controller.
   scoped_ptr<WebIntentPickerCocoa> picker_;
-  MockIntentPickerDelegate delegate_;
+  WebIntentPickerDelegateMock delegate_;
   WebIntentPickerModel model_;  // The model used by the picker
 };
 
diff --git a/chrome/browser/ui/gtk/web_intent_picker_gtk.cc b/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
index 7670b18..aeded0b 100644
--- a/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
+++ b/chrome/browser/ui/gtk/web_intent_picker_gtk.cc
@@ -286,10 +286,9 @@ void WebIntentPickerGtk::OnExtensionIconChanged(
 
 void WebIntentPickerGtk::OnInlineDisposition(const string16& title,
                                              const GURL& url) {
-  content::WebContents* web_contents = content::WebContents::Create(
-      tab_contents_->profile(),
-      tab_util::GetSiteInstanceForNewTab(tab_contents_->profile(), url),
-      MSG_ROUTING_NONE, NULL);
+  content::WebContents* web_contents =
+      delegate_->CreateWebContentsForInlineDisposition(
+          tab_contents_->profile(), url);
   inline_disposition_tab_contents_.reset(
       TabContents::Factory::CreateTabContents(web_contents));
   Browser* browser = browser::FindBrowserWithWebContents(
@@ -297,10 +296,6 @@ void WebIntentPickerGtk::OnInlineDisposition(const string16& title,
   inline_disposition_delegate_.reset(
       new WebIntentInlineDispositionDelegate(this, web_contents, browser));
 
-  // Must call this immediately after WebContents creation to avoid race
-  // with load.
-  delegate_->OnInlineDispositionWebContentsCreated(web_contents);
-
   tab_contents_container_.reset(new TabContentsContainerGtk(NULL));
   tab_contents_container_->SetTab(inline_disposition_tab_contents_.get());
 
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.cc b/chrome/browser/ui/intents/web_intent_picker_controller.cc
index 3c7622f..5866046 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.cc
@@ -415,6 +415,17 @@ void WebIntentPickerController::OnServiceChosen(
   }
 }
 
+content::WebContents*
+WebIntentPickerController::CreateWebContentsForInlineDisposition(
+    Profile* profile, const GURL& url) {
+  content::WebContents* web_contents = content::WebContents::Create(
+      profile,
+      tab_util::GetSiteInstanceForNewTab(profile, url),
+      MSG_ROUTING_NONE, NULL);
+  intents_dispatcher_->DispatchIntent(web_contents);
+  return web_contents;
+}
+
 // Take the MD5 digest of the origins of picker options
 // and record it as the default context string.
 int64 WebIntentPickerController::DigestServices() {
@@ -462,12 +473,6 @@ void WebIntentPickerController::SetDefaultServiceForSelection(const GURL& url) {
   GetWebIntentsRegistry(profile_)->RegisterDefaultIntentService(record);
 }
 
-void WebIntentPickerController::OnInlineDispositionWebContentsCreated(
-    content::WebContents* web_contents) {
-  if (web_contents)
-    intents_dispatcher_->DispatchIntent(web_contents);
-}
-
 void WebIntentPickerController::OnExtensionInstallRequested(
     const std::string& id) {
   picker_model_->SetPendingExtensionInstallId(id);
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.h b/chrome/browser/ui/intents/web_intent_picker_controller.h
index 3ff9f67..7da2ab6 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.h
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.h
@@ -116,8 +116,8 @@ class WebIntentPickerController
   virtual void OnServiceChosen(
       const GURL& url,
       webkit_glue::WebIntentServiceData::Disposition disposition) OVERRIDE;
-  virtual void OnInlineDispositionWebContentsCreated(
-      content::WebContents* web_contents) OVERRIDE;
+  virtual content::WebContents* CreateWebContentsForInlineDisposition(
+      Profile* profile, const GURL& url) OVERRIDE;
   virtual void OnExtensionInstallRequested(const std::string& id) OVERRIDE;
   virtual void OnExtensionLinkClicked(
       const std::string& id,
diff --git a/chrome/browser/ui/intents/web_intent_picker_delegate.h b/chrome/browser/ui/intents/web_intent_picker_delegate.h
index 6dfd71b..9a2f661 100644
--- a/chrome/browser/ui/intents/web_intent_picker_delegate.h
+++ b/chrome/browser/ui/intents/web_intent_picker_delegate.h
@@ -13,6 +13,8 @@ namespace content {
 class WebContents;
 }
 
+class Profile;
+
 // A class used to notify the delegate when the user has chosen a web intent
 // service.
 class WebIntentPickerDelegate {
@@ -25,10 +27,10 @@ class WebIntentPickerDelegate {
       const GURL& url,
       webkit_glue::WebIntentServiceData::Disposition disposition) = 0;
 
-  // Called when the picker has created WebContents to use for inline
-  // disposition.
-  virtual void OnInlineDispositionWebContentsCreated(
-      content::WebContents* web_contents) = 0;
+  // Called to create the WebContents into which the inline disposition will be
+  // placed.
+  virtual content::WebContents* CreateWebContentsForInlineDisposition(
+      Profile* profile, const GURL& url) = 0;
 
   // Called when the user has chosen to install a suggested extension.
   virtual void OnExtensionInstallRequested(const std::string& id) = 0;
diff --git a/chrome/browser/ui/intents/web_intent_picker_delegate_mock.h b/chrome/browser/ui/intents/web_intent_picker_delegate_mock.h
index 26f8b8c..5db4971 100644
--- a/chrome/browser/ui/intents/web_intent_picker_delegate_mock.h
+++ b/chrome/browser/ui/intents/web_intent_picker_delegate_mock.h
@@ -17,8 +17,8 @@ class WebIntentPickerDelegateMock : public WebIntentPickerDelegate {
       OnServiceChosen,
       void(const GURL& url,
            webkit_glue::WebIntentServiceData::Disposition disposition));
-  MOCK_METHOD1(OnInlineDispositionWebContentsCreated,
-               void(content::WebContents* web_contents));
+  MOCK_METHOD2(CreateWebContentsForInlineDisposition,
+               content::WebContents*(Profile* profile, const GURL& url));
   MOCK_METHOD1(OnExtensionInstallRequested, void(const std::string& id));
   MOCK_METHOD2(OnExtensionLinkClicked,
                void(const std::string& id,
diff --git a/chrome/browser/ui/views/web_intent_picker_views.cc b/chrome/browser/ui/views/web_intent_picker_views.cc
index f934229..7c85fdf 100644
--- a/chrome/browser/ui/views/web_intent_picker_views.cc
+++ b/chrome/browser/ui/views/web_intent_picker_views.cc
@@ -1178,10 +1178,8 @@ void WebIntentPickerViews::OnInlineDisposition(
   if (!webview_)
     webview_ = new views::WebView(tab_contents_->profile());
 
-  inline_web_contents_.reset(WebContents::Create(
-      tab_contents_->profile(),
-      tab_util::GetSiteInstanceForNewTab(tab_contents_->profile(), url),
-      MSG_ROUTING_NONE, NULL));
+  inline_web_contents_.reset(delegate_->CreateWebContentsForInlineDisposition(
+      tab_contents_->profile(), url));
   // Does not take ownership, so we keep a scoped_ptr
   // for the WebContents locally.
   webview_->SetWebContents(inline_web_contents_.get());
@@ -1190,12 +1188,8 @@ void WebIntentPickerViews::OnInlineDisposition(
   inline_disposition_delegate_.reset(
       new WebIntentInlineDispositionDelegate(this, inline_web_contents_.get(),
                                              browser));
-  content::WebContents* web_contents = webview_->GetWebContents();
 
-  // Must call this immediately after WebContents creation to avoid race
-  // with load.
-  delegate_->OnInlineDispositionWebContentsCreated(web_contents);
-  web_contents->GetController().LoadURL(
+  inline_web_contents_->GetController().LoadURL(
       url,
       content::Referrer(),
       content::PAGE_TRANSITION_AUTO_TOPLEVEL,