Aura: Add "Edit Search Engines" option to the aura omnibox context menu.

Aura: Add "Edit Search Engines" option to the aura omnibox context menu.

BUG=109079
TEST=manually

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118410

[oshima, 2012-01-18 22:24:24]

lgtm with nits

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.cc (right):

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:720:
DCHECK(menu_contents);
you probably don't need this as it will crash on next line if it's NULL.

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:726: return
command_updater_->IsCommandEnabled(command_id);
There is a known issue that views cannot use IDC_ and textfield is using IDS_
instead to deal with this issue. I don't expect collision will happen, but you
may add a comment that NativeTextfieldViews is using IDS_ while command_update_
is IDC_. Maybe we should file a bug to keep track of it and add reference here
and in textfield_views_model.cc

https://chromiumcodereview.appspot.com/9250016/diff/1/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_controller.h (right):

https://chromiumcodereview.appspot.com/9250016/diff/1/ui/views/controls/textf...
ui/views/controls/textfield/textfield_controller.h:56: // Returns true is the
|command_id| should be enabled in the context menu.
/is/if/

[varunjain, 2012-01-18 23:31:36]

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
File chrome/browser/ui/views/omnibox/omnibox_view_views.cc (right):

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:720:
DCHECK(menu_contents);
On 2012/01/18 22:24:24, oshima wrote:
> you probably don't need this as it will crash on next line if it's NULL.

Done.

https://chromiumcodereview.appspot.com/9250016/diff/1/chrome/browser/ui/views...
chrome/browser/ui/views/omnibox/omnibox_view_views.cc:726: return
command_updater_->IsCommandEnabled(command_id);
On 2012/01/18 22:24:24, oshima wrote:
> There is a known issue that views cannot use IDC_ and textfield is using IDS_
> instead to deal with this issue. I don't expect collision will happen, but you
> may add a comment that NativeTextfieldViews is using IDS_ while
command_update_
> is IDC_. Maybe we should file a bug to keep track of it and add reference here
> and in textfield_views_model.cc

Done.

https://chromiumcodereview.appspot.com/9250016/diff/1/ui/views/controls/textf...
File ui/views/controls/textfield/textfield_controller.h (right):

https://chromiumcodereview.appspot.com/9250016/diff/1/ui/views/controls/textf...
ui/views/controls/textfield/textfield_controller.h:56: // Returns true is the
|command_id| should be enabled in the context menu.
On 2012/01/18 22:24:24, oshima wrote:
> /is/if/

Done.

[Ben Goodger (Google), 2012-01-19 21:48:25]

lgtm

[commit-bot: I haz the power, 2012-01-19 23:15:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/9250016/5001

[commit-bot: I haz the power, 2012-01-20 02:06:53]

Try job failure for 9250016-5001 (retry) (retry) on mac_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-01-20 04:27:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varunjain@chromium.org/9250016/5001