Bookmark pop-up doesn't open if Ctrl+D is set as keyboard shortcut for added extensions.

chrome/browser/ui/views/location_bar/star_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/location_bar/star_view.h"
 
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/bookmarks/bookmark_stats.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
-StarView::StarView(CommandUpdater* command_updater)
-    : BubbleIconView(command_updater, IDC_BOOKMARK_PAGE) {
+StarView::StarView(Browser* browser)
+    : BubbleIconView(browser->command_controller()->command_updater(),
+                     IDC_BOOKMARK_PAGE) {
+  browser_ = browser;

[Mike Wittman, 2014/12/12 20:41:57]

Nit: move to initializer list.

[Deepak, 2014/12/13 03:36:53]

Done.

   set_id(VIEW_ID_STAR_BUTTON);
   SetToggled(false);
 }
 
 StarView::~StarView() {}
 
 void StarView::SetToggled(bool on) {
   SetTooltipText(l10n_util::GetStringUTF16(
       on ? IDS_TOOLTIP_STARRED : IDS_TOOLTIP_STAR));
   SetImage(ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
@@ skipping 15 matching lines @@
       entry_point = BOOKMARK_ENTRY_POINT_STAR_KEY;
       break;
     case EXECUTE_SOURCE_GESTURE:
       entry_point = BOOKMARK_ENTRY_POINT_STAR_GESTURE;
       break;
   }
   UMA_HISTOGRAM_ENUMERATION("Bookmarks.EntryPoint",
                             entry_point,
                             BOOKMARK_ENTRY_POINT_LIMIT);
 }
+
+void StarView::ExecuteCommand(ExecuteSource source) {
+  OnExecuting(source);
+  if (browser_)

[Mike Wittman, 2014/12/12 20:41:57]

Nit: you're already depending on browser to be non-null in the constructor, so
remove this check.

[Peter Kasting, 2014/12/12 21:49:05]

Actually, it turns out I was wrong about it always being safe to compute the
command updater from the browser.  Instead, the constructor should take both
Browser* and CommandUpdater* arguments, pass the latter to the parent class, and
then in this function do:

  if (browser_) {
    OnExecuting(source);
    chrome::BookmarkCurrentPageIgnoringExtensionOverrides(browser_);
  } else {
    BubbleIconView::ExecuteCommand(source);
  }

[Deepak, 2014/12/13 03:36:53]

Done.

[Deepak, 2014/12/13 03:36:54]

For considering Peter's advice, I need this if check.

+    chrome::BookmarkCurrentPageIgnoringExtensionOverrides(browser_);
+}