Make JavaScript dialogs auto-dismiss on tab switch.

chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc

Index: chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
diff --git a/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc b/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
index 4abd35a0aff6e0b6d4d828970a122d7070a912f3..1c56b46feb8850330ac0d8f3435578e70bc0a099 100644
--- a/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
+++ b/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
@@ -4,11 +4,18 @@
 
 #include "chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h"
 
+#include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h"
+#include "chrome/browser/ui/tab_modal_confirm_dialog.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "components/app_modal/javascript_dialog_manager.h"
+#include "content/public/browser/web_contents_delegate.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(JavaScriptDialogTabHelper);
 
@@ -21,10 +28,16 @@ bool IsEnabled() {
   return base::FeatureList::IsEnabled(kAutoDismissingDialogsFeature);
 }
 
-content::JavaScriptDialogManager* AppModalDialogManager() {
+app_modal::JavaScriptDialogManager* AppModalDialogManager() {
   return app_modal::JavaScriptDialogManager::GetInstance();
 }
 
+bool IsWebContentsForemost(content::WebContents* web_contents) {
+  Browser* browser = BrowserList::GetInstance()->GetLastActive();
+  DCHECK(browser);
+  return browser->tab_strip_model()->GetActiveWebContents() == web_contents;
+}
+
 }  // namespace
 
 JavaScriptDialogTabHelper::JavaScriptDialogTabHelper(
@@ -35,11 +48,8 @@ JavaScriptDialogTabHelper::JavaScriptDialogTabHelper(
 JavaScriptDialogTabHelper::~JavaScriptDialogTabHelper() {
 }
 
-void JavaScriptDialogTabHelper::WebContentsDestroyed() {
-}
-
 void JavaScriptDialogTabHelper::RunJavaScriptDialog(
-    content::WebContents* web_contents,
+    content::WebContents* alerting_web_contents,
     const GURL& origin_url,
     content::JavaScriptMessageType message_type,
     const base::string16& message_text,
@@ -47,7 +57,7 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
     const DialogClosedCallback& callback,
     bool* did_suppress_message) {
   SiteEngagementService* site_engagement_service = SiteEngagementService::Get(
-      Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+      Profile::FromBrowserContext(alerting_web_contents->GetBrowserContext()));
   double engagement_score = site_engagement_service->GetScore(origin_url);
   switch (message_type) {
     case content::JAVASCRIPT_MESSAGE_TYPE_ALERT:
@@ -79,10 +89,43 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
   }
 
   if (IsEnabled()) {
-    NOTREACHED() << "auto-dismissing dialog code does not yet exist";
+    content::WebContents* parent_web_contents =
+        WebContentsObserver::web_contents();
+
+    if (!IsWebContentsForemost(parent_web_contents) &&
+        message_type == content::JAVASCRIPT_MESSAGE_TYPE_PROMPT) {
+      // Don't allow "prompt" dialogs to steal the user's focus. TODO(avi):
+      // Eventually, for subsequent phases of http://bit.ly/project-oldspice,
+      // turn off focus stealing for other dialog types.
+      *did_suppress_message = true;
+      callback.Run(false, base::string16());
+      return;
+    }
+
+    if (dialog_) {
+      // There's already a dialog up; clear it out.
+      CloseDialog(false, false, base::string16());
+    }
+
+    parent_web_contents->GetDelegate()->ActivateContents(parent_web_contents);

[jochen (gone - plz use gerrit), 2016/10/27 15:16:03]

will this defeat the pop-under blocker? In the old code path, we've created an
AppModalDialogHelper before activating the contents, so if there was an popup
before the web contents got raised, it will be restored after the dialog closes.

[Avi (use Gerrit), 2016/10/28 14:18:07]

I was not aware of the interaction with the popup blocker and this code; thank
you for bringing it to my attention.

So that I understand, let me summarize what I think your concern is.

The attack is to open a popup window and then show a JS alert, using the alert
to raise the alerting window, in the hopes that the alerting window stays
z-ordered above the popup window. The dialog helper captures the active window
before activating the alerting window, and restores it when the alert is
complete.

I think we should talk, because that paragraph is the extent of my knowledge
about the dialog helper, and I'm having trouble working out the interaction
between the auto-dismissing behavior of these new JS alerts and the dialog
helper. In particular, the new JS alerts auto-dismiss when you switch away from
the alerting tab. If you switch away to a different tab in the same window, I
think we're OK switching back to the previously-active window, but if you switch
away to a different window, we can't lose the user's focus intent.

+
+    base::string16 title =
+        AppModalDialogManager()->GetTitle(alerting_web_contents, origin_url);
+    dialog_callback_ = callback;
+    BrowserList::AddObserver(this);
+    // TODO(avi): abstract out so that we have a Mac version too...
+    dialog_ = JavaScriptDialogViews::Create(
+        parent_web_contents, alerting_web_contents, title, message_type,
+        message_text, default_prompt_text, callback);
+
+    // Message suppression is something that we don't give the user a checkbox
+    // for any more. It was useful back in the day when dialogs were app-modal
+    // and clicking the checkbox was the only way to escape a loop that the page
+    // was doing, but now the user can just close the page.
+    *did_suppress_message = false;
   } else {
     AppModalDialogManager()->RunJavaScriptDialog(
-        web_contents, origin_url, message_type, message_text,
+        alerting_web_contents, origin_url, message_type, message_text,
         default_prompt_text, callback, did_suppress_message);
   }
 
@@ -136,24 +179,49 @@ bool JavaScriptDialogTabHelper::HandleJavaScriptDialog(
     content::WebContents* web_contents,
     bool accept,
     const base::string16* prompt_override) {
-  if (!IsEnabled()) {
-    return AppModalDialogManager()->HandleJavaScriptDialog(web_contents, accept,
-                                                           prompt_override);
+  if (dialog_) {
+    CloseDialog(false /*suppress_callback*/, accept,
+                prompt_override ? *prompt_override : base::string16());
+    return true;
   }
 
-  NOTREACHED() << "auto-dismissing dialog code does not yet exist";
-  return false;
+  // Handle any app-modal dialogs being run by the app-modal dialog system.
+  return AppModalDialogManager()->HandleJavaScriptDialog(web_contents, accept,
+                                                         prompt_override);
 }
 
 void JavaScriptDialogTabHelper::CancelDialogs(
     content::WebContents* web_contents,
     bool suppress_callbacks,
     bool reset_state) {
-  // Cancel any app-modal dialogs that may be going.
-  if (!IsEnabled()) {
-    return AppModalDialogManager()->CancelDialogs(
-        web_contents, suppress_callbacks, reset_state);
-  }
+  if (dialog_)
+    CloseDialog(suppress_callbacks, false, base::string16());
+
+  // Cancel any app-modal dialogs being run by the app-modal dialog system.
+  return AppModalDialogManager()->CancelDialogs(
+      web_contents, suppress_callbacks, reset_state);
+}
+
+void JavaScriptDialogTabHelper::WasHidden() {
+  if (dialog_)
+    CloseDialog(false, false, base::string16());
+}
+
+void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) {
+  if (dialog_ && !IsWebContentsForemost(web_contents()))
+    CloseDialog(false, false, base::string16());
+}
+
+void JavaScriptDialogTabHelper::CloseDialog(bool suppress_callback,
+                                            bool success,
+                                            const base::string16& user_input) {
+  DCHECK(dialog_);
+
+  dialog_->CloseDialogWithoutCallback();
+  if (!suppress_callback)
+    dialog_callback_.Run(success, user_input);
 
-  // More work here for the auto-dismissing dialogs.
+  dialog_.reset();
+  dialog_callback_.Reset();
+  BrowserList::RemoveObserver(this);
 }