Remove NOTIFICATION_EXTENSION_ENABLED.
Enablement can happen for the following 3 cases:
1. New extension is added
2. User enables a disabled extension
3. An extension is reloaded or a terminated extension is reloaded
It seems we used this in two classes in
a) theme_service.cc and
b) event_router.cc
a) For themes, this can be simplified to load the theme on
OnExtensionLoaded. Themes don't have process and it can't be
terminated. We still have to remember whether the load is due
to a theme update or not (OnExtensionWillBeInstalled). It is
possible to avoid this by remembering both current theme's
id (already done) and version in the prefs. This CL does not do
that.
b) The primary reason of EventRouter code is to make sure we
are aware of new lazy events on extension upgrade, and to dispatch
onInstalled event. The CL wraps the logic to use
OnExtensionWillBeInstalled + OnExtensionLoaded into a separate
class: LazyEventDispatchUtil.
However, there are also 2 other reasons for EventRouter code in b):
b1) We also need to reconnect devtools to a reloaded event
page. That was happening through event_router. Pull that logic into
extension_service as extension_service is already responsible
for remembering orphaned devtools.
b2) A component extension reload does not go through
ExtensionRegistry::TriggerOnInstalled, so it doesn't see any
OnExtensionWillBeInstalled. Couple the logic with b1).
[email protected] for added test in app_browsertest.cc
BUG=723754, 411569
Test=No visible changes expected.
Review-Url: https://codereview.chromium.org/2893693002
Cr-Commit-Position: refs/heads/master@{#477357}
diff --git a/chrome/browser/apps/app_browsertest.cc b/chrome/browser/apps/app_browsertest.cc
index 0f924731..78cf7ba7 100644
--- a/chrome/browser/apps/app_browsertest.cc
+++ b/chrome/browser/apps/app_browsertest.cc
@@ -937,13 +937,30 @@
ASSERT_TRUE(extension);
ASSERT_TRUE(GetFirstAppWindow());
- // Now tell the app to reload itself
+ // Now tell the app to reload itself.
ExtensionTestMessageListener launched_listener2("Launched", false);
launched_listener.Reply("reload");
ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
ASSERT_TRUE(GetFirstAppWindow());
}
+// Tests that reloading a component app loads its (lazy) background page.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
+ ComponentReloadLoadsLazyBackgroundPage) {
+ ExtensionTestMessageListener launched_listener("Launched", true);
+ const Extension* component_app = LoadExtensionAsComponentWithManifest(
+ test_data_dir_.AppendASCII("platform_apps")
+ .AppendASCII("component_reload"),
+ FILE_PATH_LITERAL("manifest.json"));
+ ASSERT_TRUE(component_app);
+ ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
+ // Now tell the app to reload itself.
+ ExtensionTestMessageListener launched_listener2("Launched", false);
+ launched_listener.Reply("reload");
+ ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
+}
+
namespace {
// Utility class to ensure extension installation does or does not occur in
diff --git a/chrome/browser/extensions/api/input_ime/input_ime_api.h b/chrome/browser/extensions/api/input_ime/input_ime_api.h
index 21ed2f8..5cc78e4 100644
--- a/chrome/browser/extensions/api/input_ime/input_ime_api.h
+++ b/chrome/browser/extensions/api/input_ime/input_ime_api.h
@@ -18,6 +18,7 @@
#include "chrome/browser/ui/input_method/input_method_engine_base.h"
#include "components/keyed_service/core/keyed_service.h"
#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
#include "extensions/browser/browser_context_keyed_api_factory.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_function.h"
@@ -34,10 +35,6 @@
class Profile;
-namespace content {
-class NotificationRegistrar;
-}
-
namespace ui {
class IMEEngineHandlerInterface;
diff --git a/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.h b/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.h
index 4881532..9763eec 100644
--- a/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.h
+++ b/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.h
@@ -13,6 +13,7 @@
#include "base/macros.h"
#include "base/scoped_observer.h"
#include "components/sync/device_info/device_info_tracker.h"
+#include "content/public/browser/notification_registrar.h"
#include "extensions/browser/browser_context_keyed_api_factory.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_registry_observer.h"
@@ -21,7 +22,6 @@
namespace content {
class BrowserContext;
-class NotificationRegistrar;
} // namespace content
namespace extensions {
diff --git a/chrome/browser/extensions/events_apitest.cc b/chrome/browser/extensions/events_apitest.cc
index 8787df1..143a82d 100644
--- a/chrome/browser/extensions/events_apitest.cc
+++ b/chrome/browser/extensions/events_apitest.cc
@@ -5,8 +5,12 @@
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/test/base/ui_test_utils.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/scoped_ignore_content_verifier_for_test.h"
+#include "extensions/test/result_catcher.h"
namespace extensions {
@@ -65,4 +69,146 @@
event_router->ExtensionHasEventListener(id, "webNavigation.onCompleted"));
}
+class EventsApiTest : public ExtensionApiTest {
+ public:
+ EventsApiTest() {}
+
+ protected:
+ void SetUpOnMainThread() override {
+ ExtensionApiTest::SetUpOnMainThread();
+ EXPECT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
+ }
+
+ struct ExtensionCRXData {
+ std::string unpacked_relative_path;
+ base::FilePath crx_path;
+ explicit ExtensionCRXData(const std::string& unpacked_relative_path)
+ : unpacked_relative_path(unpacked_relative_path) {}
+ };
+
+ void SetUpCRX(const std::string& root_dir,
+ const std::string& pem_filename,
+ std::vector<ExtensionCRXData>* crx_data_list) {
+ const base::FilePath test_dir = test_data_dir_.AppendASCII(root_dir);
+ const base::FilePath pem_path = test_dir.AppendASCII(pem_filename);
+ for (ExtensionCRXData& crx_data : *crx_data_list) {
+ crx_data.crx_path = PackExtensionWithOptions(
+ test_dir.AppendASCII(crx_data.unpacked_relative_path),
+ scoped_temp_dir_.GetPath().AppendASCII(
+ crx_data.unpacked_relative_path + ".crx"),
+ pem_path, base::FilePath());
+ }
+ }
+
+ private:
+ base::ScopedTempDir scoped_temp_dir_;
+ ScopedIgnoreContentVerifierForTest ignore_content_verification_;
+
+ DISALLOW_COPY_AND_ASSIGN(EventsApiTest);
+};
+
+// Tests that updating an extension sends runtime.onInstalled event to the
+// updated extension.
+IN_PROC_BROWSER_TEST_F(EventsApiTest, ExtensionUpdateSendsOnInstalledEvent) {
+ std::vector<ExtensionCRXData> data;
+ data.emplace_back("v1");
+ data.emplace_back("v2");
+ SetUpCRX("lazy_events/on_installed", "pem.pem", &data);
+
+ ExtensionId extension_id;
+ {
+ // Install version 1 of the extension and expect runtime.onInstalled.
+ ResultCatcher catcher;
+ const int expected_change = 1;
+ const Extension* extension_v1 =
+ InstallExtension(data[0].crx_path, expected_change);
+ extension_id = extension_v1->id();
+ ASSERT_TRUE(extension_v1);
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+ {
+ // Update to version 2, also expect runtime.onInstalled.
+ ResultCatcher catcher;
+ const int expected_change = 0;
+ const Extension* extension_v2 =
+ UpdateExtension(extension_id, data[1].crx_path, expected_change);
+ ASSERT_TRUE(extension_v2);
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+}
+
+// Tests that if updating an extension makes the extension disabled (due to
+// permissions increase), then enabling the extension fires runtime.onInstalled
+// correctly to the updated extension.
+IN_PROC_BROWSER_TEST_F(EventsApiTest,
+ UpdateDispatchesOnInstalledAfterEnablement) {
+ std::vector<ExtensionCRXData> data;
+ data.emplace_back("v1");
+ data.emplace_back("v2");
+ SetUpCRX("lazy_events/on_installed_permissions_increase", "pem.pem", &data);
+
+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+ ExtensionId extension_id;
+ {
+ // Install version 1 of the extension and expect runtime.onInstalled.
+ ResultCatcher catcher;
+ const int expected_change = 1;
+ const Extension* extension_v1 =
+ InstallExtension(data[0].crx_path, expected_change);
+ extension_id = extension_v1->id();
+ ASSERT_TRUE(extension_v1);
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+ {
+ // Update to version 2, which will be disabled due to permissions increase.
+ ResultCatcher catcher;
+ const int expected_change = -1; // Expect extension to be disabled.
+ ASSERT_FALSE(
+ UpdateExtension(extension_id, data[1].crx_path, expected_change));
+
+ const Extension* extension_v2 =
+ registry->disabled_extensions().GetByID(extension_id);
+ ASSERT_TRUE(extension_v2);
+ // Enable the extension.
+ extension_service()->GrantPermissionsAndEnableExtension(extension_v2);
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+}
+
+// Tests that if an extension's updated version has a new lazy listener, it
+// fires properly after the update.
+IN_PROC_BROWSER_TEST_F(EventsApiTest, NewlyIntroducedListener) {
+ std::vector<ExtensionCRXData> data;
+ data.emplace_back("v1");
+ data.emplace_back("v2");
+ SetUpCRX("lazy_events/new_event_in_new_version", "pem.pem", &data);
+
+ ExtensionId extension_id;
+ {
+ // Install version 1 of the extension.
+ ResultCatcher catcher;
+ const int expected_change = 1;
+ const Extension* extension_v1 =
+ InstallExtension(data[0].crx_path, expected_change);
+ EXPECT_TRUE(extension_v1);
+ extension_id = extension_v1->id();
+ ASSERT_TRUE(extension_v1);
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+ {
+ // Update to version 2, that has tabs.onCreated event listener.
+ ResultCatcher catcher;
+ const int expected_change = 0;
+ const Extension* extension_v2 =
+ UpdateExtension(extension_id, data[1].crx_path, expected_change);
+ ASSERT_TRUE(extension_v2);
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), GURL(url::kAboutBlankURL),
+ WindowOpenDisposition::NEW_BACKGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+ // Expect tabs.onCreated to fire.
+ EXPECT_TRUE(catcher.GetNextResult());
+ }
+}
+
} // namespace extensions
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 8654385..a77c73d 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -79,6 +79,7 @@
#include "extensions/browser/extensions_browser_client.h"
#include "extensions/browser/external_install_info.h"
#include "extensions/browser/install_flag.h"
+#include "extensions/browser/lazy_background_task_queue.h"
#include "extensions/browser/renderer_startup_helper.h"
#include "extensions/browser/runtime_data.h"
#include "extensions/browser/uninstall_reason.h"
@@ -144,6 +145,8 @@
// TODO(samuong): Remove this in M58 (see comment in ExtensionService::Init).
const char kDeprecatedLoadComponentExtension[] = "load-component-extension";
+void DoNothingWithExtensionHost(extensions::ExtensionHost* host) {}
+
} // namespace
// ExtensionService.
@@ -924,11 +927,7 @@
NotifyExtensionLoaded(extension);
- // Notify listeners that the extension was enabled.
- content::NotificationService::current()->Notify(
- extensions::NOTIFICATION_EXTENSION_ENABLED,
- content::Source<Profile>(profile_),
- content::Details<const Extension>(extension));
+ MaybeSpinUpLazyBackgroundPage(extension);
}
void ExtensionService::DisableExtension(const std::string& extension_id,
@@ -2557,3 +2556,29 @@
OnBlacklistUpdated();
}
+
+void ExtensionService::MaybeSpinUpLazyBackgroundPage(
+ const Extension* extension) {
+ if (!extensions::BackgroundInfo::HasLazyBackgroundPage(extension))
+ return;
+
+ // For orphaned devtools, we will reconnect devtools to it later in
+ // DidCreateRenderViewForBackgroundPage().
+ OrphanedDevTools::iterator iter = orphaned_dev_tools_.find(extension->id());
+ bool has_orphaned_dev_tools = iter != orphaned_dev_tools_.end();
+
+ // Reloading component extension does not trigger install, so RuntimeAPI won't
+ // be able to detect its loading. Therefore, we need to spin up its lazy
+ // background page.
+ bool is_component_extension =
+ Manifest::IsComponentLocation(extension->location());
+
+ if (!has_orphaned_dev_tools && !is_component_extension)
+ return;
+
+ // Wake up the event page by posting a dummy task.
+ extensions::LazyBackgroundTaskQueue* queue =
+ extensions::LazyBackgroundTaskQueue::Get(profile_);
+ queue->AddPendingTask(profile_, extension->id(),
+ base::Bind(&DoNothingWithExtensionHost));
+}
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 1c5005e..2dca7c3 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -587,6 +587,10 @@
// Called when the initial extensions load has completed.
void OnInstalledExtensionsLoaded();
+ // Upon reloading an extension, spins up its lazy background page if
+ // necessary.
+ void MaybeSpinUpLazyBackgroundPage(const extensions::Extension* extension_id);
+
const base::CommandLine* command_line_ = nullptr;
// The normal profile associated with this ExtensionService.
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
index 34cd105..0b7525c 100644
--- a/chrome/browser/themes/theme_service.cc
+++ b/chrome/browser/themes/theme_service.cc
@@ -51,6 +51,7 @@
#include "ui/native_theme/native_theme.h"
#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "base/scoped_observer.h"
#include "extensions/browser/extension_registry_observer.h"
#endif
@@ -162,35 +163,45 @@
class ThemeService::ThemeObserver
: public extensions::ExtensionRegistryObserver {
public:
- explicit ThemeObserver(ThemeService* service) : theme_service_(service) {
- extensions::ExtensionRegistry::Get(theme_service_->profile_)
- ->AddObserver(this);
+ explicit ThemeObserver(ThemeService* service)
+ : theme_service_(service), extension_registry_observer_(this) {
+ extension_registry_observer_.Add(
+ extensions::ExtensionRegistry::Get(theme_service_->profile_));
}
~ThemeObserver() override {
- extensions::ExtensionRegistry::Get(theme_service_->profile_)
- ->RemoveObserver(this);
}
private:
+ // extensions::ExtensionRegistryObserver:
void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
const extensions::Extension* extension,
bool is_update,
const std::string& old_name) override {
if (extension->is_theme()) {
- // The theme may be initially disabled. Wait till it is loaded (if ever).
+ // Remember ID of the newly installed theme.
theme_service_->installed_pending_load_id_ = extension->id();
}
}
void OnExtensionLoaded(content::BrowserContext* browser_context,
const extensions::Extension* extension) override {
- if (extension->is_theme() &&
+ if (!extension->is_theme())
+ return;
+
+ bool is_new_version =
theme_service_->installed_pending_load_id_ != kDefaultThemeID &&
- theme_service_->installed_pending_load_id_ == extension->id()) {
- theme_service_->SetTheme(extension);
- }
+ theme_service_->installed_pending_load_id_ == extension->id();
theme_service_->installed_pending_load_id_ = kDefaultThemeID;
+
+ // Do not load already loaded theme.
+ if (!is_new_version && extension->id() == theme_service_->GetThemeID())
+ return;
+
+ // Set the new theme during extension load:
+ // This includes: a) installing a new theme, b) enabling a disabled theme.
+ // We shouldn't get here for the update of a disabled theme.
+ theme_service_->DoSetTheme(extension, !is_new_version);
}
void OnExtensionUnloaded(
@@ -206,6 +217,12 @@
}
ThemeService* theme_service_;
+
+ ScopedObserver<extensions::ExtensionRegistry,
+ extensions::ExtensionRegistryObserver>
+ extension_registry_observer_;
+
+ DISALLOW_COPY_AND_ASSIGN(ThemeObserver);
};
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
@@ -261,12 +278,6 @@
content::Source<Profile>(profile_));
OnExtensionServiceReady();
break;
- case extensions::NOTIFICATION_EXTENSION_ENABLED: {
- const Extension* extension = Details<const Extension>(details).ptr();
- if (extension->is_theme())
- DoSetTheme(extension, true);
- break;
- }
default:
NOTREACHED();
}
@@ -779,13 +790,9 @@
}
#if BUILDFLAG(ENABLE_EXTENSIONS)
- theme_observer_.reset(new ThemeObserver(this));
+ theme_observer_ = base::MakeUnique<ThemeObserver>(this);
#endif
- registrar_.Add(this,
- extensions::NOTIFICATION_EXTENSION_ENABLED,
- content::Source<Profile>(profile_));
-
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, base::Bind(&ThemeService::RemoveUnusedThemes,
weak_ptr_factory_.GetWeakPtr(), false),
