My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 175275: Closing tab during tab capture leads to crash
4 people starred this issue and may be notified of changes. Back to list
 
Project Member Reported by nick@chromium.org, Feb 8, 2013
Here's a tabcapture-related crash that repros 100% for me:

 1. Open news.google.com
 2. Start tab capture using miu's self-mirror extension.
 3. Close the tab being captured.

 	base::internal::LockImpl::Unlock() Line 33	C++
 	ThumbnailTabHelper::WidgetHidden(widget) Line 190	C++
 	ThumbnailTabHelper::Observe(type, source, details) Line 153	C++
 	content::NotificationServiceImpl::Notify(type, source, details) Line 132	C++
 	content::RenderWidgetHostImpl::WasHidden() Line 436	C++
 	content::RenderWidgetHostViewWin::WasHidden() Line 470	C++
 	content::WebContentsImpl::WasHidden() Line 1151	C++
>	content::WebContentsImpl::DecrementCapturerCount() Line 1070	C++
 	content::`anonymous namespace'::BackingStoreCopier::WebContentsDestroyed(web_contents) Line 319	C++
 	content::WebContentsObserver::WebContentsImplDestroyed() Line 66	C++
 	content::WebContentsImpl::~WebContentsImpl() Line 370	C++
 	content::WebContentsImpl::`vector deleting destructor'()	C++
 	TabStripModel::InternalCloseTab(contents, index, create_historical_tabs) Line 1084	C++
 	TabStripModel::InternalCloseTabs(indices, close_types) Line 1066	C++
 	TabStripModel::CloseWebContentsAt(index, close_types) Line 353	C++
 	chrome::CloseWebContents(browser, contents, add_to_history) Line 116	C++
 	Browser::CloseContents(source) Line 1331	C++
Feb 8, 2013
#1 nick@chromium.org
The crash happens because web_contents() is NULL in ThumbnailTabHelper::WidgetHidden.

Starting with miu because of his recent work here ( content::WebContentsImpl::DecrementCapturerCount is on the stack)
Labels: -Type-Bug -Pri-2 -Regression Type-Regression Pri-1 Mstone-26
Feb 8, 2013
#2 justinlin@chromium.org
Is it specific to that page? I just tried the same page on mac, and it's basically unusably slow once you turn tabCapture on.
Feb 8, 2013
#3 nick@chromium.org
Not specific to that page. Happens on every site I tried (but not about:blank or the new tab page; those aren't thumbnailed). By 'crash' I mean I'm seeing a null pointer dereference. This is a debug build, on Windows.
Feb 11, 2013
#4 miu@chromium.org
(No comment was entered for this change.)
Status: Started
Feb 11, 2013
#5 dhar...@chromium.org
(No comment was entered for this change.)
Labels: ReleaseBlock-Beta
Feb 11, 2013
#6 miu@chromium.org
Submitted code review for fix.
Feb 12, 2013
#8 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=181876

------------------------------------------------------------------------
r181876 | miu@chromium.org | 2013-02-12T06:15:25.792543Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?r1=181876&r2=181875&pathrev=181876
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/web_contents/web_contents_impl.cc?r1=181876&r2=181875&pathrev=181876

Fix crash when DecrementCapturerCount() makes a delayed WasHidden() call during WebContentsImpl destruction.

Two parts:

1. WebContentsImpl::DecrementCapturerCount() will restore the true "WasHidden" status of the RenderWidgetHost once the counter reaches zero.  However, DecrementCapturerCount() can be called [indirectly] from the WebContentsImpl destructor.  In this case, it should not attempt to change/notify anything.

2. It's possible for ThumbnailTabHelper::WidgetHidden() to call ThumbnailTabHelper::UpdateThumbnailIfNecessary() with a NULL pointer.  Added a NULL check.

BUG=175275,130097


Review URL: https://chromiumcodereview.appspot.com/12209104
------------------------------------------------------------------------
Feb 13, 2013
#9 miu@chromium.org
(No comment was entered for this change.)
Labels: Merge-Requested
Feb 14, 2013
#10 miu@chromium.org
(No comment was entered for this change.)
Status: Verified
Feb 15, 2013
#11 dhar...@chromium.org
(No comment was entered for this change.)
Labels: -Merge-Requested Merge-Approved
Feb 15, 2013
#12 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=182885

------------------------------------------------------------------------
r182885 | miu@chromium.org | 2013-02-16T01:44:01.507354Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1410/src/chrome/browser/thumbnails/thumbnail_tab_helper.cc?r1=182885&r2=182884&pathrev=182885
   M http://src.chromium.org/viewvc/chrome/branches/1410/src/content/browser/web_contents/web_contents_impl.cc?r1=182885&r2=182884&pathrev=182885

Merge 181876
> Fix crash when DecrementCapturerCount() makes a delayed WasHidden() call during WebContentsImpl destruction.
> 
> Two parts:
> 
> 1. WebContentsImpl::DecrementCapturerCount() will restore the true "WasHidden" status of the RenderWidgetHost once the counter reaches zero.  However, DecrementCapturerCount() can be called [indirectly] from the WebContentsImpl destructor.  In this case, it should not attempt to change/notify anything.
> 
> 2. It's possible for ThumbnailTabHelper::WidgetHidden() to call ThumbnailTabHelper::UpdateThumbnailIfNecessary() with a NULL pointer.  Added a NULL check.
> 
> BUG=175275,130097
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/12209104

BUG=175275,130097
TBR=miu@chromium.org
Review URL: https://codereview.chromium.org/12277022
------------------------------------------------------------------------
Labels: -Merge-Approved merge-merged-1410
Mar 9, 2013
#13 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Type-Regression -Area-UI -Feature-TabCapture -Mstone-26 Type-Bug-Regression Cr-UI-Browser-TabCapture M-26 Cr-UI
Nov 11, 2013
#14 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Cr-UI-Browser-TabCapture Cr-Internals-Media-Capture-Tab
Jun 20, 2014
#15 tkonch...@chromium.org
(No comment was entered for this change.)
Labels: hasTestcase
Sep 23 (6 days ago)
#16 lafo...@google.com
(No comment was entered for this change.)
Labels: -Cr-Internals-Media-Capture-Tab Cr-Blink-GetUserMedia-Tab
Sign in to add a comment

Powered by Google Project Hosting