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();
}
diff --git a/chrome/browser/extensions/chrome_test_extension_loader.h b/chrome/browser/extensions/chrome_test_extension_loader.h
index c15e4d7..04a27be 100644
--- a/chrome/browser/extensions/chrome_test_extension_loader.h
+++ b/chrome/browser/extensions/chrome_test_extension_loader.h
@@ -10,6 +10,7 @@
#include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h"
#include "base/macros.h"
+#include "base/optional.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest.h"
@@ -91,7 +92,7 @@
const base::FilePath& unpacked_path);
// Checks that the permissions of the loaded extension are correct.
- void CheckPermissions(const std::string& extension_id);
+ void CheckPermissions(const Extension* extension);
// Checks for any install warnings associated with the extension.
bool CheckInstallWarnings(const Extension& extension);
@@ -125,7 +126,8 @@
// extension. Only used for crx installs.
int creation_flags_ = Extension::NO_FLAGS;
- // The install location of the added extension.
+ // The install location of the added extension. Not valid for unpacked
+ // extensions.
Manifest::Location location_ = Manifest::INTERNAL;
// Whether or not the extension load should fail.
@@ -144,7 +146,7 @@
bool grant_permissions_ = true;
// Whether or not to allow file access by default to the extension.
- bool allow_file_access_ = false;
+ base::Optional<bool> allow_file_access_;
// Whether or not to allow incognito access by default to the extension.
bool allow_incognito_access_ = false;
