GTK version of fullscreen API exit bubble.

GTK version of fullscreen API exit bubble.

This change depends on http://codereview.chromium.org/7740044/

BUG=73923
TEST=

[tony, 2011-10-14 17:56:05]

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (left):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:1441:
fullscreen_exit_bubble_.reset(new FullscreenExitBubbleGtk(
Why is this code being removed?  This is to handle the user going fullscreen via
their window manger.  For example, if you go to System -> Preferences ->
Keyboard Shortcuts, you can assign a keyboard shortcut to "Toggle fullscreen
mode".

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:21: } // namespace
Nit: 2 spaces before //.  Why didn't the presubmit checks catch this?

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:48: link_ =
gtk_chrome_link_button_new(exit_text_utf8.c_str());
Do you have a screenshot of what this looks like and a link to the mocks?  I'm
surprised you don't need to specify the color (don't links default to blue?).

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:74:
gtk_util::ActAsRoundedWindow(bubble, kFrameColor, 3,
Is this supposed to be a fixed frame color or should we be using the user's
theme colors?

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:91: GtkRequisition req;
I would move the size comment on lines 54-56 here.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:101:
gtk_widget_set_size_request(button_link_hbox, width, height);
Should the contents of the hbox be horizontally centered?

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.h (right):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.h:62: GtkWidget*
button_link_hbox_;
This variable isn't used.

[jeremya, 2011-10-15 01:44:55]

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/browser_w...
File chrome/browser/ui/gtk/browser_window_gtk.cc (left):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/browser_w...
chrome/browser/ui/gtk/browser_window_gtk.cc:1441:
fullscreen_exit_bubble_.reset(new FullscreenExitBubbleGtk(
On 2011/10/14 17:56:05, tony wrote:
> Why is this code being removed?  This is to handle the user going fullscreen
via
> their window manger.  For example, if you go to System -> Preferences ->
> Keyboard Shortcuts, you can assign a keyboard shortcut to "Toggle fullscreen
> mode".

Ah, oops -- I didn't know that :)

Fixed.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:21: } // namespace
On 2011/10/14 17:56:05, tony wrote:
> Nit: 2 spaces before //.  Why didn't the presubmit checks catch this?

Done. Not sure why the presubmit didn't pick up on it.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:48: link_ =
gtk_chrome_link_button_new(exit_text_utf8.c_str());
On 2011/10/14 17:56:05, tony wrote:
> Do you have a screenshot of what this looks like and a link to the mocks?  I'm
> surprised you don't need to specify the color (don't links default to blue?).

The mock (http://www.corp.google.com/~glen/chrome/spec/146_fullscreen/1/)
doesn't even have the "Press F11 to exit" being a link. The link is blue on the
other platforms, but it wouldn't be hard to change. I'll ask Glen.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:74:
gtk_util::ActAsRoundedWindow(bubble, kFrameColor, 3,
On 2011/10/14 17:56:05, tony wrote:
> Is this supposed to be a fixed frame color or should we be using the user's
> theme colors?

BubbleGtk seems to use a fixed frame color.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:91: GtkRequisition req;
On 2011/10/14 17:56:05, tony wrote:
> I would move the size comment on lines 54-56 here.

Done.

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:101:
gtk_widget_set_size_request(button_link_hbox, width, height);
On 2011/10/14 17:56:05, tony wrote:
> Should the contents of the hbox be horizontally centered?

You mean vertically..?

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.h (right):

http://codereview.chromium.org/8283028/diff/1/chrome/browser/ui/gtk/fullscree...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.h:62: GtkWidget*
button_link_hbox_;
On 2011/10/14 17:56:05, tony wrote:
> This variable isn't used.

Done.

[tony, 2011-10-15 02:26:49]

LGTM

http://codereview.chromium.org/8283028/diff/4001/chrome/browser/ui/gtk/fullsc...
File chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc (right):

http://codereview.chromium.org/8283028/diff/4001/chrome/browser/ui/gtk/fullsc...
chrome/browser/ui/gtk/fullscreen_exit_bubble_gtk.cc:71:
gtk_util::ActAsRoundedWindow(bubble, kFrameColor, 3,
It seems like it's a bug that we don't use the theme color here.  What are we
doing on other platforms? Can be fixed in a follow up.