Force sizing of a new tab as soon as TabContents is created....

Force sizing of a new tab as soon as TabContents is created.

BUG=619
BUG=15960
TEST=none



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

[brettw, 2009-06-19 17:39:52]

http://codereview.chromium.org/126290/diff/1/3
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/126290/diff/1/3#newcode1460
Line 1460: contents->view()->SizeContents(window()->GetNormalBounds().size());
I'm not sure GetNormalBounds will give you the correct size, I think it will
give you the size of the browser window outside, and you really want what size
it would be inside the window.

I recommend talking to Ben to figure out the best way to compute this.

[idanan, 2009-06-29 21:09:19]

Turns out this is the solution to
http://code.google.com/p/chromium/issues/detail?id=13526 as well. The problem is
that even though gotoAnchor() calls layout() in order to get the correct layout
before setting scrollbars, layout does not have the frame size available. I've
been debugging this all day and could not find any solution to do this on the
Webkit side alone. The OnResize event from the browser seems to be the only
place to get the top-level frame size all the way to webkit. I've merged the
duplicate issue 12874 with 13526 which stems from layout not having a valid size
for background tabs.

LGTM

[brettw, 2009-07-27 23:06:11]

Just some comment suggestions.

http://codereview.chromium.org/126290/diff/3001/4002
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/126290/diff/3001/4002#newcode1471
Line 1471: if (TabContents* old_contents =
tabstrip_model_.GetSelectedTabContents()) {
You should follow the surrounding code for whether to use {} for single-line
conditionals. This file doesn't do it (probably most of our code is like this)
so I wouldn't add them here, either.

Can you also be more specific instead of just saying "series of bugs". Perhaps
mention that some web pages perform window-size-specific operations in their
onload handlers which will be incorrect if the size is 0x0, in addition to other
similar effects.

http://codereview.chromium.org/126290/diff/3001/4001
File chrome/browser/views/tab_contents/tab_contents_view_win.cc (right):

http://codereview.chromium.org/126290/diff/3001/4001#newcode633
Line 633: UINT swp_flags = SWP_NOSENDCHANGING | SWP_NOOWNERZORDER |
SWP_NOCOPYBITS |
Can you add a quick comment about why this needs to be here?

[Ben Goodger (Google), 2009-07-27 23:08:13]

http://codereview.chromium.org/126290/diff/3001/4001
File chrome/browser/views/tab_contents/tab_contents_view_win.cc (right):

http://codereview.chromium.org/126290/diff/3001/4001#newcode633
Line 633: UINT swp_flags = SWP_NOSENDCHANGING | SWP_NOOWNERZORDER |
SWP_NOCOPYBITS |
On 2009/07/27 23:06:11, brettw wrote:
> Can you add a quick comment about why this needs to be here?

some of these flags are very odd... e.g. SWP_NOSENDCHANGING etc. it'd be nice to
know why you have specified each of these SWP flags.

[Nate Chapin, 2009-07-28 16:12:44]

Ok, new version uploaded with fewer SetWindowPos flags, as well as a more
verbose (and hopefully more clear) comment.

[brettw, 2009-07-28 16:19:52]

LGTM. I'm OK as long as Ben's happy with the SetWindowPos flags.