Fix for the flashing of the front tab....

Fix for the flashing of the front tab.
- The background tabs think they are visible causing paints to
  be executed. These steal the backing store from the visible tab

Please read the bug for more info.

BUG=20831
TEST= see bug for steps



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

[cpu_(ooo_6.6-7.5), 2009-09-03 19:01:38]

Please review the bug for all the info. 

I have tried several other approaches and failed but I am open to ideas for a
better approach.

[Evan Martin, 2009-09-03 19:02:33]

Brett may have ideas.

[Amanda Walker, 2009-09-03 19:06:36]

We're definitely getting paint requests for background tabs.  I added code
(http://codereview.chromium.org/175027) to handle that case on the Mac, since I
thought it was related to my mac backing store speedup, but not painting until
we are exposed seems better.

[Amanda Walker, 2009-09-04 03:01:29]

lgtm if brettw likes it.

[brettw, 2009-09-04 03:37:48]

Carlos and I already talked about this for a while, and this seems fine.

Brett

On Thu, Sep 3, 2009 at 12:02 PM, <evan@chromium.org> wrote:
>
> Brett may have ideas.
>
> http://codereview.chromium.org/199010
>

[darin (slow to review), 2009-09-04 06:25:57]

http://codereview.chromium.org/199010/diff/1/2
File chrome/browser/tabs/tab_strip_model.cc (right):

http://codereview.chromium.org/199010/diff/1/2#newcode406
Line 406: contents->HideContents();
shouldn't we hide the contents before we size them?  SizeContents sends a resize
IPC to the renderer, which could kick off a paint.

why not create tabs hidden first?

[cpu_(ooo_6.6-7.5), 2009-09-04 18:46:51]

> why not create tabs hidden first?

I looked into it but it seems difficult. Here is what happens today in terms of
the call stacks:

We have two messages, in the first message we create the tab:

1-DispatchToMethod<RenderWidgetHelper,..>(..)
2-RenderWidgetHelper::OnCreateWindowOnUI(int opener_id=1, int route_id=3, ..)
3-RenderViewHost::CreateNewWindow(int route_id=3, ..)
4-TabContentsView::CreateNewWindow(int route_id=3, ..)
5-RenderViewHostDelegateViewHelper::CreateNewWindow(int route_id=3,..)
6-TabContents::TabContents(Profile * profile=0x0056fc00, ..)

Then, later it comes the message to show the tab, which does much more than just
show it:

1-DispatchToMethod<RenderViewHost,void ..>(..)
2-RenderViewHost::OnMsgShowView(int route_id=3, WindowOpenDisposition
disposition=NEW_BACKGROUND_TAB, ..)
3-TabContentsView::ShowCreatedWindow(int route_id=3, disposition,..)
4-TabContents::AddNewContents(TabContents * new_contents=0x08b11200,..)
5-Browser::AddNewContents(TabContents * ..)
6-TabStripModel::AddTabContents(TabContents * ..)

So in the first message we don't know the disposition, so I chose to hide the
window in the second call when we know is a background tab.

I choose to put the hide the closest to the resize (#6) but it might be possible
to move it up to #3 or #2

What are your thoughts?

[cpu_(ooo_6.6-7.5), 2009-09-04 21:02:30]

Actually, even changing the order of

contents->view()->SizeContents(..)
contents->HideContents();

In the current CL causes bad effects, because I hit the early return in this
code:

void RenderWidgetHostViewWin::SetSize(const gfx::Size& size) {
  if (is_hidden_)
    return;

  SetWindowPos(NULL, 0, 0, w, h, flags);
  render_widget_host_->WasResized();      <---- send message to renderer
  EnsureTooltip();
}

We would need to move up both calls or have a special version of SetSize() that
does not care about is_hidden_

[darin (slow to review), 2009-09-09 05:08:04]

> We would need to move up both calls or have a special version of SetSize()
that
> does not care about is_hidden_

Ah, I see.  I noticed that the RenderWidget will force itself to be non hidden
if asked to resize while it is hidden.  So, we definitely can't hide first and
resize later.

Given the is_hidden_ check in RenderWidgetHostViewWin's DidPaintRect method, I
believe that any extra paint we might get from the RenderWidget will just be
dropped on the floor.  The only downside to resizing first and hiding later is
that we pay the cost of a layout and paint in the renderer, but addressing that
will be a larger change.

That's a long winded way of saying LGTM.  Sorry for the delay!