Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(115)

Issue 9968076: Remove Inspect Popup command from browser actions. (Closed)

Created:
8 years, 8 months ago by benwells
Modified:
8 years, 8 months ago
CC:
chromium-reviews, mihaip+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Remove Inspect Popup command from browser actions. Now that Inspect Element on the popup works, this command is no longer needed. BUG=53518 TEST=Tested on windows, linux and mac Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132542

Patch Set 1 #

Patch Set 2 : Woopsies #

Total comments: 4

Patch Set 3 : Moved includes to cc #

Total comments: 5

Patch Set 4 : Rebase #

Patch Set 5 : Removed unused parameter for gtk. #

Patch Set 6 : Rebase 2 #

Patch Set 7 : Views compile failure #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -268 lines) Patch
M chrome/app/generated_resources.grd View 2 chunks +0 lines, -6 lines 0 comments Download
M chrome/browser/extensions/extension_context_menu_model.h View 1 2 chunks +2 lines, -21 lines 0 comments Download
M chrome/browser/extensions/extension_context_menu_model.cc View 1 2 3 3 chunks +3 lines, -22 lines 0 comments Download
M chrome/browser/ui/cocoa/extensions/extension_action_context_menu.h View 3 chunks +0 lines, -20 lines 0 comments Download
M chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm View 1 2 3 5 chunks +1 line, -126 lines 0 comments Download
M chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc View 1 2 3 4 6 chunks +5 lines, -14 lines 0 comments Download
M chrome/browser/ui/gtk/extensions/extension_popup_gtk.h View 1 2 3 4 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/browser/ui/gtk/extensions/extension_popup_gtk.cc View 1 2 3 4 7 chunks +9 lines, -13 lines 0 comments Download
M chrome/browser/ui/gtk/infobars/extension_infobar_gtk.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/location_bar_view_gtk.h View 1 2 3 4 2 chunks +3 lines, -8 lines 0 comments Download
M chrome/browser/ui/gtk/location_bar_view_gtk.cc View 1 2 3 4 3 chunks +4 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/browser_actions_container.h View 1 2 3 chunks +0 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/browser_actions_container.cc View 1 2 3 3 chunks +2 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/extensions/extension_popup.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/views/infobars/extension_infobar.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/location_bar/page_action_image_view.h View 1 2 3 4 5 6 3 chunks +1 line, -5 lines 0 comments Download
M chrome/browser/ui/views/location_bar/page_action_image_view.cc View 1 2 3 chunks +2 lines, -6 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
benwells
reviewers: - aa for chrome/browser/extensions - sail for chrome/browser/ui/cocoa - estade for chrome/browser/ui/gtk - sky ...
8 years, 8 months ago (2012-04-12 04:20:40 UTC) #1
Aaron Boodman
c/b/e/... lgtm
8 years, 8 months ago (2012-04-12 04:23:37 UTC) #2
sail
lgtm
8 years, 8 months ago (2012-04-12 14:45:48 UTC) #3
sky
LGTM http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h File chrome/browser/ui/views/browser_actions_container.h (left): http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h#oldcode274 chrome/browser/ui/views/browser_actions_container.h:274: public ExtensionContextMenuModel::PopupDelegate, Are there any includes that can ...
8 years, 8 months ago (2012-04-12 15:57:23 UTC) #4
benwells
http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h File chrome/browser/ui/views/browser_actions_container.h (left): http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h#oldcode274 chrome/browser/ui/views/browser_actions_container.h:274: public ExtensionContextMenuModel::PopupDelegate, On 2012/04/12 15:57:23, sky wrote: > Are ...
8 years, 8 months ago (2012-04-13 04:56:19 UTC) #5
benwells
On 2012/04/13 04:56:19, benwells wrote: > http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h > File chrome/browser/ui/views/browser_actions_container.h (left): > > http://codereview.chromium.org/9968076/diff/5001/chrome/browser/ui/views/browser_actions_container.h#oldcode274 > ...
8 years, 8 months ago (2012-04-13 04:57:59 UTC) #6
Evan Stade
http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc File chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc (right): http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc#newcode269 chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc:269: bool ShowPopup(bool devtools) { don't need the param any ...
8 years, 8 months ago (2012-04-13 17:09:01 UTC) #7
benwells
http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc File chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc (right): http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc#newcode281 chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc:281: widget(), devtools); On 2012/04/13 17:09:01, Evan Stade wrote: > ...
8 years, 8 months ago (2012-04-13 21:23:20 UTC) #8
benwells
http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc File chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc (right): http://codereview.chromium.org/9968076/diff/12001/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc#newcode269 chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc:269: bool ShowPopup(bool devtools) { On 2012/04/13 17:09:01, Evan Stade ...
8 years, 8 months ago (2012-04-16 02:20:31 UTC) #9
Evan Stade
gtk lgtm
8 years, 8 months ago (2012-04-16 18:28:00 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/9968076/23001
8 years, 8 months ago (2012-04-16 22:31:55 UTC) #11
commit-bot: I haz the power
Can't apply patch for file chrome/browser/ui/views/infobars/extension_infobar.cc. While running patch -p1 --forward --force; patching file chrome/browser/ui/views/infobars/extension_infobar.cc ...
8 years, 8 months ago (2012-04-16 22:32:03 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/9968076/28002
8 years, 8 months ago (2012-04-16 23:09:10 UTC) #13
commit-bot: I haz the power
Try job failure for 9968076-28002 (retry) on linux_chromeos for step "compile" (clobber build). It's a ...
8 years, 8 months ago (2012-04-16 23:39:01 UTC) #14
benwells
Ran the wrong try job earlier, fixing now.
8 years, 8 months ago (2012-04-17 01:08:26 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/9968076/31002
8 years, 8 months ago (2012-04-17 03:28:31 UTC) #16
commit-bot: I haz the power
8 years, 8 months ago (2012-04-17 04:49:35 UTC) #17
Change committed as 132542

Powered by Google App Engine
This is Rietveld 408576698