JavaScript dialogs cause a page to lose fullscreen.

content/browser/web_contents/web_contents_impl_browsertest.cc

Index: content/browser/web_contents/web_contents_impl_browsertest.cc
diff --git a/content/browser/web_contents/web_contents_impl_browsertest.cc b/content/browser/web_contents/web_contents_impl_browsertest.cc
index 71c3c4239040f8529b97f593f4c3c8eac826e64a..b23fa736613228ed9e689306ec8a55374ec4b224 100644
--- a/content/browser/web_contents/web_contents_impl_browsertest.cc
+++ b/content/browser/web_contents/web_contents_impl_browsertest.cc
@@ -942,7 +942,8 @@ namespace {
 class TestJavaScriptDialogManager : public JavaScriptDialogManager,
                                     public WebContentsDelegate {
  public:
-  TestJavaScriptDialogManager() : message_loop_runner_(new MessageLoopRunner) {}
+  TestJavaScriptDialogManager()
+      : is_fullscreen_(false), message_loop_runner_(new MessageLoopRunner) {}
   ~TestJavaScriptDialogManager() override {}
 
   void Wait() {
@@ -959,6 +960,20 @@ class TestJavaScriptDialogManager : public JavaScriptDialogManager,
     return this;
   }
 
+  void EnterFullscreenModeForTab(WebContents* web_contents,
+                                 const GURL& origin) override {
+    is_fullscreen_ = true;
+  }
+
+  void ExitFullscreenModeForTab(WebContents*) override {
+    is_fullscreen_ = false;
+  }
+
+  bool IsFullscreenForTabOrPending(
+      const WebContents* web_contents) const override {
+    return is_fullscreen_;
+  }
+
   // JavaScriptDialogManager
 
   void RunJavaScriptDialog(WebContents* web_contents,
@@ -976,7 +991,10 @@ class TestJavaScriptDialogManager : public JavaScriptDialogManager,
 
   void RunBeforeUnloadDialog(WebContents* web_contents,
                              bool is_reload,
-                             const DialogClosedCallback& callback) override {}
+                             const DialogClosedCallback& callback) override {
+    callback.Run(true, base::string16());
+    message_loop_runner_->Quit();
+  }
 
   bool HandleJavaScriptDialog(WebContents* web_contents,
                               bool accept,
@@ -990,6 +1008,8 @@ class TestJavaScriptDialogManager : public JavaScriptDialogManager,
  private:
   std::string last_message_;
 
+  bool is_fullscreen_;
+
   // The MessageLoopRunner used to spin the message loop.
   scoped_refptr<MessageLoopRunner> message_loop_runner_;
 
@@ -1395,4 +1415,56 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, UserAgentOverride) {
                  old_delegate));
 }
 
+IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,

[Matt Giuca, 2017/06/09 06:16:37]

Excellent tests, thanks.

[Avi (use Gerrit), 2017/06/09 14:36:23]

Acknowledged.

+                       DialogsFromJavaScriptEndFullscreen) {
+  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell()->web_contents());
+  TestJavaScriptDialogManager dialog_manager;
+  wc->SetDelegate(&dialog_manager);
+
+  GURL url("about:blank");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  // alert
+  wc->EnterFullscreenMode(url);
+  EXPECT_TRUE(wc->IsFullscreenForCurrentTab());
+  std::string script = "alert('hi')";
+  EXPECT_TRUE(content::ExecuteScript(wc, script));
+  dialog_manager.Wait();
+  EXPECT_FALSE(wc->IsFullscreenForCurrentTab());
+
+  // confirm
+  wc->EnterFullscreenMode(url);
+  EXPECT_TRUE(wc->IsFullscreenForCurrentTab());
+  script = "confirm('hi')";
+  EXPECT_TRUE(content::ExecuteScript(wc, script));
+  dialog_manager.Wait();
+  EXPECT_FALSE(wc->IsFullscreenForCurrentTab());
+
+  // prompt
+  wc->EnterFullscreenMode(url);
+  EXPECT_TRUE(wc->IsFullscreenForCurrentTab());
+  script = "prompt('hi')";
+  EXPECT_TRUE(content::ExecuteScript(wc, script));
+  dialog_manager.Wait();
+  EXPECT_FALSE(wc->IsFullscreenForCurrentTab());
+
+  // beforeunload
+  wc->EnterFullscreenMode(url);
+  EXPECT_TRUE(wc->IsFullscreenForCurrentTab());
+  // Disable the hang monitor, otherwise there will be a race between the

[Matt Giuca, 2017/06/09 06:16:37]

Nit: Replace the commas with parens: "(otherwise ... timer)".

[Avi (use Gerrit), 2017/06/09 14:36:23]

Done.

+  // beforeunload dialog and the beforeunload hang timer, and give the page a
+  // gesture to allow dialogs.
+  wc->GetMainFrame()->DisableBeforeUnloadHangMonitorForTesting();
+  wc->GetMainFrame()->ExecuteJavaScriptWithUserGestureForTests(
+      base::string16());
+  script = "window.onbeforeunload=function(e){return 'x'};";

[Matt Giuca, 2017/06/09 06:16:37]

nit: Insert spaces in the JavaScript for readability (shouldn't overflow the
line).

[Avi (use Gerrit), 2017/06/09 14:36:23]

Done.

[Matt Giuca, 2017/06/13 01:53:38]

I meant also around the equals sign. And may as well also have a semicolon.

Oh well, let's not block landing this but if you work on this code again maybe
do that.

+  EXPECT_TRUE(content::ExecuteScript(wc, script));
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  dialog_manager.Wait();
+  EXPECT_FALSE(wc->IsFullscreenForCurrentTab());
+
+  wc->SetDelegate(nullptr);
+  wc->SetJavaScriptDialogManagerForTesting(nullptr);
+}
+
 }  // namespace content