diff options
author | mathiash <mathiash@opera.com> | 2014-11-19 00:18:50 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-11-19 08:19:29 +0000 |
commit | 72a5e46303c13d806475cd01aedfb1305fcde5d7 (patch) | |
tree | 115076de45eb77d6d0ec3ec220d681ad977acd23 | |
parent | bc4af0395bb3c948c7704a83bc989ca770d1d7f9 (diff) | |
download | chromium_src-72a5e46303c13d806475cd01aedfb1305fcde5d7.zip chromium_src-72a5e46303c13d806475cd01aedfb1305fcde5d7.tar.gz chromium_src-72a5e46303c13d806475cd01aedfb1305fcde5d7.tar.bz2 |
Add WebContents source to methods in WebContentsDelegate
The added parameter helps us avoid a potential timing-related security issue. The UI can be instructed to open a new tab and show a dialog roughly at the same time, and since it's handled asynchronously the dialog may end up in front of the new tab. Knowing what web contents issued the dialog would allow us to prevent that from happening.
BUG=428793
Review URL: https://codereview.chromium.org/691823002
Cr-Commit-Position: refs/heads/master@{#304781}
30 files changed, 64 insertions, 48 deletions
diff --git a/android_webview/native/aw_web_contents_delegate.cc b/android_webview/native/aw_web_contents_delegate.cc index ff73c1d..ca49bb8 100644 --- a/android_webview/native/aw_web_contents_delegate.cc +++ b/android_webview/native/aw_web_contents_delegate.cc @@ -55,7 +55,7 @@ AwWebContentsDelegate::~AwWebContentsDelegate() { } content::JavaScriptDialogManager* -AwWebContentsDelegate::GetJavaScriptDialogManager() { +AwWebContentsDelegate::GetJavaScriptDialogManager(WebContents* source) { return g_javascript_dialog_manager.Pointer(); } diff --git a/android_webview/native/aw_web_contents_delegate.h b/android_webview/native/aw_web_contents_delegate.h index f45d312..b3c273e 100644 --- a/android_webview/native/aw_web_contents_delegate.h +++ b/android_webview/native/aw_web_contents_delegate.h @@ -19,8 +19,8 @@ class AwWebContentsDelegate public: AwWebContentsDelegate(JNIEnv* env, jobject obj); virtual ~AwWebContentsDelegate(); - virtual content::JavaScriptDialogManager* GetJavaScriptDialogManager() - override; + virtual content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* source) override; virtual void FindReply(content::WebContents* web_contents, int request_id, int number_of_matches, diff --git a/apps/custom_launcher_page_contents.cc b/apps/custom_launcher_page_contents.cc index 37e7125..150bfcf 100644 --- a/apps/custom_launcher_page_contents.cc +++ b/apps/custom_launcher_page_contents.cc @@ -87,7 +87,8 @@ bool CustomLauncherPageContents::IsPopupOrPanel( return true; } -bool CustomLauncherPageContents::ShouldSuppressDialogs() { +bool CustomLauncherPageContents::ShouldSuppressDialogs( + content::WebContents* source) { return true; } diff --git a/apps/custom_launcher_page_contents.h b/apps/custom_launcher_page_contents.h index d3140e6..95c7c65 100644 --- a/apps/custom_launcher_page_contents.h +++ b/apps/custom_launcher_page_contents.h @@ -52,7 +52,7 @@ class CustomLauncherPageContents bool user_gesture, bool* was_blocked) override; bool IsPopupOrPanel(const content::WebContents* source) const override; - bool ShouldSuppressDialogs() override; + bool ShouldSuppressDialogs(content::WebContents* source) override; bool PreHandleGestureEvent(content::WebContents* source, const blink::WebGestureEvent& event) override; content::ColorChooser* OpenColorChooser( diff --git a/athena/content/web_activity.cc b/athena/content/web_activity.cc index 087b5d0..d302a08 100644 --- a/athena/content/web_activity.cc +++ b/athena/content/web_activity.cc @@ -365,7 +365,8 @@ class AthenaWebView : public views::WebView { layer->SetOpacity(0.f); } - content::JavaScriptDialogManager* GetJavaScriptDialogManager() override { + content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* contents) override { return GetJavaScriptDialogManagerInstance(); } diff --git a/chrome/browser/android/chrome_web_contents_delegate_android.cc b/chrome/browser/android/chrome_web_contents_delegate_android.cc index 4adc125..8a0c196 100644 --- a/chrome/browser/android/chrome_web_contents_delegate_android.cc +++ b/chrome/browser/android/chrome_web_contents_delegate_android.cc @@ -214,7 +214,8 @@ void ChromeWebContentsDelegateAndroid::FindMatchRectsReply( } content::JavaScriptDialogManager* -ChromeWebContentsDelegateAndroid::GetJavaScriptDialogManager() { +ChromeWebContentsDelegateAndroid::GetJavaScriptDialogManager( + WebContents* source) { return GetJavaScriptDialogManagerInstance(); } diff --git a/chrome/browser/android/chrome_web_contents_delegate_android.h b/chrome/browser/android/chrome_web_contents_delegate_android.h index 6a1dd1e..1fa1766 100644 --- a/chrome/browser/android/chrome_web_contents_delegate_android.h +++ b/chrome/browser/android/chrome_web_contents_delegate_android.h @@ -54,7 +54,7 @@ class ChromeWebContentsDelegateAndroid const std::vector<gfx::RectF>& rects, const gfx::RectF& active_rect) override; virtual content::JavaScriptDialogManager* - GetJavaScriptDialogManager() override; + GetJavaScriptDialogManager(content::WebContents* source) override; virtual void RequestMediaAccessPermission( content::WebContents* web_contents, const content::MediaStreamRequest& request, diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc index 0b23a79..1e2a0e6 100644 --- a/chrome/browser/devtools/devtools_window.cc +++ b/chrome/browser/devtools/devtools_window.cc @@ -929,11 +929,13 @@ void DevToolsWindow::HandleKeyboardEvent( inspected_window->HandleKeyboardEvent(event); } -content::JavaScriptDialogManager* DevToolsWindow::GetJavaScriptDialogManager() { +content::JavaScriptDialogManager* DevToolsWindow::GetJavaScriptDialogManager( + WebContents* source) { WebContents* inspected_web_contents = GetInspectedWebContents(); - return (inspected_web_contents && inspected_web_contents->GetDelegate()) ? - inspected_web_contents->GetDelegate()->GetJavaScriptDialogManager() : - content::WebContentsDelegate::GetJavaScriptDialogManager(); + return (inspected_web_contents && inspected_web_contents->GetDelegate()) + ? inspected_web_contents->GetDelegate() + ->GetJavaScriptDialogManager(inspected_web_contents) + : content::WebContentsDelegate::GetJavaScriptDialogManager(source); } content::ColorChooser* DevToolsWindow::OpenColorChooser( diff --git a/chrome/browser/devtools/devtools_window.h b/chrome/browser/devtools/devtools_window.h index 449eac8..905ede7 100644 --- a/chrome/browser/devtools/devtools_window.h +++ b/chrome/browser/devtools/devtools_window.h @@ -252,7 +252,8 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate, void HandleKeyboardEvent( content::WebContents* source, const content::NativeWebKeyboardEvent& event) override; - content::JavaScriptDialogManager* GetJavaScriptDialogManager() override; + content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* source) override; content::ColorChooser* OpenColorChooser( content::WebContents* web_contents, SkColor color, diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc index b1d8cf1..4a55361 100644 --- a/chrome/browser/prerender/prerender_contents.cc +++ b/chrome/browser/prerender/prerender_contents.cc @@ -171,7 +171,7 @@ class PrerenderContents::WebContentsDelegateImpl return false; } - bool ShouldSuppressDialogs() override { + 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); diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc index 643e63b..c7f84ed 100644 --- a/chrome/browser/prerender/prerender_manager.cc +++ b/chrome/browser/prerender/prerender_manager.cc @@ -191,9 +191,10 @@ class PrerenderManager::OnCloseWebContentsDeleter ScheduleWebContentsForDeletion(false); } - bool ShouldSuppressDialogs() override { + bool ShouldSuppressDialogs(WebContents* source) override { // Use this as a proxy for getting statistics on how often we fail to honor // the beforeunload event. + DCHECK_EQ(tab_, source); suppressed_dialog_ = true; return true; } diff --git a/chrome/browser/tab_contents/background_contents.cc b/chrome/browser/tab_contents/background_contents.cc index 20902f3..48f0f4d 100644 --- a/chrome/browser/tab_contents/background_contents.cc +++ b/chrome/browser/tab_contents/background_contents.cc @@ -94,7 +94,7 @@ void BackgroundContents::CloseContents(WebContents* source) { delete this; } -bool BackgroundContents::ShouldSuppressDialogs() { +bool BackgroundContents::ShouldSuppressDialogs(WebContents* source) { return true; } diff --git a/chrome/browser/tab_contents/background_contents.h b/chrome/browser/tab_contents/background_contents.h index 5e443fd..85bf8dea 100644 --- a/chrome/browser/tab_contents/background_contents.h +++ b/chrome/browser/tab_contents/background_contents.h @@ -56,7 +56,7 @@ class BackgroundContents : public content::WebContentsDelegate, // content::WebContentsDelegate implementation: void CloseContents(content::WebContents* source) override; - bool ShouldSuppressDialogs() override; + bool ShouldSuppressDialogs(content::WebContents* source) override; void DidNavigateMainFramePostCommit(content::WebContents* tab) override; void AddNewContents(content::WebContents* source, content::WebContents* new_contents, diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 925563c..0d269e3 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -1622,7 +1622,8 @@ void Browser::DidNavigateToPendingEntry(WebContents* web_contents) { UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_TAB_STATE); } -content::JavaScriptDialogManager* Browser::GetJavaScriptDialogManager() { +content::JavaScriptDialogManager* Browser::GetJavaScriptDialogManager( + WebContents* source) { return GetJavaScriptDialogManagerInstance(); } diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h index 4e5b674..595163e 100644 --- a/chrome/browser/ui/browser.h +++ b/chrome/browser/ui/browser.h @@ -597,7 +597,8 @@ class Browser : public TabStripModelObserver, void DidNavigateMainFramePostCommit( content::WebContents* web_contents) override; void DidNavigateToPendingEntry(content::WebContents* web_contents) override; - content::JavaScriptDialogManager* GetJavaScriptDialogManager() override; + content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* source) override; content::ColorChooser* OpenColorChooser( content::WebContents* web_contents, SkColor color, diff --git a/chrome/browser/ui/fast_unload_controller.cc b/chrome/browser/ui/fast_unload_controller.cc index 566a136..8929457 100644 --- a/chrome/browser/ui/fast_unload_controller.cc +++ b/chrome/browser/ui/fast_unload_controller.cc @@ -33,7 +33,7 @@ class FastUnloadController::DetachedWebContentsDelegate private: // WebContentsDelegate implementation. - bool ShouldSuppressDialogs() override { + bool ShouldSuppressDialogs(content::WebContents* source) override { return true; // Return true so dialogs are suppressed. } diff --git a/content/browser/device_sensors/device_inertial_sensor_browsertest.cc b/content/browser/device_sensors/device_inertial_sensor_browsertest.cc index 3380d8a..81da0a1 100644 --- a/content/browser/device_sensors/device_inertial_sensor_browsertest.cc +++ b/content/browser/device_sensors/device_inertial_sensor_browsertest.cc @@ -202,9 +202,9 @@ class DeviceInertialSensorBrowserTest : public ContentBrowserTest { } void WaitForAlertDialogAndQuitAfterDelay(base::TimeDelta delay) { - ShellJavaScriptDialogManager* dialog_manager= + ShellJavaScriptDialogManager* dialog_manager = static_cast<ShellJavaScriptDialogManager*>( - shell()->GetJavaScriptDialogManager()); + shell()->GetJavaScriptDialogManager(shell()->web_contents())); scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner(); dialog_manager->set_dialog_request_callback( diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc index f16150b..2a1ace9 100644 --- a/content/browser/devtools/protocol/page_handler.cc +++ b/content/browser/devtools/protocol/page_handler.cc @@ -373,7 +373,7 @@ Response PageHandler::HandleJavaScriptDialog(bool accept, return Response::InternalError("No JavaScript dialog to handle"); JavaScriptDialogManager* manager = - web_contents->GetDelegate()->GetJavaScriptDialogManager(); + web_contents->GetDelegate()->GetJavaScriptDialogManager(web_contents); if (manager && manager->HandleJavaScriptDialog( web_contents, accept, prompt_text ? &prompt_override : nullptr)) { return Response::OK(); diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index e6f8713..d0a157c 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -3396,15 +3396,14 @@ void WebContentsImpl::RunJavaScriptMessage( // want the hidden page's dialogs to interfere with the interstitial. bool suppress_this_message = static_cast<RenderFrameHostImpl*>(render_frame_host)->is_swapped_out() || - ShowingInterstitialPage() || - !delegate_ || - delegate_->ShouldSuppressDialogs() || - !delegate_->GetJavaScriptDialogManager(); + ShowingInterstitialPage() || !delegate_ || + delegate_->ShouldSuppressDialogs(this) || + !delegate_->GetJavaScriptDialogManager(this); if (!suppress_this_message) { std::string accept_lang = GetContentClient()->browser()-> GetAcceptLangs(GetBrowserContext()); - dialog_manager_ = delegate_->GetJavaScriptDialogManager(); + dialog_manager_ = delegate_->GetJavaScriptDialogManager(this); dialog_manager_->RunJavaScriptDialog( this, frame_url.GetOrigin(), @@ -3444,17 +3443,16 @@ void WebContentsImpl::RunBeforeUnloadConfirm( delegate_->WillRunBeforeUnloadConfirm(); bool suppress_this_message = - rfhi->rfh_state() != RenderFrameHostImpl::STATE_DEFAULT || - !delegate_ || - delegate_->ShouldSuppressDialogs() || - !delegate_->GetJavaScriptDialogManager(); + rfhi->rfh_state() != RenderFrameHostImpl::STATE_DEFAULT || !delegate_ || + delegate_->ShouldSuppressDialogs(this) || + !delegate_->GetJavaScriptDialogManager(this); if (suppress_this_message) { rfhi->JavaScriptDialogClosed(reply_msg, true, base::string16(), true); return; } is_showing_before_unload_dialog_ = true; - dialog_manager_ = delegate_->GetJavaScriptDialogManager(); + dialog_manager_ = delegate_->GetJavaScriptDialogManager(this); dialog_manager_->RunBeforeUnloadDialog( this, message, is_reload, base::Bind(&WebContentsImpl::OnDialogClosed, base::Unretained(this), diff --git a/content/public/browser/web_contents_delegate.cc b/content/public/browser/web_contents_delegate.cc index f264df7..2128a5f 100644 --- a/content/public/browser/web_contents_delegate.cc +++ b/content/public/browser/web_contents_delegate.cc @@ -33,7 +33,7 @@ gfx::Rect WebContentsDelegate::GetRootWindowResizerRect() const { return gfx::Rect(); } -bool WebContentsDelegate::ShouldSuppressDialogs() { +bool WebContentsDelegate::ShouldSuppressDialogs(WebContents* source) { return false; } @@ -140,7 +140,8 @@ bool WebContentsDelegate::ShouldCreateWebContents( return true; } -JavaScriptDialogManager* WebContentsDelegate::GetJavaScriptDialogManager() { +JavaScriptDialogManager* WebContentsDelegate::GetJavaScriptDialogManager( + WebContents* source) { return NULL; } diff --git a/content/public/browser/web_contents_delegate.h b/content/public/browser/web_contents_delegate.h index bcd5caf..92c8dcd 100644 --- a/content/public/browser/web_contents_delegate.h +++ b/content/public/browser/web_contents_delegate.h @@ -176,7 +176,7 @@ class CONTENT_EXPORT WebContentsDelegate { // Returns true if javascript dialogs and unload alerts are suppressed. // Default is false. - virtual bool ShouldSuppressDialogs(); + virtual bool ShouldSuppressDialogs(WebContents* source); // Returns whether pending NavigationEntries for aborted browser-initiated // navigations should be preserved (and thus returned from GetVisibleURL). @@ -342,7 +342,8 @@ class CONTENT_EXPORT WebContentsDelegate { // Returns a pointer to a service to manage JavaScript dialogs. May return // NULL in which case dialogs aren't shown. - virtual JavaScriptDialogManager* GetJavaScriptDialogManager(); + virtual JavaScriptDialogManager* GetJavaScriptDialogManager( + WebContents* source); // Called when color chooser should open. Returns the opened color chooser. // Returns NULL if we failed to open the color chooser (e.g. when there is a diff --git a/content/public/test/content_browser_test_utils.cc b/content/public/test/content_browser_test_utils.cc index bc7a903..3fc0971 100644 --- a/content/public/test/content_browser_test_utils.cc +++ b/content/public/test/content_browser_test_utils.cc @@ -81,9 +81,9 @@ bool NavigateToURLAndExpectNoCommit(Shell* window, const GURL& url) { } void WaitForAppModalDialog(Shell* window) { - ShellJavaScriptDialogManager* dialog_manager= + ShellJavaScriptDialogManager* dialog_manager = static_cast<ShellJavaScriptDialogManager*>( - window->GetJavaScriptDialogManager()); + window->GetJavaScriptDialogManager(window->web_contents())); scoped_refptr<MessageLoopRunner> runner = new MessageLoopRunner(); dialog_manager->set_dialog_request_callback(runner->QuitClosure()); diff --git a/content/shell/browser/shell.cc b/content/shell/browser/shell.cc index c31028f..58b53e2 100644 --- a/content/shell/browser/shell.cc +++ b/content/shell/browser/shell.cc @@ -329,7 +329,8 @@ void Shell::DidNavigateMainFramePostCommit(WebContents* web_contents) { PlatformSetAddressBarURL(web_contents->GetLastCommittedURL()); } -JavaScriptDialogManager* Shell::GetJavaScriptDialogManager() { +JavaScriptDialogManager* Shell::GetJavaScriptDialogManager( + WebContents* source) { if (!dialog_manager_) { const CommandLine& command_line = *CommandLine::ForCurrentProcess(); dialog_manager_.reset(command_line.HasSwitch(switches::kDumpRenderTree) diff --git a/content/shell/browser/shell.h b/content/shell/browser/shell.h index ee08bcf..bb06a0d 100644 --- a/content/shell/browser/shell.h +++ b/content/shell/browser/shell.h @@ -137,7 +137,8 @@ class Shell : public WebContentsDelegate, void CloseContents(WebContents* source) override; bool CanOverscrollContent() const override; void DidNavigateMainFramePostCommit(WebContents* web_contents) override; - JavaScriptDialogManager* GetJavaScriptDialogManager() override; + JavaScriptDialogManager* GetJavaScriptDialogManager( + WebContents* source) override; #if defined(OS_MACOSX) void HandleKeyboardEvent(WebContents* source, const NativeWebKeyboardEvent& event) override; diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc index 8ce9cf6..f212b80 100644 --- a/extensions/browser/app_window/app_window.cc +++ b/extensions/browser/app_window/app_window.cc @@ -887,7 +887,9 @@ void AppWindow::CloseContents(WebContents* contents) { native_app_window_->Close(); } -bool AppWindow::ShouldSuppressDialogs() { return true; } +bool AppWindow::ShouldSuppressDialogs(WebContents* source) { + return true; +} content::ColorChooser* AppWindow::OpenColorChooser( WebContents* web_contents, diff --git a/extensions/browser/app_window/app_window.h b/extensions/browser/app_window/app_window.h index 5cbe8fa..12029c7 100644 --- a/extensions/browser/app_window/app_window.h +++ b/extensions/browser/app_window/app_window.h @@ -360,7 +360,7 @@ class AppWindow : public content::NotificationObserver, // content::WebContentsDelegate implementation. void CloseContents(content::WebContents* contents) override; - bool ShouldSuppressDialogs() override; + bool ShouldSuppressDialogs(content::WebContents* source) override; content::ColorChooser* OpenColorChooser( content::WebContents* web_contents, SkColor color, diff --git a/extensions/browser/extension_host.cc b/extensions/browser/extension_host.cc index e19d146..4f2941b 100644 --- a/extensions/browser/extension_host.cc +++ b/extensions/browser/extension_host.cc @@ -372,7 +372,8 @@ void ExtensionHost::RenderViewDeleted(RenderViewHost* render_view_host) { render_view_host_ = host_contents_->GetRenderViewHost(); } -content::JavaScriptDialogManager* ExtensionHost::GetJavaScriptDialogManager() { +content::JavaScriptDialogManager* ExtensionHost::GetJavaScriptDialogManager( + WebContents* source) { return delegate_->GetJavaScriptDialogManager(); } diff --git a/extensions/browser/extension_host.h b/extensions/browser/extension_host.h index def200d..4f81501 100644 --- a/extensions/browser/extension_host.h +++ b/extensions/browser/extension_host.h @@ -85,7 +85,8 @@ class ExtensionHost : public content::WebContentsDelegate, void DidStopLoading(content::RenderViewHost* render_view_host) override; // content::WebContentsDelegate - content::JavaScriptDialogManager* GetJavaScriptDialogManager() override; + content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* source) override; void AddNewContents(content::WebContents* source, content::WebContents* new_contents, WindowOpenDisposition disposition, diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc index e0adca0..8433d52 100644 --- a/extensions/browser/guest_view/web_view/web_view_guest.cc +++ b/extensions/browser/guest_view/web_view/web_view_guest.cc @@ -902,8 +902,8 @@ void WebViewGuest::WillAttachToEmbedder() { PushWebViewStateToIOThread(); } -content::JavaScriptDialogManager* - WebViewGuest::GetJavaScriptDialogManager() { +content::JavaScriptDialogManager* WebViewGuest::GetJavaScriptDialogManager( + WebContents* source) { return &javascript_dialog_helper_; } diff --git a/extensions/browser/guest_view/web_view/web_view_guest.h b/extensions/browser/guest_view/web_view/web_view_guest.h index 7522022..e3d1a4f 100644 --- a/extensions/browser/guest_view/web_view/web_view_guest.h +++ b/extensions/browser/guest_view/web_view/web_view_guest.h @@ -136,7 +136,8 @@ class WebViewGuest : public GuestView<WebViewGuest>, const GURL& url, const std::string& request_method, const base::Callback<void(bool)>& callback) override; - content::JavaScriptDialogManager* GetJavaScriptDialogManager() override; + content::JavaScriptDialogManager* GetJavaScriptDialogManager( + content::WebContents* source) override; content::ColorChooser* OpenColorChooser( content::WebContents* web_contents, SkColor color, |