Skip to content

GitLab

T
tangled
Commit c209b74c authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot
Browse files
Options

Revert "Disable logic to kill prerender when JavaScript alert fires" 

This reverts commit 401cd174.

Reason for revert: Changes in content/browser are not needed and may
cause existing other use cases of ShouldSuppressDialogs() to break.
The |delegate_| in content/browser could be a class different than prerender_contents. 

Original change's description:
> Disable logic to kill prerender when JavaScript alert fires
> 
> This is no longer needed with NoStatePrefetch
> 
> Change-Id: Iedfd77ae5d221609eadfdb001ff04fb3b83e100e
> Bug: 755936
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1978496
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#727345}

TBR=kinuko@chromium.org,tbansal@chromium.org,ryansturm@chromium.org

Change-Id: I59533f852d55e0540730540b23f563efe3e752b8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 755936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981330Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727348}
parent 0cfa7036
Hide whitespace changes
Inline Side-by-side
Showing with 61 additions and 4 deletions
chrome/browser/prerender/prerender_browsertest.cc
View file @ c209b74c
...@@ -1145,6 +1145,20 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap2) { ...@@ -1145,6 +1145,20 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNoCommitNoSwap2) {
false); false);
} }
// Checks that the prerendering of a page is canceled correctly when a
// Javascript alert is called.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderAlertBeforeOnload) {
PrerenderTestURL("/prerender/prerender_alert_before_onload.html",
FINAL_STATUS_JAVASCRIPT_ALERT, 0);
}
// Checks that the prerendering of a page is canceled correctly when a
// Javascript alert is called.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderAlertAfterOnload) {
PrerenderTestURL("/prerender/prerender_alert_after_onload.html",
FINAL_STATUS_JAVASCRIPT_ALERT, 1);
}
// Checks that plugins are not loaded while a page is being preloaded, but // Checks that plugins are not loaded while a page is being preloaded, but
// are loaded when the page is displayed. // are loaded when the page is displayed.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderDelayLoadPlugin) { IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderDelayLoadPlugin) {
......
chrome/browser/prerender/prerender_contents.cc
View file @ c209b74c
...@@ -134,6 +134,15 @@ class PrerenderContents::WebContentsDelegateImpl ...@@ -134,6 +134,15 @@ class PrerenderContents::WebContentsDelegateImpl
return false; return false;
} }
bool ShouldSuppressDialogs(WebContents* source) override {
// We still want to show the user the message when they navigate to this
// page, so cancel this prerender.
prerender_contents_->Destroy(FINAL_STATUS_JAVASCRIPT_ALERT);
// Always suppress JavaScript messages if they're triggered by a page being
// prerendered.
return true;
}
void RegisterProtocolHandler(WebContents* web_contents, void RegisterProtocolHandler(WebContents* web_contents,
const std::string& protocol, const std::string& protocol,
const GURL& url, const GURL& url,
......
chrome/browser/prerender/prerender_final_status.h
View file @ c209b74c
...@@ -25,7 +25,7 @@ enum FinalStatus { ...@@ -25,7 +25,7 @@ enum FinalStatus {
FINAL_STATUS_CREATE_NEW_WINDOW = 5, FINAL_STATUS_CREATE_NEW_WINDOW = 5,
FINAL_STATUS_PROFILE_DESTROYED = 6, FINAL_STATUS_PROFILE_DESTROYED = 6,
FINAL_STATUS_APP_TERMINATING = 7, FINAL_STATUS_APP_TERMINATING = 7,
// Obsolete: FINAL_STATUS_JAVASCRIPT_ALERT = 8, FINAL_STATUS_JAVASCRIPT_ALERT = 8,
FINAL_STATUS_AUTH_NEEDED = 9, FINAL_STATUS_AUTH_NEEDED = 9,
// Obsolete: FINAL_STATUS_HTTPS = 10, // Obsolete: FINAL_STATUS_HTTPS = 10,
FINAL_STATUS_DOWNLOAD = 11, FINAL_STATUS_DOWNLOAD = 11,
......
chrome/test/data/prerender/prerender_alert_after_onload.html 0 → 100644
View file @ c209b74c
<html>
<!--
This test checks to make sure an alert after onLoad causes
prerendering to fail.
-->
<head>
<title>Prerender alert after onload cancellation</title>
<script language="javascript" type="text/javascript">
window.onload = function() {
// Delay the alert by an event loop iteration so the alert
// happens after the page has loaded, rather than just before.
setTimeout(alert, 0, "Testing prerender");
};
</script>
</head>
<body></body>
</html>
chrome/test/data/prerender/prerender_alert_before_onload.html 0 → 100644
View file @ c209b74c
<html>
<!--
This test checks to make sure an alert before onLoad causes
prerendering to fail.
-->
<head>
<title>Prerender alert before onload cancellation</title>
<script language="javascript" type="text/javascript">
alert("Testing prerender");
</script>
</head>
<body></body>
</html>
content/browser/web_contents/web_contents_impl.cc
View file @ c209b74c
...@@ -5502,7 +5502,8 @@ void WebContentsImpl::RunJavaScriptDialog(RenderFrameHost* render_frame_host, ...@@ -5502,7 +5502,8 @@ void WebContentsImpl::RunJavaScriptDialog(RenderFrameHost* render_frame_host,
// Suppress JavaScript dialogs when requested. Also suppress messages when // Suppress JavaScript dialogs when requested. Also suppress messages when
// showing an interstitial as it's shown over the previous page and we don't // showing an interstitial as it's shown over the previous page and we don't
// want the hidden page's dialogs to interfere with the interstitial. // want the hidden page's dialogs to interfere with the interstitial.
bool should_suppress = ShowingInterstitialPage(); bool should_suppress = ShowingInterstitialPage() ||
(delegate_ && delegate_->ShouldSuppressDialogs(this));
bool has_non_devtools_handlers = delegate_ && dialog_manager_; bool has_non_devtools_handlers = delegate_ && dialog_manager_;
bool has_handlers = page_handlers.size() || has_non_devtools_handlers; bool has_handlers = page_handlers.size() || has_non_devtools_handlers;
bool suppress_this_message = should_suppress || !has_handlers; bool suppress_this_message = should_suppress || !has_handlers;
...@@ -5580,7 +5581,8 @@ void WebContentsImpl::RunBeforeUnloadConfirm( ...@@ -5580,7 +5581,8 @@ void WebContentsImpl::RunBeforeUnloadConfirm(
javascript_dialog_navigation_deferrer_ = javascript_dialog_navigation_deferrer_ =
std::make_unique<JavaScriptDialogNavigationDeferrer>(); std::make_unique<JavaScriptDialogNavigationDeferrer>();
bool should_suppress = ShowingInterstitialPage() || !rfhi->is_active(); bool should_suppress = ShowingInterstitialPage() || !rfhi->is_active() ||
(delegate_ && delegate_->ShouldSuppressDialogs(this));
bool has_non_devtools_handlers = delegate_ && dialog_manager_; bool has_non_devtools_handlers = delegate_ && dialog_manager_;
bool has_handlers = page_handlers.size() || has_non_devtools_handlers; bool has_handlers = page_handlers.size() || has_non_devtools_handlers;
if (should_suppress || !has_handlers) { if (should_suppress || !has_handlers) {
......
tools/metrics/histograms/enums.xml
View file @ c209b74c
...@@ -50971,7 +50971,7 @@ Called by update_net_trust_anchors.py.--> ...@@ -50971,7 +50971,7 @@ Called by update_net_trust_anchors.py.-->
<int value="5" label="CREATE_NEW_WINDOW"/> <int value="5" label="CREATE_NEW_WINDOW"/>
<int value="6" label="PROFILE_DESTROYED"/> <int value="6" label="PROFILE_DESTROYED"/>
<int value="7" label="APP_TERMINATING"/> <int value="7" label="APP_TERMINATING"/>
<int value="8" label="JAVASCRIPT_ALERT (Obsolete)"/> <int value="8" label="JAVASCRIPT_ALERT"/>
<int value="9" label="AUTH_NEEDED"/> <int value="9" label="AUTH_NEEDED"/>
<int value="10" label="HTTPS"/> <int value="10" label="HTTPS"/>
<int value="11" label="DOWNLOAD"/> <int value="11" label="DOWNLOAD"/>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment