Do not allow confirm dialogs to activate tabs.

BUG=849816

Change-Id: Ie726f28d1f6c760f8330b03a93387ad7479c335c
Reviewed-on: https://chromium-review.googlesource.com/1089659Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565397}

chrome/browser/ui/android/javascript_dialog_android.cc

@@ -21,11 +21,6 @@ using base::android::JavaParamRef;
 using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 
-// JavaScriptDialog:
-JavaScriptDialog::JavaScriptDialog(content::WebContents* parent_web_contents) {
-  parent_web_contents->GetDelegate()->ActivateContents(parent_web_contents);
-}
-
 JavaScriptDialog::~JavaScriptDialog() = default;
 
 base::WeakPtr<JavaScriptDialog> JavaScriptDialog::Create(
@@ -106,9 +101,7 @@ JavaScriptDialogAndroid::JavaScriptDialogAndroid(
     const base::string16& message_text,
     const base::string16& default_prompt_text,
     content::JavaScriptDialogManager::DialogClosedCallback dialog_callback)
-    : JavaScriptDialog(parent_web_contents),
-      dialog_callback_(std::move(dialog_callback)),
-      weak_factory_(this) {
+    : dialog_callback_(std::move(dialog_callback)), weak_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   JNIEnv* env = AttachCurrentThread();

chrome/browser/ui/browser_browsertest.cc

@@ -447,27 +447,80 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, Title) {
   EXPECT_EQ(test_title, tab_title);
 }
 
-// TODO(avi): confirm() is the only dialog type left that activates. Remove this
-// test when activation is removed from it.
-IN_PROC_BROWSER_TEST_F(BrowserTest, JavascriptConfirmActivatesTab) {
+namespace {
+
+class DialogPlusConsoleObserverDelegate
+    : public content::ConsoleObserverDelegate {
+ public:
+  DialogPlusConsoleObserverDelegate(
+      content::WebContentsDelegate* original_delegate,
+      WebContents* web_contents,
+      const std::string& filter)
+      : content::ConsoleObserverDelegate(web_contents, filter),
+        web_contents_(web_contents),
+        original_delegate_(original_delegate) {}
+
+  // WebContentsDelegate method:
+  content::JavaScriptDialogManager* GetJavaScriptDialogManager(
+      WebContents* source) override {
+    return original_delegate_->GetJavaScriptDialogManager(web_contents_);
+  }
+
+ private:
+  content::WebContents* web_contents_;
+  content::WebContentsDelegate* original_delegate_;
+};
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(BrowserTest, NoJavaScriptDialogsActivateTab) {
+  // Set up two tabs, with the tab at index 0 active.
   GURL url(ui_test_utils::GetTestUrl(base::FilePath(
       base::FilePath::kCurrentDirectory), base::FilePath(kTitle1File)));
   ui_test_utils::NavigateToURL(browser(), url);
   AddTabAtIndex(0, url, ui::PAGE_TRANSITION_TYPED);
   EXPECT_EQ(2, browser()->tab_strip_model()->count());
   EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+
   WebContents* second_tab = browser()->tab_strip_model()->GetWebContentsAt(1);
   ASSERT_TRUE(second_tab);
+  content::WebContentsDelegate* original_delegate = second_tab->GetDelegate();
+
+  // Show a confirm() dialog from the tab at index 1. The active index shouldn't
+  // budge.
+  DialogPlusConsoleObserverDelegate confirm_observer(
+      original_delegate, second_tab, "*confirm*suppressed*");
+  second_tab->SetDelegate(&confirm_observer);
+  second_tab->GetMainFrame()->ExecuteJavaScriptForTests(
+      ASCIIToUTF16("confirm('Activate!');"));
+  confirm_observer.Wait();
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+
+  // Show a prompt() dialog from the tab at index 1. The active index shouldn't
+  // budge.
+  DialogPlusConsoleObserverDelegate prompt_observer(
+      original_delegate, second_tab, "*prompt*suppressed*");
+  second_tab->SetDelegate(&prompt_observer);
+  second_tab->GetMainFrame()->ExecuteJavaScriptForTests(
+      ASCIIToUTF16("prompt('Activate!');"));
+  prompt_observer.Wait();
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+
+  second_tab->SetDelegate(original_delegate);
+
+  // Show an alert() dialog from the tab at index 1. The active index shouldn't
+  // budge.
   JavaScriptDialogTabHelper* js_helper =
       JavaScriptDialogTabHelper::FromWebContents(second_tab);
-  base::RunLoop dialog_wait;
-  js_helper->SetDialogShownCallbackForTesting(dialog_wait.QuitClosure());
+  base::RunLoop alert_wait;
+  js_helper->SetDialogShownCallbackForTesting(alert_wait.QuitClosure());
   second_tab->GetMainFrame()->ExecuteJavaScriptForTests(
-      ASCIIToUTF16("confirm('Activate!');"));
-  dialog_wait.Run();
-  js_helper->HandleJavaScriptDialog(second_tab, true, nullptr);
+      ASCIIToUTF16("alert('Activate!');"));
+  alert_wait.Run();
   EXPECT_EQ(2, browser()->tab_strip_model()->count());
-  EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
 }
 
 #if defined(OS_WIN) && !defined(NDEBUG)

chrome/browser/ui/javascript_dialogs/javascript_dialog.cc

@@ -6,15 +6,7 @@
 
 #include <utility>
 
-#include "chrome/browser/ui/blocked_content/popunder_preventer.h"
 #include "chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h"
-#include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_contents_delegate.h"
-
-JavaScriptDialog::JavaScriptDialog(content::WebContents* parent_web_contents) {
-  popunder_preventer_.reset(new PopunderPreventer(parent_web_contents));
-  parent_web_contents->GetDelegate()->ActivateContents(parent_web_contents);
-}
 
 JavaScriptDialog::~JavaScriptDialog() = default;
 

chrome/browser/ui/javascript_dialogs/javascript_dialog.h

@@ -11,10 +11,6 @@
 #include "build/build_config.h"
 #include "content/public/browser/javascript_dialog_manager.h"
 
-#if !defined(OS_ANDROID)
-class PopunderPreventer;
-#endif  // !defined(OS_ANDROID)
-
 class JavaScriptDialog {
  public:
   virtual ~JavaScriptDialog();
@@ -36,14 +32,6 @@ class JavaScriptDialog {
 
   // Returns the current value of the user input for a prompt dialog.
   virtual base::string16 GetUserInput() = 0;
-
- protected:
-  explicit JavaScriptDialog(content::WebContents* parent_web_contents);
-
- private:
-#if !defined(OS_ANDROID)
-  std::unique_ptr<PopunderPreventer> popunder_preventer_;
-#endif  // !defined(OS_ANDROID)
 };
 
 #endif  // CHROME_BROWSER_UI_JAVASCRIPT_DIALOGS_JAVASCRIPT_DIALOG_H_

chrome/browser/ui/javascript_dialogs/javascript_dialog_cocoa.mm

@@ -178,8 +178,7 @@ JavaScriptDialogCocoa::JavaScriptDialogCocoa(
     const base::string16& message_text,
     const base::string16& default_prompt_text,
     content::JavaScriptDialogManager::DialogClosedCallback dialog_callback)
-    : JavaScriptDialog(parent_web_contents),
-      impl_(std::make_unique<JavaScriptDialogCocoaImpl>(
+    : impl_(std::make_unique<JavaScriptDialogCocoaImpl>(
           this,
           parent_web_contents,
           alerting_web_contents,

chrome/browser/ui/javascript_dialogs/javascript_dialog_mac.cc

@@ -4,17 +4,9 @@
 
 #include "chrome/browser/ui/javascript_dialogs/javascript_dialog.h"
 
-#include "chrome/browser/ui/blocked_content/popunder_preventer.h"
 #include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
 #include "chrome/browser/ui/javascript_dialogs/javascript_dialog_cocoa.h"
 #include "chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h"
-#include "content/public/browser/web_contents.h"
-#include "content/public/browser/web_contents_delegate.h"
-
-JavaScriptDialog::JavaScriptDialog(content::WebContents* parent_web_contents) {
-  popunder_preventer_.reset(new PopunderPreventer(parent_web_contents));
-  parent_web_contents->GetDelegate()->ActivateContents(parent_web_contents);
-}
 
 JavaScriptDialog::~JavaScriptDialog() = default;
 

chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -215,6 +216,12 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
   bool make_pending = false;
   if (!IsWebContentsForemost(parent_web_contents) &&
       !content::DevToolsAgentHost::IsDebuggerAttached(parent_web_contents)) {
+    static const char kDialogSuppressedConsoleMessageFormat[] =
+        "A window.%s() dialog generated by this page was suppressed "
+        "because this page is not the active tab of the front window. "
+        "Please make sure your dialogs are triggered by user interactions "
+        "to avoid this situation. https://www.chromestatus.com/feature/%s";
+
     switch (dialog_type) {
       case content::JAVASCRIPT_DIALOG_TYPE_ALERT: {
         // When an alert fires in the background, make the callback so that the
@@ -228,19 +235,19 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
         break;
       }
       case content::JAVASCRIPT_DIALOG_TYPE_CONFIRM: {
-        // TODO(avi): Remove confirm() dialogs from non-foremost tabs, just like
-        // has been done for prompt() dialogs.
-        break;
+        *did_suppress_message = true;
+        alerting_web_contents->GetMainFrame()->AddMessageToConsole(
+            content::CONSOLE_MESSAGE_LEVEL_WARNING,
+            base::StringPrintf(kDialogSuppressedConsoleMessageFormat, "confirm",
+                               "5140698722467840"));
+        return;
       }
       case content::JAVASCRIPT_DIALOG_TYPE_PROMPT: {
         *did_suppress_message = true;
         alerting_web_contents->GetMainFrame()->AddMessageToConsole(
             content::CONSOLE_MESSAGE_LEVEL_WARNING,
-            "A window.prompt() dialog generated by this page was suppressed "
-            "because this page is not the active tab of the front window. "
-            "Please make sure your dialogs are triggered by user interactions "
-            "to avoid this situation. "
-            "https://www.chromestatus.com/feature/5637107137642496");
+            base::StringPrintf(kDialogSuppressedConsoleMessageFormat, "prompt",
+                               "5637107137642496"));
         return;
       }
     }

chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc

@@ -103,8 +103,7 @@ JavaScriptDialogViews::JavaScriptDialogViews(
     const base::string16& message_text,
     const base::string16& default_prompt_text,
     content::JavaScriptDialogManager::DialogClosedCallback dialog_callback)
-    : JavaScriptDialog(parent_web_contents),
-      title_(title),
+    : title_(title),
       dialog_type_(dialog_type),
       message_text_(message_text),
       default_prompt_text_(default_prompt_text),