Speculative fix for instant overlay crash

Speculative fix for instant overlay crash

We're getting some crashes in
-[OverlayableContentsController setOverlay:...].

I wasn't able to reproduce the crash. My guess is that we're somehow
referencing a stale WebContents pointer.

This is a speculative fix that clears the WebContents reference when
the object is destroyed. I'll revert this change if we continue to
see crashes.

BUG=230880
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194440

[kuan, 2013-04-15 15:05:53]

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
File chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm
(right):

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm:9:
#include "chrome/browser/ui/browser_instant_controller.h"
nit: is this needed for ur changes?

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm:81:
content::Source<content::WebContents>(model.GetOverlayContents()));
note that model.GetOverlayContents() is the same when overlay:
1) starts at partial height
2) expands to full-height
3) probably the same when hidden
so u'll be re-registering for the same web contents.  if that's not too
expensive, it's ok.
i'm also not sure if u need to re-register for (3), since this registration will
happen after u've setOverlay to NULL.

[dhollowa, 2013-04-15 15:06:06]

lgtm

[sail, 2013-04-15 15:56:57]

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
File chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm
(right):

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm:9:
#include "chrome/browser/ui/browser_instant_controller.h"
On 2013/04/15 15:05:53, kuan wrote:
> nit: is this needed for ur changes?

Done. Removed.

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm:81:
content::Source<content::WebContents>(model.GetOverlayContents()));
On 2013/04/15 15:05:53, kuan wrote:
> note that model.GetOverlayContents() is the same when overlay:
> 1) starts at partial height
> 2) expands to full-height
> 3) probably the same when hidden
> so u'll be re-registering for the same web contents.  if that's not too
> expensive, it's ok.
> i'm also not sure if u need to re-register for (3), since this registration
will
> happen after u've setOverlay to NULL.

Registration is super cheap so I don't think it's worth checking here.

[kuan, 2013-04-15 15:58:36]

lgtm.

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
File chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm
(right):

https://codereview.chromium.org/14242004/diff/3001/chrome/browser/ui/cocoa/ta...
chrome/browser/ui/cocoa/tab_contents/instant_overlay_controller_mac.mm:81:
content::Source<content::WebContents>(model.GetOverlayContents()));
On 2013/04/15 15:56:57, sail wrote:
> On 2013/04/15 15:05:53, kuan wrote:
> > note that model.GetOverlayContents() is the same when overlay:
> > 1) starts at partial height
> > 2) expands to full-height
> > 3) probably the same when hidden
> > so u'll be re-registering for the same web contents.  if that's not too
> > expensive, it's ok.
> > i'm also not sure if u need to re-register for (3), since this registration
> will
> > happen after u've setOverlay to NULL.
> 
> Registration is super cheap so I don't think it's worth checking here.

ok.

[sail, 2013-04-16 20:45:16]

Committed patchset #3 manually as r194440 (presubmit successful).