HaTS Next: Re-direct new WebContents to use survey source browser

To prevent the HaTS Next survey from attempting to open windows in the
non-primary OTR profile it runs in, a WebContentsDelegate is introduced
to handle attempts to create new WebContents. These are re-opened in the
browser which initiated the HaTS survey instead.

Bug: 1110888
Change-Id: I96942014fbeb9e78f036d87187902a9dd1dcc66a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324129
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796417}

chrome/browser/ui/views/hats/hats_browsertest.cc

@@ -267,3 +267,26 @@ IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, UnknownURLFragment) {
   });
   run_loop.Run();
 }
+
+IN_PROC_BROWSER_TEST_F(HatsNextWebDialogBrowserTest, NewWebContents) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  auto* dialog = new MockHatsNextWebDialog(
+      browser(), "open_new_web_contents_for_testing",
+      embedded_test_server()->GetURL("/hats/hats_next_mock.html"),
+      base::TimeDelta::FromSeconds(100));
+
+  // The mock hats dialog will push a loaded state after it has attempted to
+  // open another web contents.
+  base::RunLoop run_loop;
+  EXPECT_CALL(*dialog, ShowWidget).WillOnce(testing::Invoke([&run_loop]() {
+    run_loop.Quit();
+  }));
+  run_loop.Run();
+
+  // Check that a tab with http://foo.com (defined in hats_next_mock.html) has
+  // been opened in the regular browser and is active.
+  EXPECT_EQ(
+      GURL("http://foo.com"),
+      browser()->tab_strip_model()->GetActiveWebContents()->GetVisibleURL());
+}

chrome/browser/ui/views/hats/hats_next_web_dialog.cc

@@ -26,6 +26,45 @@
 #include "ui/views/controls/webview/web_dialog_view.h"
 #include "ui/views/layout/fill_layout.h"
 
+// A delegate used to intercept the creation of new WebContents by the HaTS
+// Next dialog.
+class HatsNextWebDialog::WebContentsDelegate
+    : public content::WebContentsDelegate {
+ public:
+  explicit WebContentsDelegate(Browser* browser) : browser_(browser) {}
+
+  bool IsWebContentsCreationOverridden(
+      content::SiteInstance* source_site_instance,
+      content::mojom::WindowContainerType window_container_type,
+      const GURL& opener_url,
+      const std::string& frame_name,
+      const GURL& target_url) override {
+    return true;
+  }
+
+  content::WebContents* CreateCustomWebContents(
+      content::RenderFrameHost* opener,
+      content::SiteInstance* source_site_instance,
+      bool is_new_browsing_instance,
+      const GURL& opener_url,
+      const std::string& frame_name,
+      const GURL& target_url,
+      const std::string& partition_id,
+      content::SessionStorageNamespace* session_storage_namespace) override {
+    // The HaTS Next WebDialog runs with a non-primary OTR profile. This profile
+    // cannot open new browser windows, so they are instead opened in the
+    // regular browser that initiated the HaTS survey.
+    browser_->OpenURL(
+        content::OpenURLParams(target_url, content::Referrer(),
+                               WindowOpenDisposition::NEW_FOREGROUND_TAB,
+                               ui::PAGE_TRANSITION_LINK, false));
+    return nullptr;
+  }
+
+ private:
+  Browser* browser_;
+};
+
 // A thin wrapper that forwards the reference part of the URL associated with
 // navigation events to the enclosing web dialog.
 class HatsNextWebDialog::WebContentsObserver
@@ -130,10 +169,10 @@ HatsNextWebDialog::HatsNextWebDialog(Browser* browser,
                                views::BubbleBorder::TOP_RIGHT),
       otr_profile_(browser->profile()->GetOffTheRecordProfile(
           Profile::OTRProfileID::CreateUnique("HaTSNext:WebDialog"))),
+      browser_(browser),
       trigger_id_(trigger_id),
       hats_survey_url_(hats_survey_url),
