Commit fc5b6f05 authored by Fabian Sommer's avatar Fabian Sommer Committed by Commit Bot

Change function signature in ExtensionHostObserver

Remove constness of the ExtensionHostObserver passed in
OnExtensionHostDestroyed.
This allows scoped observers to stop observing a host that is getting
destroyed.

Fixed: 1086475
Change-Id: I9abc696fe164f26b4927cd7366e1cd04570dbe43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216211Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarAlexander Alekseev <alemate@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775228}
parent 4c457af2
...@@ -228,10 +228,9 @@ class ExtensionLoadObserver final ...@@ -228,10 +228,9 @@ class ExtensionLoadObserver final
// extensions::ExtensionHostObserver // extensions::ExtensionHostObserver
void OnExtensionHostDestroyed( void OnExtensionHostDestroyed(extensions::ExtensionHost* host) override {
const extensions::ExtensionHost* host) override { DCHECK(extension_host_observer_.IsObserving(host));
// TODO(crbug.com/1086475): Just stop observing |host| instead of all. extension_host_observer_.Remove(host);
extension_host_observer_.RemoveAll();
StopWaitingOnExtension(host->extension_id()); StopWaitingOnExtension(host->extension_id());
} }
......
...@@ -123,7 +123,7 @@ void ExtensionEventObserver::OnProcessManagerShutdown( ...@@ -123,7 +123,7 @@ void ExtensionEventObserver::OnProcessManagerShutdown(
} }
void ExtensionEventObserver::OnExtensionHostDestroyed( void ExtensionEventObserver::OnExtensionHostDestroyed(
const extensions::ExtensionHost* host) { extensions::ExtensionHost* host) {
auto it = keepalive_sources_.find(host); auto it = keepalive_sources_.find(host);
DCHECK(it != keepalive_sources_.end()); DCHECK(it != keepalive_sources_.end());
......
...@@ -82,7 +82,7 @@ class ExtensionEventObserver : public ProfileManagerObserver, ...@@ -82,7 +82,7 @@ class ExtensionEventObserver : public ProfileManagerObserver,
void OnProcessManagerShutdown(extensions::ProcessManager* manager) override; void OnProcessManagerShutdown(extensions::ProcessManager* manager) override;
// extensions::ExtensionHostObserver: // extensions::ExtensionHostObserver:
void OnExtensionHostDestroyed(const extensions::ExtensionHost* host) override; void OnExtensionHostDestroyed(extensions::ExtensionHost* host) override;
void OnBackgroundEventDispatched(const extensions::ExtensionHost* host, void OnBackgroundEventDispatched(const extensions::ExtensionHost* host,
const std::string& event_name, const std::string& event_name,
int event_id) override; int event_id) override;
......
...@@ -806,13 +806,9 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, CloseBackgroundPage) { ...@@ -806,13 +806,9 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, CloseBackgroundPage) {
const ExtensionHostDestructionObserver& other) = delete; const ExtensionHostDestructionObserver& other) = delete;
~ExtensionHostDestructionObserver() override = default; ~ExtensionHostDestructionObserver() override = default;
void OnExtensionHostDestroyed(const ExtensionHost* host) override { void OnExtensionHostDestroyed(ExtensionHost* host) override {
// TODO(devlin): It would be nice to ASSERT_TRUE(host_observer_.IsObserving(host));
// ASSERT_TRUE(host_observer_.IsObserving(host)); host_observer_.Remove(host);
// host_observer_.Remove(host);
// But we can't, because |host| is const. Work around it by just
// RemoveAll()ing.
host_observer_.RemoveAll();
run_loop_.QuitWhenIdle(); run_loop_.QuitWhenIdle();
} }
......
...@@ -87,8 +87,7 @@ class ExtensionHostDestructionObserver ...@@ -87,8 +87,7 @@ class ExtensionHostDestructionObserver
} }
// ExtensionHostObserver: // ExtensionHostObserver:
void OnExtensionHostDestroyed( void OnExtensionHostDestroyed(extensions::ExtensionHost* host) override {
const extensions::ExtensionHost* host) override {
if (host == host_) { if (host == host_) {
extension_host_observer_.Remove(host_); extension_host_observer_.Remove(host_);
run_loop_.Quit(); run_loop_.Quit();
......
...@@ -292,7 +292,7 @@ void ExtensionActionViewController::OnIconUpdated() { ...@@ -292,7 +292,7 @@ void ExtensionActionViewController::OnIconUpdated() {
} }
void ExtensionActionViewController::OnExtensionHostDestroyed( void ExtensionActionViewController::OnExtensionHostDestroyed(
const extensions::ExtensionHost* host) { extensions::ExtensionHost* host) {
OnPopupClosed(); OnPopupClosed();
} }
......
...@@ -105,7 +105,7 @@ class ExtensionActionViewController ...@@ -105,7 +105,7 @@ class ExtensionActionViewController
void OnIconUpdated() override; void OnIconUpdated() override;
// ExtensionHostObserver: // ExtensionHostObserver:
void OnExtensionHostDestroyed(const extensions::ExtensionHost* host) override; void OnExtensionHostDestroyed(extensions::ExtensionHost* host) override;
// Checks if the associated |extension| is still valid by checking its // Checks if the associated |extension| is still valid by checking its
// status in the registry. Since the OnExtensionUnloaded() notifications are // status in the registry. Since the OnExtensionUnloaded() notifications are
......
...@@ -21,7 +21,7 @@ class ExtensionHostObserver { ...@@ -21,7 +21,7 @@ class ExtensionHostObserver {
// ExtensionHost it's given. // ExtensionHost it's given.
// Called when an ExtensionHost is destroyed. // Called when an ExtensionHost is destroyed.
virtual void OnExtensionHostDestroyed(const ExtensionHost* host) {} virtual void OnExtensionHostDestroyed(ExtensionHost* host) {}
// Called when the ExtensionHost has finished the first load. // Called when the ExtensionHost has finished the first load.
virtual void OnExtensionHostDidStopFirstLoad(const ExtensionHost* host) {} virtual void OnExtensionHostDidStopFirstLoad(const ExtensionHost* host) {}
......
...@@ -108,14 +108,15 @@ void ExtensionBackgroundPageWaiter::OnExtensionHostDidStopFirstLoad( ...@@ -108,14 +108,15 @@ void ExtensionBackgroundPageWaiter::OnExtensionHostDidStopFirstLoad(
} }
void ExtensionBackgroundPageWaiter::OnExtensionHostDestroyed( void ExtensionBackgroundPageWaiter::OnExtensionHostDestroyed(
const ExtensionHost* host) { ExtensionHost* host) {
// This is only called while we're waiting for the host to be ready (since // This is only called while we're waiting for the host to be ready (since
// we remove ourselves as an observer when it's done). // we remove ourselves as an observer when it's done).
DCHECK(host_ready_run_loop_.running()); DCHECK(host_ready_run_loop_.running());
ASSERT_EQ(extension_->id(), host->extension_id()); ASSERT_EQ(extension_->id(), host->extension_id());
ADD_FAILURE() << "Extension host for " << extension_->name() ADD_FAILURE() << "Extension host for " << extension_->name()
<< "was destroyed before it finished loading."; << "was destroyed before it finished loading.";
extension_host_observer_.RemoveAll(); ASSERT_TRUE(extension_host_observer_.IsObserving(host));
extension_host_observer_.Remove(host);
} }
} // namespace extensions } // namespace extensions
...@@ -51,7 +51,7 @@ class ExtensionBackgroundPageWaiter : public ProcessManagerObserver, ...@@ -51,7 +51,7 @@ class ExtensionBackgroundPageWaiter : public ProcessManagerObserver,
// ExtensionHostObserver: // ExtensionHostObserver:
void OnExtensionHostDidStopFirstLoad(const ExtensionHost* host) override; void OnExtensionHostDidStopFirstLoad(const ExtensionHost* host) override;
void OnExtensionHostDestroyed(const ExtensionHost* host) override; void OnExtensionHostDestroyed(ExtensionHost* host) override;
content::BrowserContext* const browser_context_; content::BrowserContext* const browser_context_;
scoped_refptr<const Extension> extension_; scoped_refptr<const Extension> extension_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment