Always call WasShown() before trying to paint.

Always call WasShown() before trying to paint.

See the bug link for details on the crash that this CL is attempting to fix.
Although I haven't been able to reproduce the crash locally, my intuition is
that, when we call window->ShowInstant(), sometimes it tries to paint, and
since the RenderWidgetHostView hasn't yet been told that it's being shown, it
fails the DCHECK(). So, reorder the calls so that WasShown() is always called
before any change to the Instant overlay layout (ShowInstant/HideInstant).

BUG=143207
R=sky@chromium.org
TEST=Watch try bots/waterfall; Instant shouldn't fire this DCHECK().

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

[sreeram, 2012-08-17 19:47:19]

Please review.

[sreeram, 2012-08-20 15:46:06]

@sky, please review.

[sky, 2012-08-20 16:29:31]

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:81:
preview->web_contents()->WasShown();
Any reason you're not putting this in InstantController?

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:96:
chrome::GetActiveWebContents(browser_)->WasShown();
How come you're moving this?

[sreeram_google.com, 2012-08-20 16:36:44]

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:81:
preview->web_contents()->WasShown();
On 2012/08/20 16:29:31, sky wrote:
> Any reason you're not putting this in InstantController?

No particular reason. Historically, this code has been here, in Browser land. It
seems reasonable that the Browser should operate on the active tab.
InstantController only uses the active tab for things like pointer comparison,
extracting profile and passing on to other methods. It doesn't directly
"operate" on it. Let me know if you'd like to change that.

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:96:
chrome::GetActiveWebContents(browser_)->WasShown();
On 2012/08/20 16:29:31, sky wrote:
> How come you're moving this?

Same reason as before. When we call browser_->window()->HideInstant(), it may
try to paint the active tab and ask for its backing store, which might trigger
the DCHECK() since it's hidden. Hence, make the active tab visible first.

[sreeram, 2012-08-20 16:46:25]

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/10828374/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:81:
preview->web_contents()->WasShown();
On 2012/08/20 16:36:45, sreeram_google.com wrote:
> On 2012/08/20 16:29:31, sky wrote:
> > Any reason you're not putting this in InstantController?
> 
> No particular reason. Historically, this code has been here, in Browser land.
It
> seems reasonable that the Browser should operate on the active tab.
> InstantController only uses the active tab for things like pointer comparison,
> extracting profile and passing on to other methods. It doesn't directly
> "operate" on it. Let me know if you'd like to change that.

I forgot to add that, since the Browser operates on the active tab, it seems
reasonable to have it operate on the preview as well (since the Hide() on one
and Show() on the other go hand in hand).

[sky, 2012-08-20 17:16:24]

LGTM