Prevent browser actions and page actions from being used with

apps.

BUG=58904
TEST=unit_tests --gtest_filter=Extension*

Review URL: http://codereview.chromium.org/3787002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62492 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 31 insertions, 19 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 69a6d9f..76a464d 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1593,12 +1593,6 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
 
   // Initialize browser action (optional).
   if (source.HasKey(keys::kBrowserAction)) {
-    // Restrict extensions to one UI surface.
-    if (page_action_.get()) {
-      *error = errors::kOneUISurfaceOnly;
-      return false;
-    }
-
     DictionaryValue* browser_action_value;
     if (!source.GetDictionary(keys::kBrowserAction, &browser_action_value)) {
       *error = errors::kInvalidBrowserAction;
@@ -1813,6 +1807,11 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
     }
   }
 
+  if (HasMultipleUISurfaces()) {
+    *error = errors::kOneUISurfaceOnly;
+    return false;
+  }
+
   InitEffectiveHostPermissions();
 
   // Although |source| is passed in as a const, it's still possible to modify
@@ -2069,6 +2068,21 @@ void Extension::InitEffectiveHostPermissions() {
   }
 }
 
+bool Extension::HasMultipleUISurfaces() const {
+  int num_surfaces = 0;
+
+  if (page_action_.get())
+    ++num_surfaces;
+
+  if (browser_action_.get())
+    ++num_surfaces;
+
+  if (is_app())
+    ++num_surfaces;
+
+  return num_surfaces > 1;
+}
+
 // static
 bool Extension::CanExecuteScriptOnPage(
     const GURL& page_url, bool can_execute_script_everywhere,
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 1c72cec..be01f2c 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -552,6 +552,10 @@ class Extension {
   // script petterns.
   void InitEffectiveHostPermissions();
 
+  // Returns true if the extension has more than one "UI surface". For example,
+  // an extension that has a browser action and a page action.
+  bool HasMultipleUISurfaces() const;
+
   // Figures out if a source contains keys not associated with themes - we
   // don't want to allow scripts and such to be bundled with themes.
   bool ContainsNonThemeKeys(const DictionaryValue& source);
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index 17444ee..983fda4a 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -264,7 +264,7 @@ const char* kOmniboxExperimental =
     "You must request the 'experimental' permission in order to use the"
     " omnibox API.";
 const char* kOneUISurfaceOnly =
-    "An extension cannot have both a page action and a browser action.";
+    "Only one of 'browser_action', 'page_action', and 'app' can be specified.";
 const char* kReservedMessageFound =
     "Reserved key * found in message catalog.";
 const char* kThemesCannotContainExtensions =
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index 9a660bc..ac56f70 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -332,3 +332,9 @@ TEST_F(ExtensionManifestTest, NormalizeIconPaths) {
   EXPECT_EQ("48.png",
             extension->icons().Get(48, ExtensionIconSet::MATCH_EXACTLY));
 }
+
+TEST_F(ExtensionManifestTest, DisallowMultipleUISurfaces) {
+  LoadAndExpectError("multiple_ui_surfaces_1.json", errors::kOneUISurfaceOnly);
+  LoadAndExpectError("multiple_ui_surfaces_2.json", errors::kOneUISurfaceOnly);
+  LoadAndExpectError("multiple_ui_surfaces_3.json", errors::kOneUISurfaceOnly);
+}
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 47618c2..fcbec3c 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -266,18 +266,6 @@ TEST(ExtensionTest, InitFromValueInvalid) {
   EXPECT_FALSE(extension.InitFromValue(*input_value, true, &error));
   EXPECT_STREQ(errors::kInvalidPageActionsListSize, error.c_str());
 
-  // Test invalid UI surface count (both page action and browser action).
-  input_value.reset(static_cast<DictionaryValue*>(valid_value->DeepCopy()));
-  action = new DictionaryValue;
-  action->SetString(keys::kPageActionId, "MyExtensionActionId");
-  action->SetString(keys::kName, "MyExtensionActionName");
-  action_list = new ListValue;
-  action_list->Append(action->DeepCopy());
-  input_value->Set(keys::kPageActions, action_list);
-  input_value->Set(keys::kBrowserAction, action);
-  EXPECT_FALSE(extension.InitFromValue(*input_value, true, &error));
-  EXPECT_STREQ(errors::kOneUISurfaceOnly, error.c_str());
-
   // Test invalid options page url.
   input_value.reset(static_cast<DictionaryValue*>(valid_value->DeepCopy()));
   input_value->Set(keys::kOptionsPage, Value::CreateNullValue());