initialProcessData has the unfortunate side-effect of sending an entire copy of all of its data to all content processes, and eagerly decoding it. For the extension framework, this means that we wind up loading an entire copy of all of our schema data, and of every extension's manifest and locale data, into every process, even if we'll never need it. The sharedData helper allows us to store an encoded copy of that data in a shared memory region, and clone it into the current process only when we need it, which can be a significant savings. For screenshots alone, it saves about 15K on locale and manifest data per content process, plus the size we save on not copying schema data.

Comment on attachment 8987388 [details] Bug 1470783: Migrate extensions framework to use sharedData for cross-process data. https://reviewboard.mozilla.org/r/252622/#review259044 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/components/extensions/ExtensionParent.jsm:156 (Diff revision 1) > for (let url of schemaURLs) { > promises.push(Schemas.load(url)); > } > - return Promise.all(promises); > + return Promise.all(promises).then(() => { > + Schemas.broadcastSchemas(); > + }) Error: Missing semicolon. [eslint: semi]

Comment on attachment 8987388 [details] Bug 1470783: Migrate extensions framework to use sharedData for cross-process data. https://reviewboard.mozilla.org/r/252622/#review260848 Far simplified schemas handling, nice! ::: toolkit/components/extensions/Extension.jsm:1549 (Diff revision 1) > - principal: this.principal, > optionalPermissions: this.manifest.optional_permissions, > + }; > + } > + > + serializeExtended() { What's the reason for separating this? I understand these are only needed in the extension process, but other than `childModules`, they don't seem particularly big to make it worth the "savings". Please either rename to something related to the purpose, or add a comment. ::: toolkit/components/extensions/Extension.jsm:1835 (Diff revision 1) > } > > - let data = Services.ppmm.initialProcessData; > - data["Extension:Extensions"] = data["Extension:Extensions"].filter(e => e.id !== this.id); > + activeExtensionIDs.delete(this.id); > + sharedData.set("extensions/activeIDs", activeExtensionIDs); > + > + for (let key of this.sharedDataKeys) { "sharedDataKeys" sounds wrong, and I'm not sure it's worth keeping track of keys instead of just always deleting all (5) of them. ::: toolkit/components/extensions/Schemas.jsm:3015 (Diff revision 1) > + > // A separate map of schema JSON which should be available in all > // content processes. > contentSchemaJSON: new Map(), > > + privilegedSchemaJSON: new Map(), Add or update the comment above please. ::: toolkit/components/extensions/Schemas.jsm:3083 (Diff revision 1) > if (this._rootSchema) { > throw new Error("Schema loaded after root schema populated"); > } > }, > > + broadcastSchemas() { "setSharedSchemas" perhaps? Or at least add an explicit `flush()` to make the code match the name. ::: toolkit/components/extensions/extension-process-script.js:34 (Diff revision 1) > getInnerWindowID, > } = ExtensionUtils; > > +const {sharedData} = Services.cpmm; > + > +function getData(extension, key = "") { nit: called more often with just the id ::: toolkit/components/extensions/extension-process-script.js:334 (Diff revision 1) > webAccessibleResources = extension.webAccessibleResources.map(host => new MatchGlob(host)); > } > > + let {backgroundScripts} = extension; > + if (!backgroundScripts && WebExtensionPolicy.isExtensionProcess) { > + ({backgroundScripts} = getData(extension, "extendedData") || {}); ugh

Comment on attachment 8987388 [details] Bug 1470783: Migrate extensions framework to use sharedData for cross-process data. https://reviewboard.mozilla.org/r/252622/#review260848 > What's the reason for separating this? > > I understand these are only needed in the extension process, but other than `childModules`, they don't seem particularly big to make it worth the "savings". > > > Please either rename to something related to the purpose, or add a comment. It saves about 2K per process for Screenshots. More for other extensions. > "sharedDataKeys" sounds wrong, and I'm not sure it's worth keeping track of keys instead of just always deleting all (5) of them. "just deleting all" of them requires knowing in advance what all of them are. It's much easier to just delete whichever keys we set. > ugh inorite?

https://hg.mozilla.org/integration/mozilla-inbound/rev/8074c985095c9951171311dac840684b915a57f6 Bug 1470783: Migrate extensions framework to use sharedData for cross-process data. r=zombie

Backed out 10 changesets (bug 1470783, bug 1463587) for causing multiple leakcheck failures on a CLOSED TREE Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8074c985095c9951171311dac840684b915a57f6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=187729601 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=187729601&repo=mozilla-inbound Backout push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6085b77ada2767a77a1fb9fa0bd9032855cfad10&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-classifiedState=unclassified

https://hg.mozilla.org/integration/mozilla-inbound/rev/71af1a8b7eadef8c377be4d8ae5b4402ea1013fc Bug 1470783: Migrate extensions framework to use sharedData for cross-process data. r=zombie

Hello, should manual testing be performed on this bug? if yes, please provide some info on how to validate this. Otherwise please set the qe-verify- flag. Thanks!