Bug 1593226 - [remote] Create a single context observer per content session. r=remote-protocol-reviewers,maja_zf

Domains would have created their own instance of the context observer,
which results in duplicated event listeners and observer notifications
to be registered.

This is still not ideal for the observer notifications, which should be
registered only once, but still an improvement for now. Bug 1635568 will
finally fix that.

Differential Revision: https://phabricator.services.mozilla.com/D74632
This commit is contained in:
Henrik Skupin 2020-05-14 14:55:36 +00:00
parent ec9e0e8772
commit f18df4ec89
4 changed files with 37 additions and 25 deletions

View File

@ -10,19 +10,9 @@ const { Domain } = ChromeUtils.import(
"chrome://remote/content/domains/Domain.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"ContextObserver",
"chrome://remote/content/observers/ContextObserver.jsm"
);
class ContentProcessDomain extends Domain {
destructor() {
super.destructor();
if (this._contextObserver) {
this._contextObserver.destructor();
}
}
// helpers
@ -38,11 +28,4 @@ class ContentProcessDomain extends Domain {
get chromeEventHandler() {
return this.docShell.chromeEventHandler;
}
get contextObserver() {
if (!this._contextObserver) {
this._contextObserver = new ContextObserver(this.chromeEventHandler);
}
return this._contextObserver;
}
}

View File

@ -43,12 +43,12 @@ class Page extends ContentProcessDomain {
this._onFrameNavigated = this._onFrameNavigated.bind(this);
this._onScriptLoaded = this._onScriptLoaded.bind(this);
this.contextObserver.on("script-loaded", this._onScriptLoaded);
this.session.contextObserver.on("script-loaded", this._onScriptLoaded);
}
destructor() {
this.setLifecycleEventsEnabled({ enabled: false });
this.contextObserver.off("script-loaded", this._onScriptLoaded);
this.session.contextObserver.off("script-loaded", this._onScriptLoaded);
this.disable();
super.destructor();
@ -59,7 +59,10 @@ class Page extends ContentProcessDomain {
async enable() {
if (!this.enabled) {
this.enabled = true;
this.contextObserver.on("frame-navigated", this._onFrameNavigated);
this.session.contextObserver.on(
"frame-navigated",
this._onFrameNavigated
);
this.chromeEventHandler.addEventListener("readystatechange", this, {
mozSystemGroup: true,
@ -87,7 +90,10 @@ class Page extends ContentProcessDomain {
disable() {
if (this.enabled) {
this.contextObserver.off("frame-navigated", this._onFrameNavigated);
this.session.contextObserver.off(
"frame-navigated",
this._onFrameNavigated
);
this.chromeEventHandler.removeEventListener("readystatechange", this, {
mozSystemGroup: true,

View File

@ -62,14 +62,20 @@ class Runtime extends ContentProcessDomain {
this._onContextCreated = this._onContextCreated.bind(this);
this._onContextDestroyed = this._onContextDestroyed.bind(this);
// TODO Bug 1602083
this.contextObserver.on("context-created", this._onContextCreated);
this.contextObserver.on("context-destroyed", this._onContextDestroyed);
this.session.contextObserver.on("context-created", this._onContextCreated);
this.session.contextObserver.on(
"context-destroyed",
this._onContextDestroyed
);
}
destructor() {
this.disable();
this.contextObserver.off("context-created", this._onContextCreated);
this.contextObserver.off("context-destroyed", this._onContextDestroyed);
this.session.contextObserver.off("context-created", this._onContextCreated);
this.session.contextObserver.off(
"context-destroyed",
this._onContextDestroyed
);
super.destructor();
}

View File

@ -13,6 +13,12 @@ const { DomainCache } = ChromeUtils.import(
"chrome://remote/content/domains/DomainCache.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"ContextObserver",
"chrome://remote/content/observers/ContextObserver.jsm"
);
class ContentProcessSession {
constructor(messageManager, browsingContext, content, docShell) {
this.messageManager = messageManager;
@ -29,11 +35,22 @@ class ContentProcessSession {
}
destructor() {
this._contextObserver?.destructor();
this.messageManager.removeMessageListener("remote:request", this);
this.messageManager.removeMessageListener("remote:destroy", this);
this.domains.clear();
}
get contextObserver() {
if (!this._contextObserver) {
this._contextObserver = new ContextObserver(
this.docShell.chromeEventHandler
);
}
return this._contextObserver;
}
// Domain event listener
onEvent(eventName, params) {