GTK: Draw floating bookmark bar correctly during resizes....

GTK: Draw floating bookmark bar correctly during resizes.

BUG=23907
TEST=Using ratchet + clank theme, go to NTP while bookmark bar is off so you see a floating bar. Maximize the window. It should look right.


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

[tony, 2009-10-26 20:19:21]

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

http://codereview.chromium.org/329022/diff/1/3#newcode1138
Line 1138: // We get the size-allocate signal before the tab contents does,
hence we
So the bookmarkbar gets a size allocate, then the tab contents gets a size
allocate.  After that, does the bookmark bar get another size allocate?  I don't
understand why we can't paint synchronously.

[Elliot Glaysher, 2009-10-26 20:23:49]

Does this also deal with resizing the web inspector being attached
splitscreen to the browser window?

On Mon, Oct 26, 2009 at 1:19 PM,  <tony@chromium.org> wrote:
>
> http://codereview.chromium.org/329022/diff/1/3
> File chrome/browser/gtk/bookmark_bar_gtk.cc (right):
>
> http://codereview.chromium.org/329022/diff/1/3#newcode1138
> Line 1138: // We get the size-allocate signal before the tab contents
> does, hence we
> So the bookmarkbar gets a size allocate, then the tab contents gets a
> size allocate. =A0After that, does the bookmark bar get another size
> allocate? =A0I don't understand why we can't paint synchronously.
>
> http://codereview.chromium.org/329022
>

[Evan Stade, 2009-10-26 20:56:24]

> Does this also deal with resizing the web inspector being attached
> splitscreen to the browser window?

no, apparently not. Bah. Guess I will have to connect to the
TabContentsContainer's size-allocate instead.

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

http://codereview.chromium.org/329022/diff/1/3#newcode1138
Line 1138: // We get the size-allocate signal before the tab contents does,
hence we
On 2009/10/26 20:19:21, tony wrote:
> So the bookmarkbar gets a size allocate, then the tab contents gets a size
> allocate.  After that, does the bookmark bar get another size allocate?  I
don't
> understand why we can't paint synchronously.

1. bookmark bar gets allocate
2. bookmark bar paints
3. tab contents gets allocate

[Evan Stade, 2009-10-26 23:49:39]

ok, docked devtools case fixed.

[tony, 2009-10-26 23:55:30]

http://codereview.chromium.org/329022/diff/4002/4004
File chrome/browser/gtk/bookmark_bar_gtk.cc (right):

http://codereview.chromium.org/329022/diff/4002/4004#newcode627
Line 627: g_signal_connect(widget()->parent, "size-allocate",
What happens if you drag a tab from one window to another?  Does the signal
handler get fired from the old bookmarkbargtk?

http://codereview.chromium.org/329022/diff/4002/4004#newcode1163
Line 1163: MessageLoop::current()->PostTask(FROM_HERE,
Is this still necessary since we're getting the size allocate from the tab
contents?

http://codereview.chromium.org/329022/diff/4002/4003
File chrome/browser/gtk/bookmark_bar_gtk.h (right):

http://codereview.chromium.org/329022/diff/4002/4003#newcode260
Line 260: BookmarkBarGtk* window);
Nit: want to change this to |bar| as well?

[Evan Stade, 2009-10-27 00:11:12]

http://codereview.chromium.org/329022/diff/4002/4004
File chrome/browser/gtk/bookmark_bar_gtk.cc (right):

http://codereview.chromium.org/329022/diff/4002/4004#newcode627
Line 627: g_signal_connect(widget()->parent, "size-allocate",
On 2009/10/26 23:55:30, tony wrote:
> What happens if you drag a tab from one window to another?  Does the signal
> handler get fired from the old bookmarkbargtk?

the bookmark bar and the tab contents container should always stay with the same
browser window

http://codereview.chromium.org/329022/diff/4002/4004#newcode1163
Line 1163: MessageLoop::current()->PostTask(FROM_HERE,
On 2009/10/26 23:55:30, tony wrote:
> Is this still necessary since we're getting the size allocate from the tab
> contents?

we are getting it from the tab contents container, and yes. The allocates go off
in this order:

parent
child1
child2
...

[tony, 2009-10-27 00:34:19]

LGTM

http://codereview.chromium.org/329022/diff/4002/4004
File chrome/browser/gtk/bookmark_bar_gtk.cc (right):

http://codereview.chromium.org/329022/diff/4002/4004#newcode627
Line 627: g_signal_connect(widget()->parent, "size-allocate",
On 2009/10/27 00:11:12, Evan Stade wrote:
> On 2009/10/26 23:55:30, tony wrote:
> > What happens if you drag a tab from one window to another?  Does the signal
> > handler get fired from the old bookmarkbargtk?
> 
> the bookmark bar and the tab contents container should always stay with the
same
> browser window

I see, it's the container.  The g_signal_handler_find seems unfortunate, but I
guess it's because the parent can change depending on whether this is floating
or not?

[Evan Stade, 2009-10-27 00:37:47]

http://codereview.chromium.org/329022/diff/4002/4004
File chrome/browser/gtk/bookmark_bar_gtk.cc (right):

http://codereview.chromium.org/329022/diff/4002/4004#newcode627
Line 627: g_signal_connect(widget()->parent, "size-allocate",
it's because we don't want to connect mroe than once. We could also track that
with a boolean, I just don't like member var bloat.

[tony, 2009-10-27 00:46:21]

On 2009/10/27 00:37:47, Evan Stade wrote:
> http://codereview.chromium.org/329022/diff/4002/4004
> File chrome/browser/gtk/bookmark_bar_gtk.cc (right):
> 
> http://codereview.chromium.org/329022/diff/4002/4004#newcode627
> Line 627: g_signal_connect(widget()->parent, "size-allocate",
> it's because we don't want to connect mroe than once. We could also track that
> with a boolean, I just don't like member var bloat.

Right, but why can't we connect once during init?

[Evan Stade, 2009-10-27 01:06:01]

On Mon, Oct 26, 2009 at 5:46 PM,  <tony@chromium.org> wrote:
> On 2009/10/27 00:37:47, Evan Stade wrote:
>>
>> http://codereview.chromium.org/329022/diff/4002/4004
>> File chrome/browser/gtk/bookmark_bar_gtk.cc (right):
>
>> http://codereview.chromium.org/329022/diff/4002/4004#newcode627
>> Line 627: g_signal_connect(widget()->parent, "size-allocate",
>> it's because we don't want to connect mroe than once. We could also track
>> that
>> with a boolean, I just don't like member var bloat.
>
> Right, but why can't we connect once during init?

because it's not parented until after init. And we don't connect to
parent-set because, as you noted, the parent is different for
floating/docked.

>
> http://codereview.chromium.org/329022
>