Omnibox: Possibly Fix Rare OpenMatch Crash

Omnibox: Possibly Fix Rare OpenMatch Crash

The associated bug seems to imply a crash on this line
ClassifyPage(controller_->GetWebContents()->GetURL())
in OmniboxEditModel::OpenMatch().

This change corrects the part of the test that may fail
(if there's no active tab for some reason).
I think controller_ is always valid (the rest of the edit
model code use it without testing it for non-NULL).
But I see at least two places in OmniboxEditModel that
test for the non-NULLness of controller_->GetWebContents().
Hence, I think it might be able to be NULL, so this
change adds a test for it here too.

TEST=new code compiles
BUG=142931
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182244

[Mark P, 2013-02-12 22:21:52]

Peter,

What do you think about this?

--mark

[Peter Kasting, 2013-02-12 22:25:51]

If we're still getting reports for this crash, then I'd say let's check in and
see if it makes the reports stop.

I'd also like to know if anywhere else assumes a non-NULL WebContents.  If this
makes the crash reports stop, we should fix any other such places too.  It would
be nice to track down why this can be NULL, as intuitively I'd expect it not to
be.  Perhaps some sort of timing issue during browser startup/shutdown?

If this _doesn't_ fix the crashes, I'd back it out and probably remove the other
similar NULL-checks too.

This is all a very long way of saying that I want us to have NULL checks iff we
need them, and it seems like right now we don't know whether we need them.

[Peter Kasting, 2013-02-12 22:26:58]

Oh, LGTM, since the tools probably want that :)

[Mark P, 2013-02-12 22:31:22]

On 2013/02/12 22:25:51, Peter Kasting wrote:
> If we're still getting reports for this crash, then I'd say let's check in and
> see if it makes the reports stop.
> 
> I'd also like to know if anywhere else assumes a non-NULL WebContents.  If
this
> makes the crash reports stop, we should fix any other such places too.  It
would
> be nice to track down why this can be NULL, as intuitively I'd expect it not
to
> be.  Perhaps some sort of timing issue during browser startup/shutdown?
> 
> If this _doesn't_ fix the crashes, I'd back it out and probably remove the
other
> similar NULL-checks too.
> 
> This is all a very long way of saying that I want us to have NULL checks iff
we
> need them, and it seems like right now we don't know whether we need them.

Acknowledged.

--mark

[commit-bot: I haz the power, 2013-02-12 22:32:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/12250014/1

[commit-bot: I haz the power, 2013-02-12 23:52:02]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-02-12 23:56:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/12250014/1

[commit-bot: I haz the power, 2013-02-13 04:00:04]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-02-13 15:34:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/12250014/1

[commit-bot: I haz the power, 2013-02-13 16:39:44]

Change committed as 182244