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

Issue 9705098: Add Extension Keybinding test for page actions and port the PageAction keybindings to GTK (second a… (Closed)

Created:
8 years, 9 months ago by Finnur
Modified:
8 years, 9 months ago
CC:
chromium-reviews, Aaron Boodman, mihaip+watch_chromium.org
Visibility:
Public.

Description

Add Extension Keybinding test for page actions and port the PageAction keybindings to GTK (second attempt). Previous attempt failed because browser() was refactored out of the LocationBarView class before I had a chance to check in. Only changes from last changelist (https://chromiumcodereview.appspot.com/9693047) are in LocationBarView lines 1042-1044. BUG=27702 TEST=KeybindingApiTest.PageAction Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=127156

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+215 lines, -15 lines) Patch
M chrome/browser/extensions/extension_keybinding_apitest.cc View 5 chunks +46 lines, -13 lines 0 comments Download
M chrome/browser/ui/gtk/location_bar_view_gtk.h View 5 chunks +37 lines, -0 lines 0 comments Download
M chrome/browser/ui/gtk/location_bar_view_gtk.cc View 9 chunks +93 lines, -1 line 1 comment Download
M chrome/browser/ui/views/location_bar/location_bar_view.cc View 1 chunk +3 lines, -1 line 0 comments Download
A chrome/test/data/extensions/api_test/keybinding/page_action/background.js View 1 chunk +15 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/keybinding/page_action/manifest.json View 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Finnur
8 years, 9 months ago (2012-03-16 13:09:30 UTC) #1
tommi (sloooow) - chröme
8 years, 9 months ago (2012-03-16 13:13:08 UTC) #2
lgtm. relying on previous review for the most part.

https://chromiumcodereview.appspot.com/9705098/diff/1/chrome/browser/ui/gtk/l...
File chrome/browser/ui/gtk/location_bar_view_gtk.cc (right):

https://chromiumcodereview.appspot.com/9705098/diff/1/chrome/browser/ui/gtk/l...
chrome/browser/ui/gtk/location_bar_view_gtk.cc:1609: DCHECK(type ==
chrome::NOTIFICATION_WINDOW_CLOSED);
nit: DCHECK_EQ

Powered by Google App Engine
This is Rietveld 408576698