Cache --whitelisted-extension-id in SimpleFeature.

extensions/common/features/simple_feature.cc

Index: extensions/common/features/simple_feature.cc
diff --git a/extensions/common/features/simple_feature.cc b/extensions/common/features/simple_feature.cc
index 6a5bece485df46b7ccc9db8ef21c5d62a521ba93..93494065e3fbeed2187f67d76e6fb68721a07934 100644
--- a/extensions/common/features/simple_feature.cc
+++ b/extensions/common/features/simple_feature.cc
@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
+#include "base/macros.h"
 #include "base/sha1.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -24,6 +25,10 @@ namespace extensions {
 
 namespace {
 
+// A singleton copy of the --whitelisted-extension-id so that we don't need to
+// copy it from the CommandLine each time.
+std::string* g_whitelisted_extension_id = NULL;
+
 Feature::Availability IsAvailableToManifestForBind(
     const std::string& extension_id,
     Manifest::Type type,
@@ -222,8 +227,29 @@ bool IsCommandLineSwitchEnabled(const std::string& switch_name) {
   return false;
 }
 
+bool IsWhitelistedForTest(const std::string& extension_id) {
+  if (!g_whitelisted_extension_id) {

[not at google - send to devlin, 2015/04/09 14:36:52]

TODO(jackhou): Delete the commandline whitelisting mechanism.

[jackhou1, 2015/04/10 01:45:31]

Done.

+    g_whitelisted_extension_id = new std::string(
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kWhitelistedExtensionID));
+  }
+  return !g_whitelisted_extension_id->empty() &&

[not at google - send to devlin, 2015/04/09 14:36:52]

I don't think the .empty() check here is necessary. Hopefully it can never be
true, and even if it is, the .empty() check isn't any more efficient than the
first iteration of an equality comparison, I would assume.

[tapted, 2015/04/09 14:56:46]

yeah.. I suggested this nugget of paranoia :)

IsIdInWhitelist -> IsIdInList has a check, `if
!IsValidExtensionId(extension_id)`. IsWhitelistedForTest doesn't have this
check.

The worry is that if something managed to get here with an extension id of "",
it would get whitelisted by default (since it's indistinguishable from the would
match the no-command-line case).

[not at google - send to devlin, 2015/04/09 15:01:28]

ok :-) seems like a CHECK (or early-ignore) situation somewhere, but this test
is a nice bit of paranoia either way.

+         *g_whitelisted_extension_id == extension_id;
+}
+
 }  // namespace
 
+SimpleFeature::ScopedWhitelistForTest::ScopedWhitelistForTest(
+    const std::string& id)
+    : previous_id_(g_whitelisted_extension_id) {
+  g_whitelisted_extension_id = new std::string(id);
+}
+
+SimpleFeature::ScopedWhitelistForTest::~ScopedWhitelistForTest() {
+  delete g_whitelisted_extension_id;
+  g_whitelisted_extension_id = previous_id_;
+}
+
 struct SimpleFeature::Mappings {
   Mappings() {
     extension_types["extension"] = Manifest::TYPE_EXTENSION;
@@ -363,20 +389,9 @@ Feature::Availability SimpleFeature::IsAvailableToManifest(
   if (component_extensions_auto_granted_ && location == Manifest::COMPONENT)
     return CreateAvailability(IS_AVAILABLE, type);
 
-  if (!whitelist_.empty()) {
-    if (!IsIdInWhitelist(extension_id)) {
-      // TODO(aa): This is gross. There should be a better way to test the
-      // whitelist.
-      base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-      if (!command_line->HasSwitch(switches::kWhitelistedExtensionID))
-        return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
-
-      std::string whitelist_switch_value =
-          base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-              switches::kWhitelistedExtensionID);
-      if (extension_id != whitelist_switch_value)
-        return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
-    }
+  if (!whitelist_.empty() && !IsIdInWhitelist(extension_id) &&
+      !IsWhitelistedForTest(extension_id)) {
+    return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
   }
 
   if (!MatchesManifestLocation(location))