Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1393)

Unified Diff: content/browser/web_contents/web_contents_impl_browsertest.cc

Issue 2906133004: JavaScript dialogs cause a page to lose fullscreen. (Closed)
Patch Set: with a test Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 .
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 readabili
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 we
+ 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

Powered by Google App Engine
This is Rietveld 408576698