With enable-action-box add extensions to BrowserActionContainer only if they have preference kBrows…

chrome/browser/extensions/extension_prefs.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/extensions/extension_prefs.h"
 
+#include "base/command_line.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/api/alarms/alarm_manager.h"
 #include "chrome/browser/extensions/api/omnibox/omnibox_api.h"
 #include "chrome/browser/extensions/extension_pref_store.h"
 #include "chrome/browser/extensions/extension_sorting.h"
 #include "chrome/browser/prefs/pref_notifier.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest.h"
 #include "chrome/common/extensions/url_pattern.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/notification_service.h"
 
 namespace {
 
 // Additional preferences keys
 
@@ skipping 78 matching lines @@
 
 // A preference specifying if the user dragged the app on the NTP.
 const char kPrefUserDraggedApp[] = "user_dragged_app_ntp";
 
 // A preference for storing extra data sent in update checks for an extension.
 const char kUpdateUrlData[] = "update_url_data";
 
 // Whether the browser action is visible in the toolbar.
 const char kBrowserActionVisible[] = "browser_action_visible";
 
+// Whether the browser action is pinned in the toolbar. This will eventually
+// replace kBrowserActionVisible.
+const char kBrowserActionPinned[] = "browser_action_pinned";
+
 // Preferences that hold which permissions the user has granted the extension.
 // We explicitly keep track of these so that extensions can contain unknown
 // permissions, for backwards compatibility reasons, and we can still prompt
 // the user to accept them once recognized. We store the active permission
 // permissions because they may differ from those defined in the manifest.
 const char kPrefActivePermissions[] = "active_permissions";
 const char kPrefGrantedPermissions[] = "granted_permissions";
 
 // The preference names for ExtensionPermissionSet values.
 const char kPrefAPIs[] = "api";
@@ skipping 1137 matching lines @@
                                        Extension::State state) {
   UpdateExtensionPref(extension_id, kPrefState,
                       Value::CreateIntegerValue(state));
   bool enabled = (state == Extension::ENABLED);
   extension_pref_value_map_->SetExtensionState(extension_id, enabled);
   content_settings_store_->SetExtensionState(extension_id, enabled);
 }
 
 bool ExtensionPrefs::GetBrowserActionVisibility(const Extension* extension) {
   const DictionaryValue* extension_prefs = GetExtensionPref(extension->id());
-  if (!extension_prefs)
-    return true;
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  bool enable_action_box = command_line.HasSwitch(switches::kEnableActionBox);

[Aaron Boodman, 2012/05/14 20:18:13]

Nit: variable should not have typically have verb names. So action_box_enabled
would be better.

[yefimt, 2012/05/14 21:07:36]

Done.

+  if (!extension_prefs) {

[Aaron Boodman, 2012/05/14 20:18:13]

Nit: If an if statement is less than two lines total, it does not require
braces.

[Aaron Boodman, 2012/05/14 20:18:13]

You can simplify this logic somewhat:

bool default_value = !action_box_enabled;

ExtensionPrefs* prefs = ...
if (!prefs)
  return default_value;

bool value;
if (!prefs->GetBoolean(name, &value))
  return default_value;

return value;

[yefimt, 2012/05/14 21:07:36]

Done.

[yefimt, 2012/05/14 21:07:36]

Done.

[yefimt, 2012/05/14 21:07:36]

Done.

+    return enable_action_box ? false : true;
+  }
+
   bool visible = false;
-  if (!extension_prefs->GetBoolean(kBrowserActionVisible, &visible) || visible)
-    return true;
+  const char* browser_action_pref = enable_action_box ? kBrowserActionPinned :
+                                                        kBrowserActionVisible;
+  bool pref_exists = extension_prefs->GetBoolean(browser_action_pref,
+                                                 &visible);
+  if (enable_action_box) {
+    if (pref_exists && visible)
+      return true;
+  } else {
+    if (!pref_exists || visible)
+      return true;
+  }
 
   return false;
 }
 
 void ExtensionPrefs::SetBrowserActionVisibility(const Extension* extension,
                                                 bool visible) {
   if (GetBrowserActionVisibility(extension) == visible)
     return;
 
-  UpdateExtensionPref(extension->id(), kBrowserActionVisible,
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  bool enable_action_box = command_line.HasSwitch(switches::kEnableActionBox);

[Aaron Boodman, 2012/05/14 20:18:13]

Since you are checking this in multiple places, can you please add a function to
extension_switch_utils.*, and call that function instead?

+  const char* browser_action_pref = enable_action_box ? kBrowserActionPinned :
+                                                        kBrowserActionVisible;
+  UpdateExtensionPref(extension->id(), browser_action_pref,
                       Value::CreateBooleanValue(visible));
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
       content::Source<ExtensionPrefs>(this),
       content::Details<const Extension>(extension));
 }
 
 std::string ExtensionPrefs::GetVersionString(const std::string& extension_id) {
   const DictionaryValue* extension = GetExtensionPref(extension_id);
   if (!extension)
@@ skipping 596 matching lines @@
   prefs->RegisterStringPref(prefs::kExtensionBlacklistUpdateVersion,
                             "0",  // default value
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterInt64Pref(prefs::kLastExtensionsUpdateCheck,
                            0,  // default value
                            PrefService::UNSYNCABLE_PREF);
   prefs->RegisterInt64Pref(prefs::kNextExtensionsUpdateCheck,
                            0,  // default value
                            PrefService::UNSYNCABLE_PREF);
 }