Issue metadata
Sign in to add a comment
|
Certain permissions prompts not dismissed on navigation in background tabs
Reported by
leliuk...@yandex-team.ru,
Aug 21 2017
|
||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 YaBrowser/17.9.0.1340 Yowser/2.5 Safari/537.36 Steps to reproduce the problem: 1. Visit https://permission.site/ 2. Press "Protocol Handler" (prompt should appear). 3. Open another tab. 4. Close background tab with prompt. What is the expected behavior? Tab closes. What went wrong? DCHECK was hit: [89511:775:0821/133201.137234:FATAL:permission_request_manager.cc(131)] Check failed: requests_.empty(). 0 libbase.dylib 0x0000000106dea6ee base::debug::StackTrace::StackTrace(unsigned long) + 174 1 libbase.dylib 0x0000000106dea7ad base::debug::StackTrace::StackTrace(unsigned long) + 29 2 libbase.dylib 0x0000000106de8a5c base::debug::StackTrace::StackTrace() + 28 3 libbase.dylib 0x0000000106e87b8f logging::LogMessage::~LogMessage() + 479 4 libbase.dylib 0x0000000106e854f5 logging::LogMessage::~LogMessage() + 21 5 libchrome_dll.dylib 0x0000000109cfbb9f PermissionRequestManager::~PermissionRequestManager() + 271 6 libchrome_dll.dylib 0x0000000109cfc025 PermissionRequestManager::~PermissionRequestManager() + 21 7 libchrome_dll.dylib 0x0000000109cfc0a9 PermissionRequestManager::~PermissionRequestManager() + 25 8 libbase.dylib 0x00000001070484ec std::__1::pair<void const* const, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >::~pair() + 188 9 libbase.dylib 0x0000000107048425 std::__1::pair<void const* const, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >::~pair() + 21 10 libbase.dylib 0x000000010704a59e std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::erase(std::__1::__tree_const_iterator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__tree_node<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, void*>*, long>) + 718 11 libbase.dylib 0x0000000107049ee7 unsigned long std::__1::__tree<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::__map_value_compare<void const*, std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > >, std::__1::less<void const*>, true>, std::__1::allocator<std::__1::__value_type<void const*, std::__1::unique_ptr<base::SupportsUserData::Data, std::__1::default_delete<base::SupportsUserData::Data> > > > >::__erase_unique<void const*>(void const* const&) + 327 12 libbase.dylib 0x0000000107047f58 base::SupportsUserData::RemoveUserData(void const*) + 264 13 libchrome_dll.dylib 0x0000000109d0118b PermissionRequestManager::WebContentsDestroyed() + 59 14 libcontent.dylib 0x0000000123ae05d1 content::WebContentsImpl::~WebContentsImpl() + 6113 15 libcontent.dylib 0x0000000123ae22b5 content::WebContentsImpl::~WebContentsImpl() + 21 16 libcontent.dylib 0x0000000123ae23d9 content::WebContentsImpl::~WebContentsImpl() + 25 17 libchrome_dll.dylib 0x000000010ef9df84 TabStripModel::InternalCloseTab(content::WebContents*, int, bool) + 292 18 libchrome_dll.dylib 0x000000010ef9750e TabStripModel::InternalCloseTabs(std::__1::vector<int, std::__1::allocator<int> > const&, unsigned int) + 2270 19 libchrome_dll.dylib 0x000000010ef97a0b TabStripModel::CloseWebContentsAt(int, unsigned int) + 923 20 libchrome_dll.dylib 0x000000010f6625de -[TabStripController closeTab:] + 1790 21 libchrome_dll.dylib 0x000000010f659b57 -[TabController closeTab:] + 455 22 libsystem_trace.dylib 0x00007fffa61b93a7 _os_activity_initiate_impl + 53 23 AppKit 0x00007fff8e63a721 -[NSApplication(NSResponder) sendAction:to:from:] + 456 24 libchrome_dll.dylib 0x00000001097c2882 __43-[BrowserCrApplication sendAction:to:from:]_block_invoke + 98 25 libbase.dylib 0x0000000106e902ba base::mac::CallWithEHFrame(void () block_pointer) + 10 26 libchrome_dll.dylib 0x00000001097c273f -[BrowserCrApplication sendAction:to:from:] + 1199 27 AppKit 0x00007fff8e11ecc4 -[NSControl sendAction:to:] + 86 28 AppKit 0x00007fff8e11ebec __26-[NSCell _sendActionFrom:]_block_invoke + 136 29 libsystem_trace.dylib 0x00007fffa61b93a7 _os_activity_initiate_impl + 53 30 AppKit 0x00007fff8e11eb44 -[NSCell _sendActionFrom:] + 128 31 AppKit 0x00007fff8e161539 -[NSButtonCell _sendActionFrom:] + 98 32 libsystem_trace.dylib 0x00007fffa61b93a7 _os_activity_initiate_impl + 53 33 AppKit 0x00007fff8e17bf46 -[NSButtonCell performClick:] + 690 34 libui_base.dylib 0x00000001077f055d -[HoverButton mouseDown:] + 429 35 AppKit 0x00007fff8e7b624f -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:] + 6341 36 AppKit 0x00007fff8e7b2a6c -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 1942 37 AppKit 0x00007fff8e7b1f0a -[NSWindow(NSEventRouting) sendEvent:] + 541 38 libchrome_dll.dylib 0x000000010f48240a -[ChromeEventProcessingWindow sendEvent:] + 122 39 AppKit 0x00007fff8e636681 -[NSApplication(NSEvent) sendEvent:] + 1145 40 libchrome_dll.dylib 0x00000001097c2ed3 __34-[BrowserCrApplication sendEvent:]_block_invoke + 259 41 libbase.dylib 0x0000000106e902ba base::mac::CallWithEHFrame(void () block_pointer) + 10 42 libchrome_dll.dylib 0x00000001097c2c5f -[BrowserCrApplication sendEvent:] + 575 43 AppKit 0x00007fff8deb1427 -[NSApplication run] + 1002 44 libbase.dylib 0x0000000106ef186c base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 300 45 libbase.dylib 0x0000000106eef81e base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 110 46 libbase.dylib 0x0000000106edeb4b base::MessageLoop::Run() + 299 47 libbase.dylib 0x0000000106fd930e base::RunLoop::Run() + 286 48 libchrome_dll.dylib 0x00000001097d01c8 ChromeBrowserMainParts::MainMessageLoopRun(int*) + 376 49 libcontent.dylib 0x00000001226cc437 content::BrowserMainLoop::RunMainMessageLoopParts() + 455 50 libcontent.dylib 0x00000001226d6aac content::BrowserMainRunnerImpl::Run() + 396 51 libcontent.dylib 0x00000001226bf6ad content::BrowserMain(content::MainFunctionParams const&) + 397 52 libcontent.dylib 0x0000000124db3b69 content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) + 585 53 libcontent.dylib 0x0000000124db53d0 content::ContentMainRunnerImpl::Run() + 1280 54 libcontent.dylib 0x0000000124db1e4d content::ContentServiceManagerMainDelegate::RunEmbedderProcess() + 61 55 libembedder.dylib 0x00000001068f7167 service_manager::Main(service_manager::MainParams const&) + 1911 56 libcontent.dylib 0x0000000124db38d9 content::ContentMain(content::ContentMainParams const&) + 89 57 libchrome_dll.dylib 0x0000000107b303de ChromeMain + 270 58 Chromium 0x0000000104e9adb6 main + 630 59 libdyld.dylib 0x00007fffa5f87235 start + 1 60 ??? 0x0000000000000001 0x0 + 1 Did this work before? N/A Chrome version: 62.0.3190.0 Channel: dev OS Version: OS X 10.12.6 Flash Version: Shockwave Flash 26.0 r0
,
Aug 22 2017
https://chromium-review.googlesource.com/c/chromium/src/+/597507 should fix once it's done (see change in PermissionRequestManager::CleanUpRequests())
,
Aug 22 2017
I'll pull out from the CL linked about the relevant bit into an individual change. Turns out for some (maybe just mic/camera) permissions, if the background tab navigates then the prompt will appear on the subsequent page. The UI shows where the request came from but allowing will properly allow the permission. I don't think you can do anything particularly bad with it but it might be worth merging to 61 if it's there. I think the bug is in M61 but I haven't checked yet. Repro not requiring DCHECKs below: https://output.jsbin.com/habigajaco 1) Click the button (you might need a camera for this to work) 2) Wait 5 seconds (a new tab will open) 3) Close the tab, the page is now google.com 4) There should NOT be a permission prompt
,
Aug 23 2017
Whoops, forgot guest jsbin previews expire... Updated test link: https://output.jsbin.com/micakuhemo
,
Aug 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a2efd081df7f83a2b00627679b4936e013f0f2e commit 1a2efd081df7f83a2b00627679b4936e013f0f2e Author: Timothy Loh <timloh@chromium.org> Date: Wed Aug 23 07:44:43 2017 Fix clearing of hidden permission prompts in background tabs This patch fixes a bug where permission prompts that were hidden due to tab switching wouldn't be cleared upon navigation/destruction, leading to in some cases a DCHECK being hit or prompts persisting across navs. Bug: 757394 Change-Id: Iae960fb241ed635bd4eea93cd21cc50657aa4a9e Reviewed-on: https://chromium-review.googlesource.com/627349 Reviewed-by: Ben Wells <benwells@chromium.org> Commit-Queue: Timothy Loh <timloh@chromium.org> Cr-Commit-Position: refs/heads/master@{#496617} [modify] https://crrev.com/1a2efd081df7f83a2b00627679b4936e013f0f2e/chrome/browser/permissions/permission_request_manager.cc [modify] https://crrev.com/1a2efd081df7f83a2b00627679b4936e013f0f2e/chrome/browser/permissions/permission_request_manager_browsertest.cc
,
Aug 25 2017
Well.. I wanted to request a merge to 61 but I realise this isn't going to make it with stable being in 2 weeks, so closing as fixed.
,
Aug 25 2017
Actually I guess it doesn't hurt to try anyway. The actual code change is only one line so it should be safe to merge.
,
Aug 25 2017
This bug requires manual review: We are only 10 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 25 2017
This is P2, not a Release blocker and per comment #6 it could wait until M62. Hence, rejecting merge to M61. Please let me know if there is any concern here. Thank you. Please go/chrome-merges for updated merge request process.
,
Aug 25 2017
+ awhalley@ as per "Team-Security-UX". Pls approve the merge if you think we need it for security reason.
,
Aug 28 2017
We'll just wait until 62, I think it's fine. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by krajshree@chromium.org
, Aug 22 2017Components: UI>Browser>Permissions>Prompts
Labels: Needs-Feedback Needs-Triage-M62
1.2 MB
1.2 MB View Download