GTK: allow inspecting of extension popups.

GTK: allow inspecting of extension popups.

BUG=24477
TEST=manual


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

[Daniel Erat, 2010-03-23 15:28:55]

Mostly looks good to me, but adding a few other people (summary: when we're
inspecting an extension popup window, use a non-override-redirect window and
don't grab the pointer and keyboard, so the user can still interact with the dev
tools window).

http://codereview.chromium.org/1170001/diff/1/8
File chrome/browser/gtk/bookmark_bubble_gtk.cc (right):

http://codereview.chromium.org/1170001/diff/1/8#newcode232
chrome/browser/gtk/bookmark_bubble_gtk.cc:232: true,
this method is kinda getting out of control; can you add comments after these
two 'true' params saying what they are?

http://codereview.chromium.org/1170001/diff/1/11
File chrome/browser/gtk/info_bubble_gtk.cc (right):

http://codereview.chromium.org/1170001/diff/1/11#newcode112
chrome/browser/gtk/info_bubble_gtk.cc:112: window_ = gtk_window_new(grab_input ?
GTK_WINDOW_POPUP : GTK_WINDOW_TOPLEVEL);
add a comment noting that the GTK_WINDOW_TOPLEVEL case can cause placement
issues with various window managers but is necessary in the non-grab case so
that the window can be focused so it can receive input

http://codereview.chromium.org/1170001/diff/1/12
File chrome/browser/gtk/info_bubble_gtk.h (right):

http://codereview.chromium.org/1170001/diff/1/12#newcode238
chrome/browser/gtk/info_bubble_gtk.h:238: // extension popups with dev tools.
Note that this it's not just input to Chrome that it's getting; it's all
keyboard and mouse input to the X server.

[Evan Martin, 2010-03-23 15:49:36]

looks good to me too, though I don't fully understand it

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

http://codereview.chromium.org/1170001/diff/1/3#newcode189
chrome/browser/gtk/browser_actions_toolbar_gtk.cc:189: return true;
should this be false?

http://codereview.chromium.org/1170001/diff/1/4
File chrome/browser/gtk/extension_popup_gtk.h (right):

http://codereview.chromium.org/1170001/diff/1/4#newcode52
chrome/browser/gtk/extension_popup_gtk.h:52: bool being_inspected() {
const modifier on this (?)

[Evan Stade, 2010-03-23 17:21:59]

http://codereview.chromium.org/1170001/diff/1/8
File chrome/browser/gtk/bookmark_bubble_gtk.cc (right):

http://codereview.chromium.org/1170001/diff/1/8#newcode232
chrome/browser/gtk/bookmark_bubble_gtk.cc:232: true,
On 2010/03/23 15:28:55, Daniel Erat wrote:
> this method is kinda getting out of control; can you add comments after these
> two 'true' params saying what they are?

Done.

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

http://codereview.chromium.org/1170001/diff/1/3#newcode189
chrome/browser/gtk/browser_actions_toolbar_gtk.cc:189: return true;
On 2010/03/23 15:49:36, Evan Martin wrote:
> should this be false?

no, it should be true because to match the current behavior we have to bail out
in this case rather than continue and try to activate the action. I was
struggling with the wording in the comment above to convey this.

http://codereview.chromium.org/1170001/diff/1/4
File chrome/browser/gtk/extension_popup_gtk.h (right):

http://codereview.chromium.org/1170001/diff/1/4#newcode52
chrome/browser/gtk/extension_popup_gtk.h:52: bool being_inspected() {
On 2010/03/23 15:49:36, Evan Martin wrote:
> const modifier on this (?)

Done.

http://codereview.chromium.org/1170001/diff/1/11
File chrome/browser/gtk/info_bubble_gtk.cc (right):

http://codereview.chromium.org/1170001/diff/1/11#newcode112
chrome/browser/gtk/info_bubble_gtk.cc:112: window_ = gtk_window_new(grab_input ?
GTK_WINDOW_POPUP : GTK_WINDOW_TOPLEVEL);
On 2010/03/23 15:28:55, Daniel Erat wrote:
> add a comment noting that the GTK_WINDOW_TOPLEVEL case can cause placement
> issues with various window managers but is necessary in the non-grab case so
> that the window can be focused so it can receive input

Done.

http://codereview.chromium.org/1170001/diff/1/12
File chrome/browser/gtk/info_bubble_gtk.h (right):

http://codereview.chromium.org/1170001/diff/1/12#newcode238
chrome/browser/gtk/info_bubble_gtk.h:238: // extension popups with dev tools.
On 2010/03/23 15:28:55, Daniel Erat wrote:
> Note that this it's not just input to Chrome that it's getting; it's all
> keyboard and mouse input to the X server.

Done.