New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Oct 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 100521: Chrome: Crash Report - Stack Signature: InstantController::PrepareForCommit()-f9109...

Reported by dharani@google.com, Oct 16 2011 Project Member

Issue description

May be this changeset would have caused this crash: http://src.chromium.org/viewvc/chrome?view=rev&revision=105309

Product: Chrome
Stack Signature: InstantController::PrepareForCommit()-CC797E
New Signature Label: InstantController::PrepareForCommit()
New Signature Hash: f9109d56_759a0799_6193dba3_e35c606d_af94a524

Report link: http://go/crash/reportdetail?reportid=2d9b520df07c4d6e

Meta information:
Product Name: Chrome
Product Version: 16.0.910.0
Report ID: 2d9b520df07c4d6e
Report Time: 2011/10/16 23:00:27, Sun
Uptime: 750 sec
Cumulative Uptime: 0 sec
OS Name: Windows NT
OS Version: 6.1.7601 Service Pack 1
CPU Architecture: x86
CPU Info: GenuineIntel family 6 model 23 stepping 10
ptype:browser


Thread 0 *CRASHED* ( EXCEPTION_ACCESS_VIOLATION_READ @ 0x00000039 )

0x68d98889	 [chrome.dll	 - instant_controller.cc:235	InstantController::PrepareForCommit()
0x68cad5a4	 [chrome.dll	 - browser.cc:5151	Browser::OpenInstant(WindowOpenDisposition)
0x68ca730e	 [chrome.dll	 - browser.cc:1550	Browser::OpenCurrentURL()
0x68ca8f99	 [chrome.dll	 - browser.cc:2680	Browser::ExecuteCommandWithDisposition(int,WindowOpenDisposition)
0x68ea18aa	 [chrome.dll	 - location_bar_view.cc:837	LocationBarView::OnAutocompleteAccept(GURL const &,WindowOpenDisposition,content::PageTransition,GURL const &)
0x68f59345	 [chrome.dll	 - autocomplete_edit.cc:569	AutocompleteEditModel::OpenMatch(AutocompleteMatch const &,WindowOpenDisposition,GURL const &,unsigned int,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &)
0x68f51e03	 [chrome.dll	 - omnibox_view_win.cc:609	OmniboxViewWin::OpenMatch(AutocompleteMatch const &,WindowOpenDisposition,GURL const &,unsigned int,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &)
0x68f58f97	 [chrome.dll	 - autocomplete_edit.cc:484	AutocompleteEditModel::AcceptInput(WindowOpenDisposition,bool)
0x68f53bae	 [chrome.dll	 - omnibox_view_win.cc:1899	OmniboxViewWin::OnKeyDownOnlyWritable(wchar_t,unsigned int,unsigned int)
0x68f53162	 [chrome.dll	 - omnibox_view_win.cc:1389	OmniboxViewWin::OnKeyDown(wchar_t,unsigned int,unsigned int)
0x68f50f6c	 [chrome.dll	 - omnibox_view_win.h:189	OmniboxViewWin::ProcessWindowMessage(HWND__ *,unsigned int,unsigned int,long,long &,unsigned long)
0x68f55698	 [chrome.dll	 - atlwin.h:3081]	ATL::CWindowImplBaseT<WTL::CRichEditCtrlT<ATL::CWindow>,ATL::CWinTraits<1342177664,0> >::WindowProc(HWND__ *,unsigned int,unsigned int,long)
0x76f362f9	 [user32.dll	 + 0x000162f9]	InternalCallWinProc
0x76f36d39	 [user32.dll	 + 0x00016d39]	UserCallWinProcCheckWow
0x76f377c3	 [user32.dll	 + 0x000177c3]	DispatchMessageWorker
0x76f37889	 [user32.dll	 + 0x00017889]	DispatchMessageW
0x69489600	 [chrome.dll	 - accelerator_handler_win.cc:54	views::AcceleratorHandler::Dispatch(tagMSG const &)
0x68bf14a6	 [chrome.dll	 - message_pump_win.cc:354	base::MessagePumpForUI::ProcessMessageHelper(tagMSG const &)
0x68bf12ff	 [chrome.dll	 - message_pump_win.cc:199	base::MessagePumpForUI::DoRunLoop()
0x68bf111e	 [chrome.dll	 - message_pump_win.cc:51	base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate *,base::MessagePumpWin::Dispatcher *)
0x68bd7303	 [chrome.dll	 - message_loop.cc:439	MessageLoop::RunInternal()
0x68bd7293	 [chrome.dll	 - message_loop.cc:417	MessageLoop::RunHandler()
0x68bd7a2b	 [chrome.dll	 - message_loop.cc:830	MessageLoopForUI::Run(base::MessagePumpWin::Dispatcher *)
0x68cef49a	 [chrome.dll	 - chrome_browser_main.cc:1950	ChromeBrowserMainParts::MainMessageLoopRun()
0x69110c16	 [chrome.dll	 - browser_main.cc:437	BrowserMain(MainFunctionParams const &)
0x68bf99aa	 [chrome.dll	 - content_main.cc:252	`anonymous namespace'::RunNamedProcessTypeMain(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,MainFunctionParams const &,content::ContentMainDelegate *)
0x68bf9d40	 [chrome.dll	 - content_main.cc:442	content::ContentMain(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *,content::ContentMainDelegate *)
0x68ab295a	 [chrome.dll	 - chrome_main.cc:28	ChromeMain
0x000b1dd2	 [chrome.exe	 - client_util.cc:346	MainDllLoader::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *)
0x000b10c8	 [chrome.exe	 - chrome_exe_main_win.cc:36	wWinMain
0x0010a1e7	 [chrome.exe	 - crt0.c:263	__tmainCRTStartup
0x76383399	 [kernel32.dll	 + 0x00013399]	BaseThreadInitThunk
0x77cb9ed1	 [ntdll.dll	 + 0x00039ed1]	__RtlUserThreadStart
0x77cb9ea4	 [ntdll.dll	 + 0x00039ea4]	_RtlUserThreadStart
 

Comment 2 by bugdroid1@chromium.org, Oct 17 2011

Project Member
Summary: Chrome: Crash Report - Stack Signature: InstantController::PrepareForCommit()-f9109...
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105858

------------------------------------------------------------------------
r105858 | sreeram@chromium.org | Mon Oct 17 11:33:41 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/instant/instant_controller.cc?r1=105858&r2=105857&pathrev=105858

Crash fix.

I don't see how tab_contents_ could be NULL inside PrepareForCommit().
Even if Update() had never been called, OnAutocompleteGotFocus() should
have set a valid tab_contents_. Anyway, check for it before trying to
access the profile().

BUG= 100521 
TEST=none

Review URL: http://codereview.chromium.org/8322005
------------------------------------------------------------------------

Comment 3 by sreeram@chromium.org, Oct 17 2011

Status: Fixed

Comment 4 by bugdroid1@chromium.org, Oct 17 2011

Project Member
Labels: merge-merged-911
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105862

------------------------------------------------------------------------
r105862 | dharani@chromium.org | Mon Oct 17 11:44:48 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/911/src/chrome/browser/instant/instant_controller.cc?r1=105862&r2=105861&pathrev=105862

Merge 105858 - Crash fix.

I don't see how tab_contents_ could be NULL inside PrepareForCommit().
Even if Update() had never been called, OnAutocompleteGotFocus() should
have set a valid tab_contents_. Anyway, check for it before trying to
access the profile().

BUG= 100521 
TEST=none

Review URL: http://codereview.chromium.org/8322005

TBR=sreeram@chromium.org
Review URL: http://codereview.chromium.org/8294017
------------------------------------------------------------------------

Comment 5 by aocampo@chromium.org, Oct 17 2011

i just saw this with today's  canary 16.0.911.0 (Official Build 105769)

Comment 6 by dharani@google.com, Oct 17 2011

This is fixed in 16.0.911.2 Mac build. Yet to see confirm the fix on Windows as it is still building.

Comment 7 by dharani@google.com, Oct 18 2011

Status: Assigned
I checked too early. These crashes are still seen in 911.2 for mac and windows. Reopening the bug again.

Comment 8 by sreeram@chromium.org, Oct 18 2011

Cc: sky@chromium.org
I think I know the problem. We have a dangling TabContentsWrapper pointer. Previously, we used to be protected from accessing the pointer due to the "is_active()" check that browser.cc did before calling PrepareForCommit(). Since that check was removed in http://crrev.com/105664, we now don't have that protection anymore.

I'll send a fix shortly.

Comment 9 by sreeram@chromium.org, Oct 18 2011

Labels: ReleaseBlock-Dev ReleaseBlock-Beta

Comment 10 by bugdroid1@chromium.org, Oct 18 2011

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=106075

------------------------------------------------------------------------
r106075 | sreeram@chromium.org | Tue Oct 18 10:08:20 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/instant/instant_controller.cc?r1=106075&r2=106074&pathrev=106075

Crash fix.

We never reset |tab_contents_| to NULL anywhere (and indeed we can't,
even in ReleasePreviewContents(), because browser.cc accesses it). We
were protected from acccessing a dangling tab_contents_ before, due to
the is_active() check that preceded a call to PrepareForCommit(). That
was removed in http://crrev.com/105664 however; hence this fix.

BUG= 100521 
TEST=none

Review URL: http://codereview.chromium.org/8329020
------------------------------------------------------------------------

Comment 11 by sreeram@chromium.org, Oct 18 2011

Labels: Merge-Requested
Requesting merge into dev branch (16.0.912.0).

Comment 12 by laforge@google.com, Oct 18 2011

Labels: -Merge-Requested Merge-Approved

Comment 13 by bugdroid1@chromium.org, Oct 18 2011

Project Member
Labels: merge-merged-912
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=106084

------------------------------------------------------------------------
r106084 | sreeram@chromium.org | Tue Oct 18 11:06:37 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/912/src/chrome/browser/instant/instant_controller.cc?r1=106084&r2=106083&pathrev=106084

Merge 106075 - Crash fix.

We never reset |tab_contents_| to NULL anywhere (and indeed we can't,
even in ReleasePreviewContents(), because browser.cc accesses it). We
were protected from acccessing a dangling tab_contents_ before, due to
the is_active() check that preceded a call to PrepareForCommit(). That
was removed in http://crrev.com/105664 however; hence this fix.

BUG= 100521 
TEST=none

Review URL: http://codereview.chromium.org/8329020

TBR=sreeram@chromium.org
Review URL: http://codereview.chromium.org/8344005
------------------------------------------------------------------------

Comment 14 by sreeram@chromium.org, Oct 18 2011

Labels: -ReleaseBlock-Dev -Merge-Approved

Comment 15 by sky@chromium.org, Oct 18 2011

 Issue 100791  has been merged into this issue.

Comment 16 by eroman@chromium.org, Oct 18 2011

Cc: eroman@chromium.org
I think you misread the crash data.

The problem isn't that |InstantController::tab_contents_| itself is NULL, but rather that tab_contents_.tab_contents_ is NULL.

Specifically the crash here is happening in tab_contents_->profile(), which has inlined 4 functions into that one line.

For instance, profile() is:

Profile* TabContentsWrapper::profile() const {
  return Profile::FromBrowserContext(tab_contents()->browser_context());
}

and browser_context() is inlined from:

content::BrowserContent* TabContents::browser_context() const {
  return controller_.browser_context();
}

The crash dump I looked at shows that the we were inside of TabContentsWrapper::profile() and |tab_contents()| returned NULL. (i.e. the scoped_ptr<TabContents> held by TabContentsWrapper).

Comment 17 by sreeram@chromium.org, Oct 19 2011

Labels: -ReleaseBlock-Beta
@eroman, you may be right. I haven't been able to reproduce this crash, so I was kinda guessing earlier about the tab_contents_ being NULL thing (and it didn't really fix anything).

However, I am reasonably confident that the new patch (r106075) will fix it, because it makes sense (see CL description). You'll notice that we are no longer checking for NULL.

If anybody is able to reproduce this crash reliably, please post instructions. Thanks!

Comment 18 by sreeram@chromium.org, Oct 19 2011

Status: Fixed
This crash doesn't seem to happen in the latest canary (16.0.912.2), so I'm considering this fixed.

Comment 19 by venkataramana@chromium.org, Oct 19 2011

Cc: venkataramana@chromium.org
 Issue 100947  has been merged into this issue.

Comment 20 by thestig@chromium.org, Oct 27 2011

 Issue 101369  has been merged into this issue.

Comment 21 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 22 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-UI -Mstone-16 -Feature-Instant Cr-UI-Browser-Instant Cr-UI M-16

Comment 23 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment