Fix crash when using Instant.

Fix crash when using Instant.

Steps to reproduce (on Mac OS, with Chrome Instant enabled):
1. On the NTP, type a partial query (without hitting Enter).
2. After the results preview appears, hit Alt-Enter to open it in a new
   foreground tab.
3. Without doing anything else, close the tab. The previous NTP tab is
   now the active tab again.
4. Type another partial query.
--> Previously, this would crash. This CL fixes the crash.

The root cause is that, after step 2, previewContents_ still referred to
the now committed tab. So, when that tab disappears in step 3,
previewContents_ is left dangling, causing the crash in step 4.

The bug was caused by my earlier CL (http://crrev.com/158613), which
removed the HideInstant() call that we made after every Instant commit.
HideInstant() nulls out previewContents_, which is why this crash
surfaced after that call was removed. Instead of just putting back
HideInstant(), I think the right thing to do is to adjust
previewContents_, in much the same way that
MakePreviewContentsActiveContents() does in the views UI code.

BUG=153902
R=sky@chromium.org
TEST=See steps above.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160433

[sreeram, 2012-10-04 21:11:13]

Please review.

[sky, 2012-10-04 21:49:53]

I'm not a good reviewer for cocoa. Maybe Avi.

[Avi (use Gerrit), 2012-10-04 21:54:44]

The cocoa code lg but I don't know much about instant. Who wrote the Mac instant
code?

[sreeram, 2012-10-04 22:35:05]

+rohitrao, please review.

[rohitrao (ping after 24h), 2012-10-04 22:47:26]

How does the views code handle this case, since "new foreground tab" doesn't go
through any of the TabReplacedAt() codepaths?

[rohitrao (ping after 24h), 2012-10-04 22:50:04]

LGTM for the crash fix.

Would like us to dig into this more to find out why mac is crashing but windows
is not.

[sreeram, 2012-10-04 22:53:45]

On 2012/10/04 22:50:04, rohitrao wrote:
> LGTM for the crash fix.

Thanks. @avi, can I get an OWNERS lgtm?

> Would like us to dig into this more to find out why mac is crashing but
windows
> is not.

Yup, I'm in the process of doing that. I am digging into the various code paths,
on both views and gtk. Will fix issues or file bugs accordingly.

[sreeram, 2012-10-04 23:26:59]

On 2012/10/04 22:47:26, rohitrao wrote:
> How does the views code handle this case, since "new foreground tab" doesn't
go
> through any of the TabReplacedAt() codepaths?

So, the answer turns out to be: It doesn't (handle this case). the
ContentsContainer::preview_web_contents_ is also left dangling, but since we
don't actually use it for anything (for example, we don't try to send it a
WasHidden() message), there's no crash.

I'll go look at tightening this, and other code paths as well, in a separate CL.

[Avi (use Gerrit), 2012-10-05 00:44:50]

owner lgtm stamp

[commit-bot: I haz the power, 2012-10-05 01:01:07]

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

[commit-bot: I haz the power, 2012-10-05 01:30:05]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-05 16:45:02]

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

[commit-bot: I haz the power, 2012-10-05 18:46:21]

Change committed as 160433