Commit 970988a3 authored by Balazs Engedy's avatar Balazs Engedy Committed by Commit Bot

Suppress permission prompts for detached WebContents.

If the active WebContents has a pending permission prompt when its
renderer crashes, and then the user closes the WebContents, the
following sequence of events takes place:
 1) The WebContents is detached from the Browser.
 2) This causes the WebView's crash overlay view to get torn down, and to
    make sure hit testing continues to work correctly on Mac, the hosted
    WebContents is made visible again temporarily.
 3) This triggers the PermissionRequestManager, which has pending requests,
    to show the prompt bubble again, but at this point there is no
    BrowserView to anchor the prompt to.

Make sure we silently suppress the prompt in step (3) as a quick fix.

Bug: 933321
Change-Id: If8dbf09e1faae33fc615e7a6117c155d5f8707c4
TBR: raymes@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1489245
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarKamila Hasanbega <hkamila@google.com>
Cr-Commit-Position: refs/heads/master@{#635975}
parent 035117fc
...@@ -328,6 +328,9 @@ void PermissionRequestManager::ShowBubble(bool is_reshow) { ...@@ -328,6 +328,9 @@ void PermissionRequestManager::ShowBubble(bool is_reshow) {
DCHECK(!tab_is_hidden_); DCHECK(!tab_is_hidden_);
view_ = view_factory_.Run(web_contents(), this); view_ = view_factory_.Run(web_contents(), this);
if (!view_)
return;
if (!is_reshow) if (!is_reshow)
PermissionUmaUtil::PermissionPromptShown(requests_); PermissionUmaUtil::PermissionPromptShown(requests_);
NotifyBubbleAdded(); NotifyBubbleAdded();
......
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "components/variations/variations_associated_data.h" #include "components/variations/variations_associated_data.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
...@@ -506,6 +507,40 @@ IN_PROC_BROWSER_TEST_F(PermissionDialogTest, SwitchBrowserWindow) { ...@@ -506,6 +507,40 @@ IN_PROC_BROWSER_TEST_F(PermissionDialogTest, SwitchBrowserWindow) {
owned_requests_.clear(); owned_requests_.clear();
} }
// Regression test for https://crbug.com/933321.
IN_PROC_BROWSER_TEST_F(
PermissionDialogTest,
ActiveTabClosedAfterRendererCrashesWithPendingPermissionRequest) {
ShowUi("geolocation");
ASSERT_TRUE(VerifyUi());
// Simulate a render process crash while the permission prompt is pending.
content::RenderViewHost* render_view_host =
browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost();
content::RenderProcessHost* render_process_host =
render_view_host->GetProcess();
content::RenderProcessHostWatcher crash_observer(
render_process_host,
content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
ASSERT_TRUE(render_process_host->Shutdown(0));
crash_observer.Wait();
// The permission request is still pending, but the BrowserView's WebView is
// now showing a crash overlay, so the permission prompt is hidden.
//
// Now close the tab. This will first detach the WebContents, causing the
// WebView's crash overlay to be torn down, which, in turn, will temporarily
// make the dying WebContents visible again, albeit without being attached to
// any BrowserView.
//
// Wait until the WebContents, and with it, the PermissionRequestManager, is
// gone, and make sure nothing crashes.
content::WebContentsDestroyedWatcher web_contents_destroyed_watcher(
browser()->tab_strip_model()->GetActiveWebContents());
browser()->tab_strip_model()->CloseAllTabs();
web_contents_destroyed_watcher.Wait();
}
// Host wants to run flash. // Host wants to run flash.
IN_PROC_BROWSER_TEST_F(PermissionDialogTest, InvokeUi_flash) { IN_PROC_BROWSER_TEST_F(PermissionDialogTest, InvokeUi_flash) {
ShowAndVerifyUi(); ShowAndVerifyUi();
......
...@@ -10,6 +10,11 @@ ...@@ -10,6 +10,11 @@
std::unique_ptr<PermissionPrompt> PermissionPrompt::Create( std::unique_ptr<PermissionPrompt> PermissionPrompt::Create(
content::WebContents* web_contents, content::WebContents* web_contents,
Delegate* delegate) { Delegate* delegate) {
return base::WrapUnique(new PermissionPromptImpl( Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
chrome::FindBrowserWithWebContents(web_contents), delegate)); if (!browser) {
DLOG(WARNING) << "Permission prompt suppressed because the WebContents is "
"not attached to any Browser window.";
return nullptr;
}
return base::WrapUnique(new PermissionPromptImpl(browser, delegate));
} }
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