Do not finish delayed installs if they should remain being delayed
ExtensionServiceInterface exposed FinishDelayedInstallation interface,
which unconditionally finishes delayed extension installations. The
current usages do not correctly determine whether the delayed installs
are ready to be installed, e.g. they do not verify that shared module
dependencies specified by the extension are satisfied (it seems that
current calling sites assume the only reason an update can be delayed
is due to the extension being active at the time of the last
installation attempt).
This CL replaces FinishDelayedInstallation with
AttemptDelayedInstallation which runs ShouldDelayExtensionInstall
again before finishing the delayed installation - the behavior is
the same as existing MaybeFinishDelayedInstallation private method,
so the MaybeFinishDelayedInstallation calls are replace with
AttemptDelayedInstallation as well.
Note that this makes check for whether primary app has pending delayed
install before running updater during kiosk app launch unnecessary,
thus the check is removed. Previously, given that the extension update
check was run with install_immediately set, running update checks at
that point would risk installing the app's installation delayed due to
non-compliant platform (which is undesirable behavior) - this cl fixes
the issue this code was working around.
Bug: 800020
Change-Id: Ic9bba8aa9d5c8a974fba3a9f5331fe24a21e86d1
Reviewed-on: https://chromium-review.googlesource.com/853358
Reviewed-by: Devlin <[email protected]>
Reviewed-by: Xiyuan Xia <[email protected]>
Commit-Queue: Toni Barzic <[email protected]>
Cr-Commit-Position: refs/heads/master@{#528110}diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 8335f20..9dbb7864 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -587,6 +587,42 @@
DISALLOW_COPY_AND_ASSIGN(MockUpdateProviderVisitor);
};
+struct MockExtensionRegistryObserver
+ : public extensions::ExtensionRegistryObserver {
+ MockExtensionRegistryObserver() = default;
+ ~MockExtensionRegistryObserver() override = default;
+
+ // extensions::ExtensionRegistryObserver:
+ void OnExtensionLoaded(content::BrowserContext* browser_context,
+ const Extension* extension) override {
+ last_extension_loaded = extension->id();
+ }
+ void OnExtensionUnloaded(content::BrowserContext* browser_context,
+ const Extension* extension,
+ UnloadedExtensionReason reason) override {
+ last_extension_unloaded = extension->id();
+ }
+ void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
+ const Extension* extension,
+ bool is_update,
+ const std::string& old_name) override {
+ last_extension_installed = extension->id();
+ }
+ void OnExtensionUninstalled(content::BrowserContext* browser_context,
+ const Extension* extension,
+ extensions::UninstallReason reason) override {
+ last_extension_uninstalled = extension->id();
+ }
+
+ std::string last_extension_loaded;
+ std::string last_extension_unloaded;
+ std::string last_extension_installed;
+ std::string last_extension_uninstalled;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockExtensionRegistryObserver);
+};
+
class ExtensionServiceTest
: public extensions::ExtensionServiceTestWithInstall {
public:
@@ -1001,6 +1037,105 @@
EXPECT_TRUE(service()->pending_extension_manager()->Remove(pending_id));
}
+// Tests that reloading extension with a install delayed due to pending imports
+// reloads currently installed extension version, rather than installing the
+// delayed install.
+TEST_F(ExtensionServiceTest, ReloadExtensionWithPendingImports) {
+ InitializeEmptyExtensionService();
+
+ // Wait for GarbageCollectExtensions task to complete.
+ content::RunAllTasksUntilIdle();
+
+ const base::FilePath base_path =
+ data_dir()
+ .AppendASCII("pending_updates_with_imports")
+ .AppendASCII("updated_with_imports");
+
+ const base::FilePath pem_path = base_path.AppendASCII("update.pem");
+
+ // Initially installed version - the version with no imports.
+ const base::FilePath installed_path = base_path.AppendASCII("1.0.0");
+
+ // The updated version - has import that is not satisfied (due to the imported
+ // extension not being installed).
+ const base::FilePath updated_path = base_path.AppendASCII("2.0.0");
+
+ ASSERT_TRUE(base::PathExists(pem_path));
+ ASSERT_TRUE(base::PathExists(installed_path));
+ ASSERT_TRUE(base::PathExists(updated_path));
+
+ ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+
+ // Install version 1.
+ const Extension* extension =
+ PackAndInstallCRX(installed_path, pem_path, INSTALL_NEW,
+ Extension::FROM_WEBSTORE, Manifest::Location::INTERNAL);
+ content::RunAllTasksUntilIdle();
+ ASSERT_TRUE(extension);
+ const std::string id = extension->id();
+
+ ASSERT_TRUE(registry()->enabled_extensions().Contains(id));
+ EXPECT_EQ("1.0.0", extension->VersionString());
+
+ // No pending extensions at this point.
+ EXPECT_FALSE(service()->pending_extension_manager()->HasPendingExtensions());
+
+ // Update to version 2 that adds an unsatisfied import.
+ PackCRXAndUpdateExtension(id, updated_path, pem_path, ENABLED);
+ content::RunAllTasksUntilIdle();
+
+ EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
+ extension = service()->GetInstalledExtension(id);
+ ASSERT_TRUE(extension);
+
+ // The extension update should be delayed at this point - the old version
+ // should still be installed.
+ EXPECT_EQ("1.0.0", extension->VersionString());
+
+ // Make sure the import started for the extension with a dependency.
+ EXPECT_TRUE(prefs->GetDelayedInstallInfo(id));
+ EXPECT_EQ(ExtensionPrefs::DELAY_REASON_WAIT_FOR_IMPORTS,
+ prefs->GetDelayedInstallReason(id));
+
+ const std::string pending_id(32, 'e');
+ EXPECT_TRUE(service()->pending_extension_manager()->IsIdPending(pending_id));
+
+ MockExtensionRegistryObserver reload_observer;
+ registry()->AddObserver(&reload_observer);
+
+ // Reload the extension, and verify that the installed version does not
+ // change.
+ service()->ReloadExtension(id);
+ EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
+ EXPECT_EQ(id, reload_observer.last_extension_loaded);
+ EXPECT_EQ(id, reload_observer.last_extension_unloaded);
+ registry()->RemoveObserver(&reload_observer);
+
+ extension = service()->GetInstalledExtension(id);
+ ASSERT_TRUE(extension);
+ EXPECT_EQ("1.0.0", extension->VersionString());
+
+ // The update should remain delayed, with the import pending.
+ EXPECT_TRUE(prefs->GetDelayedInstallInfo(id));
+ EXPECT_EQ(ExtensionPrefs::DELAY_REASON_WAIT_FOR_IMPORTS,
+ prefs->GetDelayedInstallReason(id));
+
+ // Attempt delayed installed - similar to reloading the extension, the update
+ // should remain delayed.
+ EXPECT_FALSE(service()->FinishDelayedInstallationIfReady(id, true));
+
+ extension = service()->GetInstalledExtension(id);
+ ASSERT_TRUE(extension);
+ EXPECT_EQ("1.0.0", extension->VersionString());
+ EXPECT_EQ(ExtensionPrefs::DELAY_REASON_WAIT_FOR_IMPORTS,
+ prefs->GetDelayedInstallReason(id));
+ EXPECT_TRUE(service()->pending_extension_manager()->IsIdPending(pending_id));
+
+ // Remove the pending install because the pending extension manager's
+ // ability to download and install extensions is not important for this test.
+ EXPECT_TRUE(service()->pending_extension_manager()->Remove(pending_id));
+}
+
// Tests that installation fails with extensions disabled.
TEST_F(ExtensionServiceTest, InstallExtensionsWithExtensionsDisabled) {
InitializeExtensionServiceWithExtensionsDisabled();
@@ -1064,25 +1199,6 @@
// TODO(erikkay): add tests for upgrade cases.
}
-struct MockExtensionRegistryObserver
- : public extensions::ExtensionRegistryObserver {
- void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
- const Extension* extension,
- bool is_update,
- const std::string& old_name) override {
- last_extension_installed = extension->id();
- }
-
- void OnExtensionUninstalled(content::BrowserContext* browser_context,
- const Extension* extension,
- extensions::UninstallReason reason) override {
- last_extension_uninstalled = extension->id();
- }
-
- std::string last_extension_installed;
- std::string last_extension_uninstalled;
-};
-
// Test that correct notifications are sent to ExtensionRegistryObserver on
// extension install and uninstall.
TEST_F(ExtensionServiceTest, InstallObserverNotified) {
