Skip to content

Commit

Permalink
Bring back Inspect Popup on gtk
Browse files Browse the repository at this point in the history 
This rolls back an earlier change, just for gtk, as bringing up the
dev tools on a live popup kills the keyboard.

BUG=132957
TEST=Test popups work; test inspept popup command is visible on gtk.


Review URL: https://chromiumcodereview.appspot.com/10544172

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143095 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
benwells@chromium.org committed Jun 20, 2012
1 parent 6a657d2 commit c82526d
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 33 deletions.
6 changes: 6 additions & 0 deletions chrome/app/generated_resources.grd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4676,6 +4676,9 @@ Update checks have repeatedly failed for the extension "<ph name="EXTENSION_NAME
<message name="IDS_MANAGE_EXTENSIONS" desc="The 'Manage Extensions...' text in the context menu for when right-clicking on extension icons (sentence case).">
Manage extensions...
</message>
<message name="IDS_EXTENSION_ACTION_INSPECT_POPUP" desc="The text for the right-click menu of page and browser actions which shows the popup and opens the developer tools (sentence case).">
Inspect popup
</message>
</if>
<if expr="pp_ifdef('use_titlecase')">
<message name="IDS_EXTENSIONS_OPTIONS_MENU_ITEM" desc="The text for the options menu item in context menus (title case).">
Expand All @@ -4690,6 +4693,9 @@ Update checks have repeatedly failed for the extension "<ph name="EXTENSION_NAME
<message name="IDS_MANAGE_EXTENSIONS" desc="The 'Manage Extensions...' text in the context menu for when right-clicking on extension icons (title case).">
Manage Extensions...
</message>
<message name="IDS_EXTENSION_ACTION_INSPECT_POPUP" desc="The text for the right-click menu of page and browser actions which shows the popup and opens the developer tools (title case).">
Inspect Popup
</message>
</if>

<message name="IDS_EXTENSIONS_POLICY_CONTROLLED" desc="The text in the extensions UI informing the user that an extension is policy controlled">
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/extensions/extension_context_menu_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, PolicyDisablesItems) {
ASSERT_TRUE(extension != NULL);

scoped_refptr<ExtensionContextMenuModel> menu(
new ExtensionContextMenuModel(extension, browser()));
new ExtensionContextMenuModel(extension, browser(), NULL));

ExtensionSystem::Get(
browser()->profile())->management_policy()->UnregisterAllProviders();
Expand Down
50 changes: 39 additions & 11 deletions chrome/browser/extensions/extension_context_menu_model.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
#include "chrome/browser/extensions/extension_tab_helper.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tab_contents/tab_contents.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_action.h"
#include "chrome/common/extensions/extension_constants.h"
Expand All @@ -28,16 +30,31 @@ using extensions::Extension;

ExtensionContextMenuModel::ExtensionContextMenuModel(
const Extension* extension,
Browser* browser)
Browser* browser,
PopupDelegate* delegate)
: ALLOW_THIS_IN_INITIALIZER_LIST(SimpleMenuModel(this)),
extension_id_(extension->id()),
browser_(browser),
profile_(browser->profile()) {
extension_action_ = extension->browser_action();
if (!extension_action_)
extension_action_ = extension->page_action();
profile_(browser->profile()),
delegate_(delegate) {
InitMenu(extension);

if (profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode) &&
delegate_) {
AddSeparator();
AddItemWithStringId(INSPECT_POPUP, IDS_EXTENSION_ACTION_INSPECT_POPUP);
}
}

InitCommonCommands();
ExtensionContextMenuModel::ExtensionContextMenuModel(
const Extension* extension,
Browser* browser)
: ALLOW_THIS_IN_INITIALIZER_LIST(SimpleMenuModel(this)),
extension_id_(extension->id()),
browser_(browser),
profile_(browser->profile()),
delegate_(NULL) {
InitMenu(extension);
}

