Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 175275 Closing tab during tab capture leads to crash
Starred by 4 users Project Member Reported by n...@chromium.org, Feb 9, 2013 Back to list
Status: Verified
Owner: m...@chromium.org
Closed: Feb 2013
Cc: sky@chromium.org
Components:
OS: Windows
Pri: 1
Type: Bug-Regression


Sign in to add a comment
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++
 
Comment 1 by n...@chromium.org, Feb 9, 2013
Labels: -Type-Bug -Pri-2 -Regression Type-Regression Pri-1 Mstone-26
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)
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.
Comment 3 by n...@chromium.org, Feb 9, 2013
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.
Comment 4 by m...@chromium.org, Feb 11, 2013
Status: Started
Comment 5 by dharani@chromium.org, Feb 11, 2013
Labels: ReleaseBlock-Beta
Comment 6 by m...@chromium.org, Feb 11, 2013
Submitted code review for fix.
Project Member Comment 8 by bugdroid1@chromium.org, Feb 12, 2013
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
------------------------------------------------------------------------
Comment 9 by m...@chromium.org, Feb 14, 2013
Labels: Merge-Requested
Comment 10 by m...@chromium.org, Feb 14, 2013
Status: Verified
Comment 11 by dharani@chromium.org, Feb 15, 2013
Labels: -Merge-Requested Merge-Approved
Project Member Comment 12 by bugdroid1@chromium.org, Feb 16, 2013
Labels: -Merge-Approved merge-merged-1410
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
------------------------------------------------------------------------
Project Member Comment 13 by bugdroid1@chromium.org, Mar 9, 2013
Labels: -Type-Regression -Area-UI -Feature-TabCapture -Mstone-26 Type-Bug-Regression Cr-UI-Browser-TabCapture M-26 Cr-UI
Project Member Comment 14 by bugdroid1@chromium.org, Nov 11, 2013
Labels: -Cr-UI-Browser-TabCapture Cr-Internals-Media-Capture-Tab
Labels: hasTestcase
Comment 16 by laforge@google.com, Sep 23, 2014
Labels: -Cr-Internals-Media-Capture-Tab Cr-Blink-GetUserMedia-Tab
Sign in to add a comment