-      timeout_(timeout),
-      close_bubble_helper_(this, browser) {
+      timeout_(timeout) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   otr_profile_->AddObserver(this);
   set_close_on_deactivate(false);
@@ -141,13 +180,16 @@ HatsNextWebDialog::HatsNextWebDialog(Browser* browser,
   SetButtons(ui::DIALOG_BUTTON_NONE);
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
-  auto* web_view = AddChildView(std::make_unique<views::WebDialogView>(
+  web_view_ = AddChildView(std::make_unique<views::WebDialogView>(
       otr_profile_, this, std::make_unique<ChromeWebContentsHandler>(),
       /* use_dialog_frame */ true));
   widget_ = views::BubbleDialogDelegateView::CreateBubble(this);
 
   web_contents_observer_ =
-      std::make_unique<WebContentsObserver>(web_view->web_contents(), this);
+      std::make_unique<WebContentsObserver>(web_view_->web_contents(), this);
+
+  web_contents_delegate_ = std::make_unique<WebContentsDelegate>(browser_);
+  web_view_->web_contents()->SetDelegate(web_contents_delegate_.get());
 
   loading_timer_.Start(FROM_HERE, timeout_,
                        base::BindOnce(&HatsNextWebDialog::CloseWidget,
@@ -160,6 +202,9 @@ HatsNextWebDialog::~HatsNextWebDialog() {
     otr_profile_->RemoveObserver(this);
     ProfileDestroyer::DestroyProfileWhenAppropriate(otr_profile_);
   }
+  // Explicitly clear the delegate to ensure it is not invalid between now and
+  // when the web contents is destroyed in the base class.
+  web_view_->web_contents()->SetDelegate(nullptr);
 }
 
 void HatsNextWebDialog::OnSurveyStateUpdateReceived(std::string state) {

chrome/browser/ui/views/hats/hats_next_web_dialog.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_UI_VIEWS_HATS_HATS_NEXT_WEB_DIALOG_H_
 
 #include "chrome/browser/profiles/profile_observer.h"
-#include "chrome/browser/ui/views/close_bubble_on_tab_activation_helper.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/controls/webview/web_dialog_view.h"
@@ -67,6 +66,7 @@ class HatsNextWebDialog : public ui::WebDialogDelegate,
                     const GURL& hats_survey_url_,
                     const base::TimeDelta& timeout);
 
+  class WebContentsDelegate;
   class WebContentsObserver;
 
   // Fired by the observer when the survey page has pushed state to the window
@@ -95,18 +95,20 @@ class HatsNextWebDialog : public ui::WebDialogDelegate,
   // The off-the-record profile used for browsing to the Chrome HaTS webpage.
   Profile* otr_profile_;
 
+  Browser* browser_;
+
   // The HaTS Next survey trigger ID that is provided to the HaTS webpage.
   const std::string& trigger_id_;
 
+  views::WebDialogView* web_view_ = nullptr;
   views::Widget* widget_ = nullptr;
 
+  std::unique_ptr<WebContentsDelegate> web_contents_delegate_;
   std::unique_ptr<WebContentsObserver> web_contents_observer_;
   GURL hats_survey_url_;
 
   base::TimeDelta timeout_;
 
-  CloseBubbleOnTabActivationHelper close_bubble_helper_;
-
   base::WeakPtrFactory<HatsNextWebDialog> weak_factory_{this};
 };
 

chrome/test/data/hats/hats_next_mock.html

@@ -15,6 +15,11 @@
       if (params.get('trigger_id') == "invalid_url_fragment_for_testing") {
         history.pushState('', '', '#foo');
       }
+
+      if (params.get('trigger_id') == "open_new_web_contents_for_testing"){
+        window.open('http://foo.com', '_blank');
+        history.pushState('', '', '#loaded');
+      }
     </script>
   </head>
   <body>