mac: Allows extensions to hide the bookmark page menu item.

chrome/browser/ui/bookmarks/bookmark_utils.cc

Index: chrome/browser/ui/bookmarks/bookmark_utils.cc
diff --git a/chrome/browser/ui/bookmarks/bookmark_utils.cc b/chrome/browser/ui/bookmarks/bookmark_utils.cc
index 132bfd424a382028d346f6ca4e2a0477b560b899..8c4fce09b7f3b17b1d34f2c7f1f1c9d9b92ab823 100644
--- a/chrome/browser/ui/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_utils.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/extensions/api/commands/command_service.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/app_list/app_list_util.h"
@@ -24,6 +25,8 @@
 #include "chrome/common/url_constants.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/common/extension_set.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/net_util.h"
@@ -295,15 +298,29 @@ bool ShouldShowAppsShortcutInBookmarkBar(
       profile->GetPrefs()->GetBoolean(prefs::kShowAppsShortcutInBookmarkBar);
 }
 
-BookmarkShortcutDisposition GetBookmarkShortcutDisposition(
-    const extensions::CommandService* command_service,
-    const extensions::Extension* extension) {
-  if (command_service->OverridesBookmarkShortcut(extension))
-    return BOOKMARK_SHORTCUT_DISPOSITION_OVERRIDDEN;
+BookmarkShortcutDisposition GetBookmarkShortcutDisposition(Profile* profile) {
+  extensions::CommandService* command_service =
+      extensions::CommandService::Get(profile);
+
+  const extensions::ExtensionSet* extension_set =
+      extensions::ExtensionSystem::Get(profile)
+          ->extension_service()

[Robert Sesek, 2014/02/26 18:05:13]

It's more common to put the pointer indirection operator on the preceding line:

https://code.google.com/p/chromium/codesearch#search/&q=%5E%5Cs%2B-%3E%20file...
https://code.google.com/p/chromium/codesearch#search/&q=%5C-%3E$%20file:src/c...

[erikchen, 2014/02/26 19:58:23]

Done. I was using the output of clang-format, fyi.
Also, your links don't load. :(

On 2014/02/26 18:05:13, rsesek wrote:

[Robert Sesek, 2014/02/26 20:30:00]

Weird, Rietveld double-percent-escapes them. It's was something like 160
instances of on new line vs 1200 on previous line. 

+          ->extensions();

[Robert Sesek, 2014/02/26 18:05:13]

This will fit on the previous line.

[erikchen, 2014/02/26 19:58:23]

ditto

On 2014/02/26 18:05:13, rsesek wrote:

+  for (extensions::ExtensionSet::const_iterator i = extension_set->begin();

[erikchen, 2014/02/26 01:08:19]

The extension proposal:
https://docs.google.com/a/chromium.org/document/d/1C2Mle92O9uGlji5y5gGDM5tNJ_...

is unclear about what to do if multiple extensions set the flag to override the
bookmark menu item. I take the simplest approach, which is to use disposition of
the first extension with the given flag. Feel free to suggest a different
approach.

[Robert Sesek, 2014/02/26 18:05:13]

This approach is probably fine, but you may want to check with the people
designing this feature if the behavior should be better defined.

[Mike Wittman, 2014/02/26 18:29:05]

The extension keybinding behavior is that the first extension to request the
shortcut gets it. So we don't need to care about which extension asks for the
override as long as one does.

I think the proper logic here should be something like:

if any extension overrides the bookmark shortcut
  return BOOKMARK_SHORTCUT_DISPOSITION_OVERRIDDEN
else if any extension removes the bookmark shortcut
  return BOOKMARK_SHORTCUT_DISPOSITION_REMOVED
else
  return BOOKMARK_SHORTCUT_DISPOSITION_UNCHANGED

[erikchen, 2014/02/26 19:58:23]

Done.

+       i != extension_set->end();
+       ++i) {
+    if (command_service->OverridesBookmarkShortcut(*i))
+      return BOOKMARK_SHORTCUT_DISPOSITION_OVERRIDDEN;
+
+    if (extensions::CommandService::RemovesBookmarkShortcut(*i))
+      return BOOKMARK_SHORTCUT_DISPOSITION_REMOVED;
+  }
+  return BOOKMARK_SHORTCUT_DISPOSITION_UNCHANGED;
+}
 
-  return extensions::CommandService::RemovesBookmarkShortcut(extension) ?
-      BOOKMARK_SHORTCUT_DISPOSITION_REMOVED :
-      BOOKMARK_SHORTCUT_DISPOSITION_UNCHANGED;
+bool ShouldShowBookmarkPageMenuItem(Profile* profile) {
+  return GetBookmarkShortcutDisposition(profile) !=
+         BOOKMARK_SHORTCUT_DISPOSITION_REMOVED;
 }
 
 }  // namespace chrome