bool ExtensionContextMenuModel::IsCommandIdChecked(int command_id) const {
Expand All @@ -55,6 +72,13 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
// The NAME links to the Homepage URL. If the extension doesn't have a
// homepage, we just disable this menu item.
return extension->GetHomepageURL().is_valid();
} else if (command_id == INSPECT_POPUP) {
TabContents* tab_contents = browser_->GetActiveTabContents();
if (!tab_contents)
return false;

return extension_action_->HasPopup(
tab_contents->extension_tab_helper()->tab_id());
} else if (command_id == DISABLE || command_id == UNINSTALL) {
// Some extension types can not be disabled or uninstalled.
return ExtensionSystem::Get(
Expand Down Expand Up @@ -109,6 +133,10 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id) {
browser_->ShowExtensionsTab();
break;
}
case INSPECT_POPUP: {
delegate_->InspectPopup(extension_action_);
break;
}
default:
NOTREACHED() << "Unknown option";
break;
Expand All @@ -129,13 +157,13 @@ void ExtensionContextMenuModel::ExtensionUninstallCanceled() {

ExtensionContextMenuModel::~ExtensionContextMenuModel() {}

void ExtensionContextMenuModel::InitCommonCommands() {
const Extension* extension = GetExtension();

// The extension pointer should only be null if the extension was uninstalled,
// and since the menu just opened, it should still be installed.
void ExtensionContextMenuModel::InitMenu(const Extension* extension) {
DCHECK(extension);

extension_action_ = extension->browser_action();
if (!extension_action_)
extension_action_ = extension->page_action();

AddItem(NAME, UTF8ToUTF16(extension->name()));
AddSeparator();
AddItemWithStringId(CONFIGURE, IDS_EXTENSIONS_OPTIONS_MENU_ITEM);
Expand Down
30 changes: 27 additions & 3 deletions chrome/browser/extensions/extension_context_menu_model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,31 @@ class ExtensionContextMenuModel
HIDE,
DISABLE,
UNINSTALL,
MANAGE
MANAGE,
INSPECT_POPUP
};

// Creates a menu model for the given extension action.
// Delegate to handle showing an ExtensionAction popup.
class PopupDelegate {
public:
// Called when the user selects the menu item which requests that the
// popup be shown and inspected.
virtual void InspectPopup(ExtensionAction* action) = 0;

protected:
virtual ~PopupDelegate() {}
};

// Creates a menu model for the given extension action. If
// prefs::kExtensionsUIDeveloperMode is enabled then a menu item
// will be shown for "Inspect Popup" which, when selected, will cause
// ShowPopupForDevToolsWindow() to be called on |delegate|.
ExtensionContextMenuModel(const extensions::Extension* extension,
Browser* browser,
PopupDelegate* delegate);

// Create a menu model for the given extension action, without support
// for the "Inspect Popup" command.
ExtensionContextMenuModel(const extensions::Extension* extension,
Browser* browser);

Expand All @@ -57,7 +78,7 @@ class ExtensionContextMenuModel
friend class base::RefCounted<ExtensionContextMenuModel>;
virtual ~ExtensionContextMenuModel();

void InitCommonCommands();
void InitMenu(const extensions::Extension* extension);

// Gets the extension we are displaying the menu for. Returns NULL if the
// extension has been uninstalled and no longer exists.
Expand All @@ -73,6 +94,9 @@ class ExtensionContextMenuModel

Profile* profile_;

// The delegate which handles the 'inspect popup' menu command (or NULL).
PopupDelegate* delegate_;

// Keeps track of the extension uninstall dialog.
scoped_ptr<ExtensionUninstallDialog> extension_uninstall_dialog_;

Expand Down
16 changes: 13 additions & 3 deletions chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ using ui::SimpleMenuModel;

class BrowserActionButton : public content::NotificationObserver,
public ImageLoadingTracker::Observer,
public ExtensionContextMenuModel::PopupDelegate,
public MenuGtk::Delegate {
public:
BrowserActionButton(BrowserActionsToolbarGtk* toolbar,
Expand Down Expand Up @@ -254,7 +255,7 @@ class BrowserActionButton : public content::NotificationObserver,
return NULL;

context_menu_model_ =
new ExtensionContextMenuModel(extension_, toolbar_->browser());
new ExtensionContextMenuModel(extension_, toolbar_->browser(), this);
context_menu_.reset(
new MenuGtk(this, context_menu_model_.get()));
return context_menu_.get();
Expand All @@ -272,7 +273,8 @@ class BrowserActionButton : public content::NotificationObserver,
case ExtensionToolbarModel::ACTION_NONE:
break;
case ExtensionToolbarModel::ACTION_SHOW_POPUP:
ExtensionPopupGtk::Show(popup_url, browser, widget);
ExtensionPopupGtk::Show(popup_url, browser, widget,
ExtensionPopupGtk::SHOW);
break;
}
}
Expand All @@ -294,6 +296,13 @@ class BrowserActionButton : public content::NotificationObserver,
toolbar_->overflow_menu_->Cancel();
}

// ExtensionContextMenuModel::PopupDelegate implementation.
virtual void InspectPopup(ExtensionAction* action) {
GURL popup_url = action->GetPopupUrl(toolbar_->GetCurrentTabId());
ExtensionPopupGtk::Show(popup_url, toolbar_->browser(), widget(),
ExtensionPopupGtk::SHOW_AND_INSPECT);
}

void SetImage(GdkPixbuf* image) {
if (!image_) {
image_ = gtk_image_new_from_pixbuf(image);
Expand Down Expand Up @@ -795,7 +804,8 @@ void BrowserActionsToolbarGtk::ExecuteCommand(int command_id) {
case ExtensionToolbarModel::ACTION_NONE:
break;
case ExtensionToolbarModel::ACTION_SHOW_POPUP:
ExtensionPopupGtk::Show(popup_url, browser(), chevron());
ExtensionPopupGtk::Show(popup_url, browser(), chevron(),
ExtensionPopupGtk::SHOW);
break;
}
}
Expand Down
21 changes: 14 additions & 7 deletions chrome/browser/ui/gtk/extensions/extension_popup_gtk.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ const int ExtensionPopupGtk::kMaxHeight = 600;

ExtensionPopupGtk::ExtensionPopupGtk(Browser* browser,
ExtensionHost* host,
GtkWidget* anchor)
GtkWidget* anchor,
ShowAction show_action)
: browser_(browser),
bubble_(NULL),
host_(host),
anchor_(anchor),
being_inspected_(false),
weak_factory_(this) {
host_->view()->SetContainer(this);
being_inspected_ = show_action == SHOW_AND_INSPECT;

// If the host had somehow finished loading, then we'd miss the notification
// and not show. This seems to happen in single-process mode.
Expand All @@ -65,16 +66,18 @@ ExtensionPopupGtk::ExtensionPopupGtk(Browser* browser,
registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_CLOSING,
content::Source<Profile>(host->profile()));

registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_OPENING,
content::Source<Profile>(host->profile()));
if (!being_inspected_) {
registrar_.Add(this, content::NOTIFICATION_DEVTOOLS_WINDOW_OPENING,
content::Source<Profile>(host->profile()));
}
}

ExtensionPopupGtk::~ExtensionPopupGtk() {
}

// static
void ExtensionPopupGtk::Show(const GURL& url, Browser* browser,
GtkWidget* anchor) {
GtkWidget* anchor, ShowAction show_action) {
ExtensionProcessManager* manager =
browser->profile()->GetExtensionProcessManager();
DCHECK(manager);
Expand All @@ -83,7 +86,7 @@ void ExtensionPopupGtk::Show(const GURL& url, Browser* browser,

ExtensionHost* host = manager->CreatePopupHost(url, browser);
// This object will delete itself when the bubble is closed.
new ExtensionPopupGtk(browser, host, anchor);
new ExtensionPopupGtk(browser, host, anchor, show_action);
}

void ExtensionPopupGtk::Observe(int type,
Expand Down Expand Up @@ -161,6 +164,9 @@ void ExtensionPopupGtk::ShowPopup() {
return;
}

if (being_inspected_)
DevToolsWindow::OpenDevToolsWindow(host_->render_view_host());

// Only one instance should be showing at a time. Get rid of the old one, if
// any. Typically, |current_extension_popup_| will be NULL, but it can be
// non-NULL if a browser action button is clicked while another extension
Expand All @@ -180,7 +186,8 @@ void ExtensionPopupGtk::ShowPopup() {
NULL,
host_->view()->native_view(),
arrow_location,
BubbleGtk::POPUP_WINDOW | BubbleGtk::GRAB_INPUT,
being_inspected_ ? 0 :
BubbleGtk::POPUP_WINDOW | BubbleGtk::GRAB_INPUT,
GtkThemeService::GetFrom(browser_->profile()),
this);
}
Expand Down
17 changes: 12 additions & 5 deletions chrome/browser/ui/gtk/extensions/extension_popup_gtk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ class ExtensionPopupGtk : public content::NotificationObserver,
public BubbleDelegateGtk,
public ExtensionViewGtk::Container {
public:
ExtensionPopupGtk(Browser* browser,
ExtensionHost* host,
GtkWidget* anchor);
virtual ~ExtensionPopupGtk();
enum ShowAction {
SHOW,
SHOW_AND_INSPECT
};

static void Show(const GURL& url,
Browser* browser,
GtkWidget* anchor);
GtkWidget* anchor,
ShowAction show_action);

// content::NotificationObserver implementation.
virtual void Observe(int type,
Expand Down Expand Up @@ -67,6 +68,12 @@ class ExtensionPopupGtk : public content::NotificationObserver,
static const int kMaxHeight;

private:
ExtensionPopupGtk(Browser* browser,
ExtensionHost* host,
GtkWidget* anchor,
ShowAction show_action);
virtual ~ExtensionPopupGtk();

// Shows the popup widget. Called after loading completes.
void ShowPopup();

Expand Down
14 changes: 12 additions & 2 deletions chrome/browser/ui/gtk/location_bar_view_gtk.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1876,12 +1876,13 @@ gboolean LocationBarViewGtk::PageActionViewGtk::OnButtonPressed(
ExtensionPopupGtk::Show(
page_action_->GetPopupUrl(current_tab_id_),
owner_->browser_,
event_box_.get());
event_box_.get(),
ExtensionPopupGtk::SHOW);
break;

case LocationBarController::ACTION_SHOW_CONTEXT_MENU:
context_menu_model_ =
new ExtensionContextMenuModel(extension, owner_->browser_);
new ExtensionContextMenuModel(extension, owner_->browser_, this);
context_menu_.reset(
new MenuGtk(NULL, context_menu_model_.get()));
context_menu_->PopupForWidget(sender, event->button, event->time);
Expand Down Expand Up @@ -1933,3 +1934,12 @@ gboolean LocationBarViewGtk::PageActionViewGtk::OnGtkAccelerator(
event.button = 1;
return view->OnButtonPressed(view->widget(), &event);
}

void LocationBarViewGtk::PageActionViewGtk::InspectPopup(
ExtensionAction* action) {
ExtensionPopupGtk::Show(
action->GetPopupUrl(current_tab_id_),
owner_->browser_,
event_box_.get(),
ExtensionPopupGtk::SHOW_AND_INSPECT);
}
6 changes: 5 additions & 1 deletion chrome/browser/ui/gtk/location_bar_view_gtk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ class LocationBarViewGtk : public AutocompleteEditController,
};

class PageActionViewGtk : public ImageLoadingTracker::Observer,
public content::NotificationObserver {
public content::NotificationObserver,
public ExtensionContextMenuModel::PopupDelegate {
public:
PageActionViewGtk(LocationBarViewGtk* owner, ExtensionAction* page_action);
virtual ~PageActionViewGtk();
Expand Down Expand Up @@ -263,6 +264,9 @@ class LocationBarViewGtk : public AutocompleteEditController,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;

// Overridden from ExtensionContextMenuModel::PopupDelegate:
virtual void InspectPopup(ExtensionAction* action) OVERRIDE;

private:
// Connect the accelerator for the page action popup.
void ConnectPageActionAccelerator();
Expand Down

0 comments on commit c82526d

Please sign in to comment.