Fix issue where we weren't requiring the command-line flag for

experimental API use and weren't requiring that it be declared in the
manifest.

BUG=31537
TEST=none

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

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

8 files changed, 127 insertions, 13 deletions

diff --git a/chrome/browser/extensions/permissions_apitest.cc b/chrome/browser/extensions/permissions_apitest.cc
new file mode 100644
index 0000000..5554f82
--- /dev/null
+++ b/chrome/browser/extensions/permissions_apitest.cc
@@ -0,0 +1,31 @@
+// Copyright (c) 2009 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_apitest.h"
+#include "chrome/common/chrome_switches.h"
+
+class ExperimentalApiTest : public ExtensionApiTest {
+public:
+  void SetUpCommandLine(CommandLine* command_line) {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, PermissionsFail) {
+  ASSERT_TRUE(RunExtensionTest("permissions/disabled")) << message_;
+
+  // Since the experimental APIs require a flag, this will fail even though
+  // It's enabled.
+  // TODO(erikkay) This test is currently broken because LoadExtension in
+  // ExtensionBrowserTest doesn't actually fail, it just times out.  To fix this
+  // I'll need to add an EXTENSION_LOAD_ERROR notification, which is probably
+  // too much for the branch.  I'll enable this on trunk later.
+  //ASSERT_FALSE(RunExtensionTest("permissions/enabled"))) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(ExperimentalApiTest, PermissionsSucceed) {
+  ASSERT_TRUE(RunExtensionTest("permissions/enabled")) << message_;
+}
+
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 829be9e..fa2bc4f 100755
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1114,6 +1114,7 @@
         'browser/extensions/incognito_noscript_apitest.cc',
         'browser/extensions/isolated_world_apitest.cc',
         'browser/extensions/page_action_apitest.cc',
+        'browser/extensions/permissions_apitest.cc',
         'browser/extensions/stubs_apitest.cc',
         'browser/gtk/bookmark_manager_browsertest.cc',
         'browser/net/cookie_policy_browsertest.cc',
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 5dfdf1c..1ccaeec 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -61,15 +61,15 @@ static void ConvertHexadecimalToIDAlphabet(std::string* id) {
 // Returns true if the given string is an API permission (see kPermissionNames).
 static bool IsAPIPermission(const std::string& str) {
   for (size_t i = 0; i < Extension::kNumPermissions; ++i) {
-    if (str == Extension::kPermissionNames[i])
+    if (str == Extension::kPermissionNames[i]) {
+      if (str == Extension::kExperimentalPermission &&
+          !CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kEnableExperimentalExtensionApis)) {
+        return false;
+      }
       return true;
+    }
   }
-
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableExperimentalExtensionApis) &&
-      str == Extension::kExperimentalName)
-    return true;
-
   return false;
 }
 
@@ -116,17 +116,17 @@ const int Extension::kBrowserActionIconMaxSize = 19;
 const char* Extension::kTabPermission = "tabs";
 const char* Extension::kBookmarkPermission = "bookmarks";
 const char* Extension::kNotificationPermission = "notifications";
+const char* Extension::kExperimentalPermission = "experimental";
 
 const char* Extension::kPermissionNames[] = {
   Extension::kTabPermission,
   Extension::kBookmarkPermission,
-  Extension::kNotificationPermission
+  Extension::kNotificationPermission,
+  Extension::kExperimentalPermission
 };
 const size_t Extension::kNumPermissions =
     arraysize(Extension::kPermissionNames);
 
-const char* Extension::kExperimentalName = "experimental";
-
 Extension::~Extension() {
 }
 
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 9906aea..b1e1b69 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -73,13 +73,11 @@ class Extension {
   static const char* kTabPermission;
   static const char* kBookmarkPermission;
   static const char* kNotificationPermission;
+  static const char* kExperimentalPermission;
 
   static const char* kPermissionNames[];
   static const size_t kNumPermissions;
 
-  // Experimental extension APIs are guarded by the following flag.
-  static const char* kExperimentalName;
-
   struct PrivacyBlacklistInfo {
     FilePath path;  // Path to the plain-text blacklist.
   };
diff --git a/chrome/test/data/extensions/api_test/permissions/disabled/background.html b/chrome/test/data/extensions/api_test/permissions/disabled/background.html
new file mode 100755
index 0000000..d235c08
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/permissions/disabled/background.html
@@ -0,0 +1,37 @@
+<script>

+// All of these tests should fail, since this extension has requested no

+// permissions.

+

+chrome.test.runTests([

+  function history() {

+    try {

+      var query = { 'search': '', 'maxResults': 1 };

+      chrome.experimental.history.search(query, function(results) {

+        chrome.test.fail();

+      });

+    } catch (e) {

+      chrome.test.succeed();

+    }

+  },

+

+  function bookmarks() {

+    try {

+      chrome.bookmarks.get("1", function(results) {

+        chrome.test.fail();

+      });

+    } catch (e) {

+      chrome.test.succeed();

+    }

+  },

+

+  function tabs() {

+    try {

+      chrome.tabs.getSelected(null, function(results) {

+        chrome.test.fail();

+      });

+    } catch (e) {

+      chrome.test.succeed();

+    }

+  }

+]);

+</script>
\ No newline at end of file
diff --git a/chrome/test/data/extensions/api_test/permissions/disabled/manifest.json b/chrome/test/data/extensions/api_test/permissions/disabled/manifest.json
new file mode 100755
index 0000000..55b27ee
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/permissions/disabled/manifest.json
@@ -0,0 +1,7 @@
+{

+   "name": "permissions failure apitest",

+   "description": "permissions failure extension",

+   "version": "0.1",

+   "background_page": "background.html",

+   "permissions": [ ]

+}

diff --git a/chrome/test/data/extensions/api_test/permissions/enabled/background.html b/chrome/test/data/extensions/api_test/permissions/enabled/background.html
new file mode 100755
index 0000000..e5e94b1
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/permissions/enabled/background.html
@@ -0,0 +1,33 @@
+<script>

+// All of these tests should pass, since this extension has requested all of the

+// permissions.

+

+var pass = chrome.test.callbackPass;

+

+chrome.test.runTests([

+  function history() {

+    try {

+      var query = { 'search': '', 'maxResults': 1 };

+      chrome.experimental.history.search(query, pass(function(results) {}));

+    } catch (e) {

+      chrome.test.fail();

+    }

+  },

+

+  function bookmarks() {

+    try {

+      chrome.bookmarks.get("1", pass(function(results) {}));

+    } catch (e) {

+      chrome.test.fail();

+    }

+  },

+

+  function tabs() {

+    try {

+      chrome.tabs.getSelected(null, pass(function(results) {}));

+    } catch (e) {

+      chrome.test.fail();

+    }

+  }

+]);

+</script>
\ No newline at end of file
diff --git a/chrome/test/data/extensions/api_test/permissions/enabled/manifest.json b/chrome/test/data/extensions/api_test/permissions/enabled/manifest.json
new file mode 100755
index 0000000..060964b
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/permissions/enabled/manifest.json
@@ -0,0 +1,7 @@
+{

+   "name": "permissions failure apitest",

+   "description": "permissions failure extension",

+   "version": "0.1",

+   "background_page": "background.html",

+   "permissions": [ "tabs", "bookmarks", "experimental" ]

+}