crash from Mac: http://crash/reportdetail?reportid=5274edf468e0bd55

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
------------------------------------------------------------------------

(No comment was entered for this change.)

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
------------------------------------------------------------------------

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

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

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

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.

(No comment was entered for this change.)

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
------------------------------------------------------------------------

Requesting merge into dev branch (16.0.912.0).

(No comment was entered for this change.)

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
------------------------------------------------------------------------

(No comment was entered for this change.)

 Issue 100791  has been merged into this issue.

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).

@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!

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

 Issue 100947  has been merged into this issue.

 Issue 101369  has been merged into this issue.