ChromeExtensionLoader: Prevent redundant loading of unpacked extensions.

Unpacked extensions loaded through the ChromeTestExtensionLoader are loaded
twice. The redundant load is due to the  false default value of
|allow_file_access_|. However, unpacked extensions are by default, loaded with
file access. This mismatch causes the unpacked extensions to be reloaded in
ChromeTestExtensionLoader::CheckPermissions.

Prevent this reload by using Manifest::ShouldAllowFileAccess for cases where
clients don't explicitly set |allow_file_access_|.

BUG=757695

Change-Id: Ied4b25ccb89ff1410072186d360634609b8666e5
Reviewed-on: https://chromium-review.googlesource.com/625247
Reviewed-by: Devlin <[email protected]>
Commit-Queue: Karan Bhatia <[email protected]>
Cr-Commit-Position: refs/heads/master@{#496472}
diff --git a/chrome/browser/extensions/chrome_test_extension_loader.cc b/chrome/browser/extensions/chrome_test_extension_loader.cc
index 3c7632f..600586a 100644
--- a/chrome/browser/extensions/chrome_test_extension_loader.cc
+++ b/chrome/browser/extensions/chrome_test_extension_loader.cc
@@ -71,8 +71,10 @@
   // non-shared modules.
   // TODO(devlin): That's not good; we shouldn't be crashing.
   if (!SharedModuleInfo::IsSharedModule(extension.get())) {
+    CheckPermissions(extension.get());
+    // Make |extension| null since it may have been reloaded invalidating
+    // pointers to it.
     extension = nullptr;
-    CheckPermissions(extension_id_);
   }
 
   if (!install_param_.empty()) {
@@ -191,15 +193,25 @@
   return extension;
 }
 
-void ChromeTestExtensionLoader::CheckPermissions(
-    const std::string& extension_id) {
-  std::string id = extension_id;
+void ChromeTestExtensionLoader::CheckPermissions(const Extension* extension) {
+  std::string id = extension->id();
+
+  // If the client explicitly set |allow_file_access_|, use that value. Else
+  // use the default as per the extensions manifest location.
+  if (!allow_file_access_) {
+    allow_file_access_ =
+        Manifest::ShouldAlwaysAllowFileAccess(extension->location());
+  }
+
+  // |extension| may be reloaded subsequently, invalidating the pointer. Hence
+  // make it null.
+  extension = nullptr;
 
   // Toggling incognito or file access will reload the extension, so wait for
   // the reload.
-  if (allow_file_access_ != util::AllowFileAccess(id, browser_context_)) {
+  if (*allow_file_access_ != util::AllowFileAccess(id, browser_context_)) {
     TestExtensionRegistryObserver registry_observer(extension_registry_, id);
-    util::SetAllowFileAccess(id, browser_context_, allow_file_access_);
+    util::SetAllowFileAccess(id, browser_context_, *allow_file_access_);
     registry_observer.WaitForExtensionLoaded();
   }