From 89ea21ef92978fece80e586d2952c8b2ef91384f Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Mon, 25 Mar 2019 07:37:21 -0600 Subject: [PATCH 01/11] Bug 1537657 - Detach from workers when navigating, r=loganfsmyth. --HG-- extra : histedit_source : 17ad5a2896c81d61d3b4a13fff28627f291e7f12%2C7b53c3a13ed1836f9769268ea4abbcbe0b01fd17 --- .../client/debugger/new/src/actions/navigation.js | 1 + .../debugger/new/src/actions/tests/navigation.spec.js | 11 ++++++----- .../debugger/new/src/client/firefox/commands.js | 9 ++++++++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/devtools/client/debugger/new/src/actions/navigation.js b/devtools/client/debugger/new/src/actions/navigation.js index 591bfdf62034..04ac7de9bf66 100644 --- a/devtools/client/debugger/new/src/actions/navigation.js +++ b/devtools/client/debugger/new/src/actions/navigation.js @@ -41,6 +41,7 @@ export function willNavigate(event: Object) { clearASTs(); clearScopes(); clearSources(); + client.detachWorkers(); dispatch(navigate(event.url)); }; } diff --git a/devtools/client/debugger/new/src/actions/tests/navigation.spec.js b/devtools/client/debugger/new/src/actions/tests/navigation.spec.js index f151747b5067..cebf5508fa21 100644 --- a/devtools/client/debugger/new/src/actions/tests/navigation.spec.js +++ b/devtools/client/debugger/new/src/actions/tests/navigation.spec.js @@ -27,7 +27,8 @@ const threadClient = { source: "function foo1() {\n const foo = 5; return foo;\n}", contentType: "text/javascript" }), - getBreakpointPositions: async () => ({}) + getBreakpointPositions: async () => ({}), + detachWorkers: () => {} }; describe("navigation", () => { @@ -67,7 +68,7 @@ describe("navigation", () => { }); it("navigation removes activeSearch 'project' value", async () => { - const { dispatch, getState } = createStore(); + const { dispatch, getState } = createStore(threadClient); dispatch(actions.setActiveSearch("project")); expect(getActiveSearch(getState())).toBe("project"); @@ -76,7 +77,7 @@ describe("navigation", () => { }); it("navigation clears the file-search query", async () => { - const { dispatch, getState } = createStore(); + const { dispatch, getState } = createStore(threadClient); dispatch(actions.setFileSearchQuery("foobar")); expect(getFileSearchQuery(getState())).toBe("foobar"); @@ -87,7 +88,7 @@ describe("navigation", () => { }); it("navigation clears the file-search results", async () => { - const { dispatch, getState } = createStore(); + const { dispatch, getState } = createStore(threadClient); const searchResults = [{ line: 1, ch: 3 }, { line: 3, ch: 2 }]; dispatch(actions.updateSearchResults(2, 3, searchResults)); @@ -109,7 +110,7 @@ describe("navigation", () => { }); it("navigation removes activeSearch 'file' value", async () => { - const { dispatch, getState } = createStore(); + const { dispatch, getState } = createStore(threadClient); dispatch(actions.setActiveSearch("file")); expect(getActiveSearch(getState())).toBe("file"); diff --git a/devtools/client/debugger/new/src/client/firefox/commands.js b/devtools/client/debugger/new/src/client/firefox/commands.js index e975de06e90c..8fa4fcb301c8 100644 --- a/devtools/client/debugger/new/src/client/firefox/commands.js +++ b/devtools/client/debugger/new/src/client/firefox/commands.js @@ -187,6 +187,12 @@ function waitForWorkers(shouldWait: boolean) { shouldWaitForWorkers = shouldWait; } +function detachWorkers() { + for (const thread of listWorkerThreadClients()) { + thread.detach(); + } +} + function maybeGenerateLogGroupId(options) { if (options.logValue && tabTarget.traits && tabTarget.traits.canRewind) { return { ...options, logGroupId: `logGroup-${Math.random()}` }; @@ -485,7 +491,8 @@ const clientCommands = { sendPacket, setSkipPausing, setEventListenerBreakpoints, - waitForWorkers + waitForWorkers, + detachWorkers }; export { setupCommands, clientCommands }; From 10ea70e9c19cc70e766133ccbdc9484397408f1e Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Mon, 25 Mar 2019 10:30:38 -0500 Subject: [PATCH 02/11] Bug 1487113 - Baldr: pass ownership of optimized encoding bytes (r=lth) --- js/src/jsapi.h | 5 ++++- js/src/shell/js.cpp | 9 ++++----- js/src/wasm/WasmJS.cpp | 6 +++--- js/src/wasm/WasmModule.cpp | 8 ++++---- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index c651c5cfa8d7..9814dcdf2394 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2147,6 +2147,9 @@ namespace JS { * as when it is later consumed. */ +using OptimizedEncodingBytes = js::Vector; +using UniqueOptimizedEncodingBytes = js::UniquePtr; + class OptimizedEncodingListener { protected: virtual ~OptimizedEncodingListener() {} @@ -2159,7 +2162,7 @@ class OptimizedEncodingListener { // SpiderMonkey may optionally call storeOptimizedEncoding() after it has // finished processing a streamed resource. - virtual void storeOptimizedEncoding(const uint8_t* bytes, size_t length) = 0; + virtual void storeOptimizedEncoding(UniqueOptimizedEncodingBytes bytes) = 0; }; class JS_PUBLIC_API StreamConsumer { diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index b61d60ae17a6..ed76559ef546 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -6909,9 +6909,8 @@ class StreamCacheEntry : public AtomicRefCounted, const Uint8Vector& bytes() const { return bytes_; } - void storeOptimizedEncoding(const uint8_t* srcBytes, - size_t srcLength) override { - MOZ_ASSERT(srcLength > 0); + void storeOptimizedEncoding(JS::UniqueOptimizedEncodingBytes src) override { + MOZ_ASSERT(src->length() > 0); // Tolerate races since a single StreamCacheEntry object can be used as // the source of multiple streaming compilations. @@ -6920,10 +6919,10 @@ class StreamCacheEntry : public AtomicRefCounted, return; } - if (!dstBytes->resize(srcLength)) { + if (!dstBytes->resize(src->length())) { return; } - memcpy(dstBytes->begin(), srcBytes, srcLength); + memcpy(dstBytes->begin(), src->begin(), src->length()); } bool hasOptimizedEncoding() const { return !optimized_.lock()->empty(); } diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index cdceb45d12a2..df787e127a03 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -474,11 +474,11 @@ struct MOZ_STACK_CLASS SerializeListener : JS::OptimizedEncodingListener { Bytes* serialized; explicit SerializeListener(Bytes* serialized) : serialized(serialized) {} - void storeOptimizedEncoding(const uint8_t* bytes, size_t length) override { + void storeOptimizedEncoding(JS::UniqueOptimizedEncodingBytes bytes) override { MOZ_ASSERT(!called); called = true; - if (serialized->resize(length)) { - memcpy(serialized->begin(), bytes, length); + if (serialized->resize(bytes->length())) { + memcpy(serialized->begin(), bytes->begin(), bytes->length()); } } }; diff --git a/js/src/wasm/WasmModule.cpp b/js/src/wasm/WasmModule.cpp index 66c051ee1b13..13b5388abf78 100644 --- a/js/src/wasm/WasmModule.cpp +++ b/js/src/wasm/WasmModule.cpp @@ -310,14 +310,14 @@ MutableModule Module::deserialize(const uint8_t* begin, size_t size, void Module::serialize(const LinkData& linkData, JS::OptimizedEncodingListener& listener) const { - Vector bytes; - if (!bytes.resize(serializedSize(linkData))) { + auto bytes = MakeUnique(); + if (!bytes || !bytes->resize(serializedSize(linkData))) { return; } - serialize(linkData, bytes.begin(), bytes.length()); + serialize(linkData, bytes->begin(), bytes->length()); - listener.storeOptimizedEncoding(bytes.begin(), bytes.length()); + listener.storeOptimizedEncoding(std::move(bytes)); } /* virtual */ From dc598c819754509972a24ef0a0e705d5e6733e1b Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Thu, 14 Mar 2019 15:20:22 -1000 Subject: [PATCH 03/11] Bug 1534786 - Add test. --HG-- extra : rebase_source : f66f930fa8494fc1a5a11a811acc053d7089390d --- .../debugger/new/test/mochitest/browser.ini | 1 + .../mochitest/browser_dbg-old-breakpoint.js | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js diff --git a/devtools/client/debugger/new/test/mochitest/browser.ini b/devtools/client/debugger/new/test/mochitest/browser.ini index 8f1ee320763b..ef68909f67ee 100644 --- a/devtools/client/debugger/new/test/mochitest/browser.ini +++ b/devtools/client/debugger/new/test/mochitest/browser.ini @@ -781,3 +781,4 @@ skip-if = true [browser_dbg-event-handler.js] [browser_dbg-eval-throw.js] [browser_dbg-sourceURL-breakpoint.js] +[browser_dbg-old-breakpoint.js] diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js new file mode 100644 index 000000000000..22bbf0d682a3 --- /dev/null +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js @@ -0,0 +1,90 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test that we show a breakpoint in the UI when there is an old pending +// breakpoint with an invalid original location. +add_task(async function() { + clearDebuggerPreferences(); + + const pending = { + bp1: { + location: { + sourceId: "", + sourceUrl: EXAMPLE_URL + "nowhere2.js", + line: 5 + }, + generatedLocation: { + sourceUrl: EXAMPLE_URL + "simple1.js", + line: 4 + }, + options: {}, + disabled: false + }, + bp2: { + location: { + sourceId: "", + sourceUrl: EXAMPLE_URL + "nowhere.js", + line: 5 + }, + generatedLocation: { + sourceUrl: EXAMPLE_URL + "simple3.js", + line: 2 + }, + options: {}, + disabled: false + }, + }; + asyncStorage.setItem("debugger.pending-breakpoints", pending); + + const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + "doc-scripts.html", "jsdebugger"); + const dbg = createDebuggerContext(toolbox); + + // Pending breakpoints are installed asynchronously, keep invoking the entry + // function until the debugger pauses. + await waitUntil(() => { + invokeInTab("main"); + return isPaused(dbg); + }); + + ok(true, "paused at unmapped breakpoint"); + await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 2); + ok(true, "unmapped breakpoints shown in UI"); +}); + +// Test that if we show a breakpoint with an old generated location, it is +// removed after we load the original source and find the new generated +// location. +add_task(async function() { + clearDebuggerPreferences(); + + const pending = { + bp1: { + location: { + sourceId: "", + sourceUrl: "webpack:///entry.js", + line: 15, + column: 0 + }, + generatedLocation: { + sourceUrl: EXAMPLE_URL + "sourcemaps/bundle.js", + line: 47, + column: 16 + }, + astLocation: {}, + options: {}, + disabled: false + }, + }; + asyncStorage.setItem("debugger.pending-breakpoints", pending); + + const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + "doc-sourcemaps.html", "jsdebugger"); + const dbg = createDebuggerContext(toolbox); + + await waitForState(dbg, state => { + const bps = dbg.selectors.getBreakpointsList(state); + return bps.length == 1 + && bps[0].location.sourceUrl.includes("entry.js") + && bps[0].location.line == 15; + }); + ok(true, "removed old breakpoint during sync"); +}); From 3c7d2391d54de91cea50a8864b287daee110b079 Mon Sep 17 00:00:00 2001 From: Eugen Sawin Date: Mon, 21 Jan 2019 23:14:48 +0100 Subject: [PATCH 04/11] Bug 1530789 - [1.2] Extend Content Blocking API: cryptomining protection. r=geckoview-reviewers,dimi,snorp,Ehsan Tags: #secure-revision Differential Revision: https://phabricator.services.mozilla.com/D21410 --- mobile/android/app/geckoview-prefs.js | 3 ++ .../mozilla/geckoview/ContentBlocking.java | 51 +++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/mobile/android/app/geckoview-prefs.js b/mobile/android/app/geckoview-prefs.js index 4c60d6298d14..a67fa5334024 100644 --- a/mobile/android/app/geckoview-prefs.js +++ b/mobile/android/app/geckoview-prefs.js @@ -42,3 +42,6 @@ pref("browser.safebrowsing.features.malware.update", true); // Enable Tracking Protection blocklist updates pref("browser.safebrowsing.features.trackingAnnotation.update", true); pref("browser.safebrowsing.features.trackingProtection.update", true); + +// Enable cryptomining protection blocklist updates +pref("browser.safebrowsing.features.cryptomining.update", true); diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java index 4e357772e131..941921a06ca1 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java @@ -75,6 +75,12 @@ public class ContentBlocking { "urlclassifier.trackingTable", ContentBlocking.catToAtPref( AT_TEST | AT_ANALYTIC | AT_SOCIAL | AT_AD)); + /* package */ final Pref mCm = new Pref( + "privacy.trackingprotection.cryptomining.enabled", false); + /* package */ final Pref mCmList = new Pref( + "urlclassifier.features.cryptomining.blacklistTables", + ContentBlocking.catToCmListPref(NONE)); + /* package */ final Pref mSbMalware = new Pref( "browser.safebrowsing.malware.enabled", true); /* package */ final Pref mSbPhishing = new Pref( @@ -130,6 +136,10 @@ public class ContentBlocking { */ public @NonNull Settings setCategories(final @Category int cat) { mAt.commit(ContentBlocking.catToAtPref(cat)); + + mCm.commit(ContentBlocking.catToCmPref(cat)); + mCmList.commit(ContentBlocking.catToCmListPref(cat)); + mSbMalware.commit(ContentBlocking.catToSbMalware(cat)); mSbPhishing.commit(ContentBlocking.catToSbPhishing(cat)); return this; @@ -141,7 +151,8 @@ public class ContentBlocking { * @return The categories of resources to be blocked. */ public @Category int getCategories() { - return ContentBlocking.listToCat(mAt.get()) + return ContentBlocking.atListToCat(mAt.get()) + | ContentBlocking.cmListToCat(mCmList.get()) | ContentBlocking.sbMalwareToCat(mSbMalware.get()) | ContentBlocking.sbPhishingToCat(mSbPhishing.get()); } @@ -195,7 +206,7 @@ public class ContentBlocking { @Retention(RetentionPolicy.SOURCE) @IntDef(flag = true, value = { NONE, AT_AD, AT_ANALYTIC, AT_SOCIAL, AT_CONTENT, - AT_ALL, AT_TEST, + AT_ALL, AT_TEST, AT_CRYPTOMINING, SB_MALWARE, SB_UNWANTED, SB_HARMFUL, SB_PHISHING }) /* package */ @interface Category {} @@ -228,11 +239,17 @@ public class ContentBlocking { */ public static final int AT_TEST = 1 << 5; + /** + * Block cryptocurrency miners. + */ + public static final int AT_CRYPTOMINING = 1 << 6; + /** * Block all known trackers. + * Blocks all {@link #AT_AD AT_*} types. */ public static final int AT_ALL = - AT_AD | AT_ANALYTIC | AT_SOCIAL | AT_CONTENT | AT_TEST; + AT_AD | AT_ANALYTIC | AT_SOCIAL | AT_CONTENT | AT_TEST | AT_CRYPTOMINING; // Safe browsing /** @@ -350,7 +367,8 @@ public class ContentBlocking { @Category int cats = NONE; if (matchedList != null) { - cats = ContentBlocking.listToCat(matchedList); + cats = ContentBlocking.atListToCat(matchedList) | + ContentBlocking.cmListToCat(matchedList); } else if (error != 0L) { cats = ContentBlocking.errorToCat(error); } @@ -383,6 +401,8 @@ public class ContentBlocking { private static final String ANALYTIC = "analytics-track-digest256"; private static final String SOCIAL = "social-track-digest256"; private static final String CONTENT = "content-track-digest256"; + private static final String CRYPTOMINING = + "base-cryptomining-track-digest256"; /* package */ static @Category int sbMalwareToCat(final boolean enabled) { return enabled ? (SB_MALWARE | SB_UNWANTED | SB_HARMFUL) @@ -427,7 +447,20 @@ public class ContentBlocking { return builder.substring(0, builder.length() - 1); } - /* package */ static @Category int listToCat(final String list) { + /* package */ static boolean catToCmPref(@Category int cat) { + return (cat & AT_CRYPTOMINING) != 0; + } + + /* package */ static String catToCmListPref(@Category int cat) { + StringBuilder builder = new StringBuilder(); + + if ((cat & AT_CRYPTOMINING) != 0) { + builder.append(CRYPTOMINING); + } + return builder.toString(); + } + + /* package */ static @Category int atListToCat(final String list) { int cat = 0; if (list.indexOf(TEST) != -1) { cat |= AT_TEST; @@ -447,6 +480,14 @@ public class ContentBlocking { return cat; } + /* package */ static @Category int cmListToCat(final String list) { + int cat = 0; + if (list.indexOf(CRYPTOMINING) != -1) { + cat |= AT_CRYPTOMINING; + } + return cat; + } + /* package */ static @Category int errorToCat(final long error) { // Match flags with XPCOM ErrorList.h. if (error == 0x805D001FL) { From 9e20531da6cd38171ff194e8b38e355ccf04d5c3 Mon Sep 17 00:00:00 2001 From: Eugen Sawin Date: Fri, 22 Mar 2019 20:46:04 +0000 Subject: [PATCH 05/11] Bug 1530789 - [2.0] Initialize the SafeBrowsing module only in the parent process. r=dimi Differential Revision: https://phabricator.services.mozilla.com/D21422 --- mobile/android/components/geckoview/GeckoViewStartup.js | 5 +++++ mobile/android/modules/geckoview/GeckoViewSettings.jsm | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/mobile/android/components/geckoview/GeckoViewStartup.js b/mobile/android/components/geckoview/GeckoViewStartup.js index c8f4cfd1f598..5014a75bfaa5 100644 --- a/mobile/android/components/geckoview/GeckoViewStartup.js +++ b/mobile/android/components/geckoview/GeckoViewStartup.js @@ -12,6 +12,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { GeckoViewTelemetryController: "resource://gre/modules/GeckoViewTelemetryController.jsm", L10nRegistry: "resource://gre/modules/L10nRegistry.jsm", Preferences: "resource://gre/modules/Preferences.jsm", + SafeBrowsing: "resource://gre/modules/SafeBrowsing.jsm", Services: "resource://gre/modules/Services.jsm", }); @@ -125,6 +126,10 @@ GeckoViewStartup.prototype = { ChromeUtils.import("resource://gre/modules/NotificationDB.jsm"); + // Initialize safe browsing module. This is required for content + // blocking features and manages blocklist downloads and updates. + SafeBrowsing.init(); + // Listen for global EventDispatcher messages EventDispatcher.instance.registerListener(this, ["GeckoView:ResetUserPrefs", diff --git a/mobile/android/modules/geckoview/GeckoViewSettings.jsm b/mobile/android/modules/geckoview/GeckoViewSettings.jsm index 26b4c6a14e27..e460a02cf165 100644 --- a/mobile/android/modules/geckoview/GeckoViewSettings.jsm +++ b/mobile/android/modules/geckoview/GeckoViewSettings.jsm @@ -9,10 +9,6 @@ var EXPORTED_SYMBOLS = ["GeckoViewSettings"]; const {GeckoViewModule} = ChromeUtils.import("resource://gre/modules/GeckoViewModule.jsm"); const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); -XPCOMUtils.defineLazyModuleGetters(this, { - SafeBrowsing: "resource://gre/modules/SafeBrowsing.jsm", -}); - XPCOMUtils.defineLazyGetter( this, "MOBILE_USER_AGENT", function() { @@ -45,11 +41,9 @@ const USER_AGENT_MODE_VR = 2; class GeckoViewSettings extends GeckoViewModule { onInit() { debug `onInit`; - this._useTrackingProtection = false; this._userAgentMode = USER_AGENT_MODE_MOBILE; this._userAgentOverride = null; // Required for safe browsing and tracking protection. - SafeBrowsing.init(); this.registerListener([ "GeckoView:GetUserAgent", From c8131a108b5cafb5e8424eb8dcda5847582b1e79 Mon Sep 17 00:00:00 2001 From: Eugen Sawin Date: Mon, 25 Mar 2019 16:07:48 +0100 Subject: [PATCH 06/11] Bug 1530789 - [3.0] Update API changelog. r=agi Differential Revision: https://phabricator.services.mozilla.com/D24715 --- mobile/android/geckoview/api.txt | 3 ++- .../main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/mobile/android/geckoview/api.txt b/mobile/android/geckoview/api.txt index c4c266849863..284691565f51 100644 --- a/mobile/android/geckoview/api.txt +++ b/mobile/android/geckoview/api.txt @@ -43,9 +43,10 @@ package org.mozilla.geckoview { @android.support.annotation.AnyThread public class ContentBlocking { ctor public ContentBlocking(); field public static final int AT_AD = 2; - field public static final int AT_ALL = 62; + field public static final int AT_ALL = 126; field public static final int AT_ANALYTIC = 4; field public static final int AT_CONTENT = 16; + field public static final int AT_CRYPTOMINING = 64; field public static final int AT_SOCIAL = 8; field public static final int AT_TEST = 32; field public static final int COOKIE_ACCEPT_ALL = 0; diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md index c71735b22817..270333ad5f90 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md @@ -18,6 +18,10 @@ exclude: true [68.2]: ../GeckoSession.ProgressDelegate.html +- Added [`ContentBlocking#AT_CRYPTOMINING`][68.3] for cryptocurrency miner blocking. + +[68.3]: ../ContentBlocking.html#AT_CRYPTOMINING + ## v67 - Added [`setAutomaticFontSizeAdjustment`][67.2] to [`GeckoRuntimeSettings`][67.3] for automatically adjusting font size settings @@ -224,4 +228,4 @@ exclude: true [65.24]: ../CrashReporter.html#sendCrashReport-android.content.Context-android.os.Bundle-java.lang.String- [65.25]: ../GeckoResult.html -[api-version]: e8fa4ed799e78f64fddd3f2c463202942811de11 +[api-version]: e48935ac13c3a907d46d50bb2b00f5e84d30ef4b From 540f42b70f01b8d86ce56d3a72a79ee9b5f5fc3a Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Wed, 20 Mar 2019 13:44:56 -0500 Subject: [PATCH 07/11] Bug 1536905 - Baldr: split Resolve() into ResolveCompile() and AsyncInstance() (r=lth) This patch doesn't change behavior; it just splits out the compile and instantiate paths so that the latter can be updated in a later patch. This splitting also slightly simplifies the code. The bits of duplication introduced here are refactored in the next patch. Differential Revision: https://phabricator.services.mozilla.com/D24241 --HG-- extra : rebase_source : b3eab314f7ba2018c46db657da897b080a1e5633 --- js/src/wasm/WasmJS.cpp | 137 ++++++++++++++++++++++++----------------- 1 file changed, 81 insertions(+), 56 deletions(-) diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index df787e127a03..e179074ae5d2 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -1345,27 +1345,6 @@ static bool GetImportArg(JSContext* cx, CallArgs callArgs, return true; } -static bool Instantiate(JSContext* cx, const Module& module, - HandleObject importObj, - MutableHandleWasmInstanceObject instanceObj) { - RootedObject instanceProto( - cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); - - Rooted funcs(cx, FunctionVector(cx)); - Rooted tables(cx); - RootedWasmMemoryObject memory(cx); - Rooted globalObjs(cx); - - RootedValVector globals(cx); - if (!GetImports(cx, module, importObj, &funcs, tables.get(), &memory, - globalObjs.get(), &globals)) { - return false; - } - - return module.instantiate(cx, funcs, tables.get(), memory, globals, - globalObjs.get(), instanceProto, instanceObj); -} - /* static */ bool WasmInstanceObject::construct(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); @@ -1392,8 +1371,23 @@ bool WasmInstanceObject::construct(JSContext* cx, unsigned argc, Value* vp) { return false; } + RootedObject instanceProto( + cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); + + Rooted funcs(cx, FunctionVector(cx)); + Rooted tables(cx); + RootedWasmMemoryObject memory(cx); + Rooted globalObjs(cx); + + RootedValVector globals(cx); + if (!GetImports(cx, *module, importObj, &funcs, tables.get(), &memory, + globalObjs.get(), &globals)) { + return false; + } + RootedWasmInstanceObject instanceObj(cx); - if (!Instantiate(cx, *module, importObj, &instanceObj)) { + if (!module->instantiate(cx, funcs, tables.get(), memory, globals, + globalObjs.get(), instanceProto, &instanceObj)) { return false; } @@ -2866,30 +2860,44 @@ static bool Reject(JSContext* cx, const CompileArgs& args, return PromiseObject::reject(cx, promise, rejectionValue); } -static bool Resolve(JSContext* cx, const Module& module, - Handle promise, bool instantiate, - HandleObject importObj, const UniqueCharsVector& warnings, - const char* methodSuffix = "") { - if (!ReportCompileWarnings(cx, warnings)) { - return false; +enum class Ret { Pair, Instance }; + +static bool AsyncInstantiate(JSContext* cx, const Module& module, + HandleObject importObj, Ret ret, + Handle promise) { + RootedObject instanceProto( + cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); + + Rooted funcs(cx, FunctionVector(cx)); + Rooted tables(cx); + RootedWasmMemoryObject memory(cx); + Rooted globalObjs(cx); + + RootedValVector globals(cx); + if (!GetImports(cx, module, importObj, &funcs, tables.get(), &memory, + globalObjs.get(), &globals)) { + return RejectWithPendingException(cx, promise); } - RootedObject proto( - cx, &cx->global()->getPrototype(JSProto_WasmModule).toObject()); - RootedObject moduleObj(cx, WasmModuleObject::create(cx, module, proto)); - if (!moduleObj) { + RootedWasmInstanceObject instanceObj(cx); + if (!module.instantiate(cx, funcs, tables.get(), memory, globals, + globalObjs.get(), instanceProto, &instanceObj)) { return RejectWithPendingException(cx, promise); } RootedValue resolutionValue(cx); - if (instantiate) { - RootedWasmInstanceObject instanceObj(cx); - if (!Instantiate(cx, module, importObj, &instanceObj)) { + if (ret == Ret::Instance) { + resolutionValue = ObjectValue(*instanceObj); + } else { + RootedObject resultObj(cx, JS_NewPlainObject(cx)); + if (!resultObj) { return RejectWithPendingException(cx, promise); } - RootedObject resultObj(cx, JS_NewPlainObject(cx)); - if (!resultObj) { + RootedObject proto( + cx, &cx->global()->getPrototype(JSProto_WasmModule).toObject()); + RootedObject moduleObj(cx, WasmModuleObject::create(cx, module, proto)); + if (!moduleObj) { return RejectWithPendingException(cx, promise); } @@ -2904,18 +2912,31 @@ static bool Resolve(JSContext* cx, const Module& module, } resolutionValue = ObjectValue(*resultObj); - } else { - MOZ_ASSERT(!importObj); - resolutionValue = ObjectValue(*moduleObj); } if (!PromiseObject::resolve(cx, promise, resolutionValue)) { return RejectWithPendingException(cx, promise); } - Log(cx, "async %s%s() succeeded", (instantiate ? "instantiate" : "compile"), - methodSuffix); + Log(cx, "async instantiate succeeded"); + return true; +} +static bool ResolveCompile(JSContext* cx, const Module& module, + Handle promise) { + RootedObject proto( + cx, &cx->global()->getPrototype(JSProto_WasmModule).toObject()); + RootedObject moduleObj(cx, WasmModuleObject::create(cx, module, proto)); + if (!moduleObj) { + return RejectWithPendingException(cx, promise); + } + + RootedValue resolutionValue(cx, ObjectValue(*moduleObj)); + if (!PromiseObject::resolve(cx, promise, resolutionValue)) { + return RejectWithPendingException(cx, promise); + } + + Log(cx, "async compile succeeded"); return true; } @@ -2950,9 +2971,16 @@ struct CompileBufferTask : PromiseHelperTask { } bool resolve(JSContext* cx, Handle promise) override { - return module - ? Resolve(cx, *module, promise, instantiate, importObj, warnings) - : Reject(cx, *compileArgs, promise, error); + if (!module) { + return Reject(cx, *compileArgs, promise, error); + } + if (!ReportCompileWarnings(cx, warnings)) { + return false; + } + if (instantiate) { + return AsyncInstantiate(cx, *module, importObj, Ret::Pair, promise); + } + return ResolveCompile(cx, *module, promise); } }; @@ -3063,17 +3091,9 @@ static bool WebAssembly_instantiate(JSContext* cx, unsigned argc, Value* vp) { const Module* module; if (IsModuleObject(firstArg, &module)) { - RootedWasmInstanceObject instanceObj(cx); - if (!Instantiate(cx, *module, importObj, &instanceObj)) { - return RejectWithPendingException(cx, promise, callArgs); - } - - RootedValue resolutionValue(cx, ObjectValue(*instanceObj)); - if (!PromiseObject::resolve(cx, promise, resolutionValue)) { + if (!AsyncInstantiate(cx, *module, importObj, Ret::Instance, promise)) { return false; } - - Log(cx, "async instantiate() succeeded"); } else { auto task = cx->make_unique(cx, promise, importObj); if (!task || !task->init(cx, "WebAssembly.instantiate")) { @@ -3410,8 +3430,13 @@ class CompileStreamTask : public PromiseHelperTask, public JS::StreamConsumer { if (module_) { MOZ_ASSERT(!streamFailed_ && !streamError_ && !compileError_); - return Resolve(cx, *module_, promise, instantiate_, importObj_, warnings_, - "Streaming"); + if (!ReportCompileWarnings(cx, warnings_)) { + return false; + } + if (instantiate_) { + return AsyncInstantiate(cx, *module_, importObj_, Ret::Pair, promise); + } + return ResolveCompile(cx, *module_, promise); } if (streamError_) { From 32ae9f2564cb0e22fe11e7cd039517b51588b5a6 Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Wed, 20 Mar 2019 13:45:00 -0500 Subject: [PATCH 08/11] Bug 1536905 - Baldr: factor out ImportValues struct (r=lth) This patch is also pure refactoring, replacing the N Rooteds with one ImportValues struct that can be Rooted once and then passed around. This reduces existing duplication and also simplifies the next patch. Differential Revision: https://phabricator.services.mozilla.com/D24242 --HG-- extra : rebase_source : bd5397286ee0981486ffffdd4724c41811e3479a --- js/src/wasm/AsmJS.cpp | 26 ++++++-------- js/src/wasm/WasmBuiltins.cpp | 2 +- js/src/wasm/WasmBuiltins.h | 2 +- js/src/wasm/WasmInstance.cpp | 11 +++--- js/src/wasm/WasmInstance.h | 4 +-- js/src/wasm/WasmJS.cpp | 70 ++++++++++++------------------------ js/src/wasm/WasmJS.h | 5 +-- js/src/wasm/WasmModule.cpp | 51 +++++++++++++------------- js/src/wasm/WasmModule.h | 42 ++++++++++++++++------ js/src/wasm/WasmTypes.h | 19 +++++----- 10 files changed, 111 insertions(+), 121 deletions(-) diff --git a/js/src/wasm/AsmJS.cpp b/js/src/wasm/AsmJS.cpp index dc37842d41bf..b5dd4b7fbeed 100644 --- a/js/src/wasm/AsmJS.cpp +++ b/js/src/wasm/AsmJS.cpp @@ -6779,8 +6779,7 @@ static bool CheckBuffer(JSContext* cx, const AsmJSMetadata& metadata, static bool GetImports(JSContext* cx, const AsmJSMetadata& metadata, HandleValue globalVal, HandleValue importVal, - MutableHandle funcImports, - MutableHandleValVector valImports) { + ImportValues* imports) { Rooted ffis(cx, FunctionVector(cx)); if (!ffis.resize(metadata.numFFIs)) { return false; @@ -6793,7 +6792,7 @@ static bool GetImports(JSContext* cx, const AsmJSMetadata& metadata, if (!ValidateGlobalVariable(cx, global, importVal, &litVal)) { return false; } - if (!valImports.append(Val(litVal->asLitVal()))) { + if (!imports->globalValues.append(Val(litVal->asLitVal()))) { return false; } break; @@ -6823,7 +6822,7 @@ static bool GetImports(JSContext* cx, const AsmJSMetadata& metadata, } for (const AsmJSImport& import : metadata.asmJSImports) { - if (!funcImports.append(ffis[import.ffiIndex()])) { + if (!imports->funcs.append(ffis[import.ffiIndex()])) { return false; } } @@ -6845,30 +6844,27 @@ static bool TryInstantiate(JSContext* cx, CallArgs args, const Module& module, return LinkFail(cx, "no compiler support"); } - RootedArrayBufferObjectMaybeShared buffer(cx); - RootedWasmMemoryObject memory(cx); + Rooted imports(cx); + if (module.metadata().usesMemory()) { + RootedArrayBufferObjectMaybeShared buffer(cx); if (!CheckBuffer(cx, metadata, bufferVal, &buffer)) { return false; } - memory = WasmMemoryObject::create(cx, buffer, nullptr); - if (!memory) { + imports.get().memory = WasmMemoryObject::create(cx, buffer, nullptr); + if (!imports.get().memory) { return false; } } - RootedValVector valImports(cx); - Rooted funcs(cx, FunctionVector(cx)); - if (!GetImports(cx, metadata, globalVal, importVal, &funcs, &valImports)) { + if (!GetImports(cx, metadata, globalVal, importVal, imports.address())) { return false; } - Rooted globalObjs(cx); - Rooted tables(cx); - if (!module.instantiate(cx, funcs, tables.get(), memory, valImports, - globalObjs.get(), nullptr, instanceObj)) + if (!module.instantiate(cx, imports.get(), nullptr, instanceObj)) { return false; + } exportObj.set(&instanceObj->exportsObj()); return true; diff --git a/js/src/wasm/WasmBuiltins.cpp b/js/src/wasm/WasmBuiltins.cpp index 79fde2cd898e..07f134a495b3 100644 --- a/js/src/wasm/WasmBuiltins.cpp +++ b/js/src/wasm/WasmBuiltins.cpp @@ -1223,7 +1223,7 @@ static Maybe ToBuiltinABIFunctionType( return Some(ABIFunctionType(abiType)); } -void* wasm::MaybeGetBuiltinThunk(HandleFunction f, const FuncType& funcType) { +void* wasm::MaybeGetBuiltinThunk(JSFunction* f, const FuncType& funcType) { MOZ_ASSERT(builtinThunks); if (!f->isNative() || !f->hasJitInfo() || diff --git a/js/src/wasm/WasmBuiltins.h b/js/src/wasm/WasmBuiltins.h index bda8d2ce668f..5f8afa940bf6 100644 --- a/js/src/wasm/WasmBuiltins.h +++ b/js/src/wasm/WasmBuiltins.h @@ -93,7 +93,7 @@ void* HandleThrow(JSContext* cx, WasmFrameIter& iter); void* SymbolicAddressTarget(SymbolicAddress sym); -void* MaybeGetBuiltinThunk(HandleFunction f, const FuncType& funcType); +void* MaybeGetBuiltinThunk(JSFunction* f, const FuncType& funcType); void ReleaseBuiltinThunks(); diff --git a/js/src/wasm/WasmInstance.cpp b/js/src/wasm/WasmInstance.cpp index 42e6c095016f..44050523ac28 100644 --- a/js/src/wasm/WasmInstance.cpp +++ b/js/src/wasm/WasmInstance.cpp @@ -1078,8 +1078,8 @@ Instance::Instance(JSContext* cx, Handle object, SharedCode code, UniqueTlsData tlsDataIn, HandleWasmMemoryObject memory, SharedTableVector&& tables, StructTypeDescrVector&& structTypeDescrs, - Handle funcImports, - HandleValVector globalImportValues, + const JSFunctionVector& funcImports, + const ValVector& globalImportValues, const WasmGlobalObjectVector& globalObjs, UniqueDebugState maybeDebug) : realm_(cx->realm()), @@ -1120,7 +1120,7 @@ Instance::Instance(JSContext* cx, Handle object, Tier callerTier = code_->bestTier(); for (size_t i = 0; i < metadata(callerTier).funcImports.length(); i++) { - HandleFunction f = funcImports[i]; + JSFunction* f = funcImports[i]; const FuncImport& fi = metadata(callerTier).funcImports[i]; FuncImportTls& import = funcImportTls(fi); import.fun = f; @@ -1171,7 +1171,7 @@ Instance::Instance(JSContext* cx, Handle object, if (global.isIndirect()) { *(void**)globalAddr = globalObjs[imported]->cell(); } else { - CopyValPostBarriered(globalAddr, globalImportValues[imported].get()); + CopyValPostBarriered(globalAddr, globalImportValues[imported]); } break; } @@ -1193,8 +1193,7 @@ Instance::Instance(JSContext* cx, Handle object, // the source global should never be indirect. MOZ_ASSERT(!imported.isIndirect()); - RootedVal dest(cx, - globalImportValues[imported.importIndex()].get()); + RootedVal dest(cx, globalImportValues[imported.importIndex()]); if (global.isIndirect()) { void* address = globalObjs[i]->cell(); *(void**)globalAddr = address; diff --git a/js/src/wasm/WasmInstance.h b/js/src/wasm/WasmInstance.h index f955a593dd51..329911719c9b 100644 --- a/js/src/wasm/WasmInstance.h +++ b/js/src/wasm/WasmInstance.h @@ -74,8 +74,8 @@ class Instance { Instance(JSContext* cx, HandleWasmInstanceObject object, SharedCode code, UniqueTlsData tlsData, HandleWasmMemoryObject memory, SharedTableVector&& tables, StructTypeDescrVector&& structTypeDescrs, - Handle funcImports, - HandleValVector globalImportValues, + const JSFunctionVector& funcImports, + const ValVector& globalImportValues, const WasmGlobalObjectVector& globalObjs, UniqueDebugState maybeDebug); ~Instance(); diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index e179074ae5d2..2bbc422fb685 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -241,14 +241,8 @@ static bool GetProperty(JSContext* cx, HandleObject obj, const char* chars, } static bool GetImports(JSContext* cx, const Module& module, - HandleObject importObj, - MutableHandle funcImports, - WasmTableObjectVector& tableImports, - MutableHandleWasmMemoryObject memoryImport, - WasmGlobalObjectVector& globalObjs, - MutableHandleValVector globalImportValues) { - const ImportVector& imports = module.imports(); - if (!imports.empty() && !importObj) { + HandleObject importObj, ImportValues* imports) { + if (!module.imports().empty() && !importObj) { return ThrowBadImportArg(cx); } @@ -258,7 +252,7 @@ static bool GetImports(JSContext* cx, const Module& module, const GlobalDescVector& globals = metadata.globals; uint32_t tableIndex = 0; const TableDescVector& tables = metadata.tables; - for (const Import& import : imports) { + for (const Import& import : module.imports()) { RootedValue v(cx); if (!GetProperty(cx, importObj, import.module.get(), &v)) { return false; @@ -282,7 +276,7 @@ static bool GetImports(JSContext* cx, const Module& module, return ThrowBadImportType(cx, import.field.get(), "Function"); } - if (!funcImports.append(&v.toObject().as())) { + if (!imports->funcs.append(&v.toObject().as())) { return false; } @@ -301,7 +295,7 @@ static bool GetImports(JSContext* cx, const Module& module, return false; } - if (!tableImports.append(obj)) { + if (!imports->tables.append(obj)) { return false; } break; @@ -311,8 +305,8 @@ static bool GetImports(JSContext* cx, const Module& module, return ThrowBadImportType(cx, import.field.get(), "Memory"); } - MOZ_ASSERT(!memoryImport); - memoryImport.set(&v.toObject().as()); + MOZ_ASSERT(!imports->memory); + imports->memory = &v.toObject().as(); break; } case DefinitionKind::Global: { @@ -335,11 +329,12 @@ static bool GetImports(JSContext* cx, const Module& module, return false; } - if (globalObjs.length() <= index && !globalObjs.resize(index + 1)) { + if (imports->globalObjs.length() <= index && + !imports->globalObjs.resize(index + 1)) { ReportOutOfMemory(cx); return false; } - globalObjs[index] = obj; + imports->globalObjs[index] = obj; obj->val(&val); } else { if (IsNumberType(global.type())) { @@ -371,7 +366,7 @@ static bool GetImports(JSContext* cx, const Module& module, } } - if (!globalImportValues.append(val)) { + if (!imports->globalValues.append(val)) { return false; } @@ -450,19 +445,12 @@ bool wasm::Eval(JSContext* cx, Handle code, return false; } - Rooted funcs(cx, FunctionVector(cx)); - Rooted tables(cx); - RootedWasmMemoryObject memory(cx); - Rooted globalObjs(cx); - - RootedValVector globals(cx); - if (!GetImports(cx, *module, importObj, &funcs, tables.get(), &memory, - globalObjs.get(), &globals)) { + Rooted imports(cx); + if (!GetImports(cx, *module, importObj, imports.address())) { return false; } - return module->instantiate(cx, funcs, tables.get(), memory, globals, - globalObjs.get(), nullptr, instanceObj); + return module->instantiate(cx, imports.get(), nullptr, instanceObj); } struct MOZ_STACK_CLASS SerializeListener : JS::OptimizedEncodingListener { @@ -1250,8 +1238,8 @@ WasmInstanceObject* WasmInstanceObject::create( const ElemSegmentVector& elemSegments, UniqueTlsData tlsData, HandleWasmMemoryObject memory, SharedTableVector&& tables, StructTypeDescrVector&& structTypeDescrs, - Handle funcImports, const GlobalDescVector& globals, - HandleValVector globalImportValues, + const JSFunctionVector& funcImports, const GlobalDescVector& globals, + const ValVector& globalImportValues, const WasmGlobalObjectVector& globalObjs, HandleObject proto, UniqueDebugState maybeDebug) { UniquePtr exports = js::MakeUnique(); @@ -1374,20 +1362,13 @@ bool WasmInstanceObject::construct(JSContext* cx, unsigned argc, Value* vp) { RootedObject instanceProto( cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); - Rooted funcs(cx, FunctionVector(cx)); - Rooted tables(cx); - RootedWasmMemoryObject memory(cx); - Rooted globalObjs(cx); - - RootedValVector globals(cx); - if (!GetImports(cx, *module, importObj, &funcs, tables.get(), &memory, - globalObjs.get(), &globals)) { + Rooted imports(cx); + if (!GetImports(cx, *module, importObj, imports.address())) { return false; } RootedWasmInstanceObject instanceObj(cx); - if (!module->instantiate(cx, funcs, tables.get(), memory, globals, - globalObjs.get(), instanceProto, &instanceObj)) { + if (!module->instantiate(cx, imports.get(), instanceProto, &instanceObj)) { return false; } @@ -2868,20 +2849,13 @@ static bool AsyncInstantiate(JSContext* cx, const Module& module, RootedObject instanceProto( cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); - Rooted funcs(cx, FunctionVector(cx)); - Rooted tables(cx); - RootedWasmMemoryObject memory(cx); - Rooted globalObjs(cx); - - RootedValVector globals(cx); - if (!GetImports(cx, module, importObj, &funcs, tables.get(), &memory, - globalObjs.get(), &globals)) { + Rooted imports(cx); + if (!GetImports(cx, module, importObj, imports.address())) { return RejectWithPendingException(cx, promise); } RootedWasmInstanceObject instanceObj(cx); - if (!module.instantiate(cx, funcs, tables.get(), memory, globals, - globalObjs.get(), instanceProto, &instanceObj)) { + if (!module.instantiate(cx, imports.get(), instanceProto, &instanceObj)) { return RejectWithPendingException(cx, promise); } diff --git a/js/src/wasm/WasmJS.h b/js/src/wasm/WasmJS.h index fd192daa2a18..edca362518da 100644 --- a/js/src/wasm/WasmJS.h +++ b/js/src/wasm/WasmJS.h @@ -245,8 +245,9 @@ class WasmInstanceObject : public NativeObject { Vector, 0, SystemAllocPolicy>&& tables, GCVector, 0, SystemAllocPolicy>&& structTypeDescrs, - Handle funcImports, const wasm::GlobalDescVector& globals, - wasm::HandleValVector globalImportValues, + const JSFunctionVector& funcImports, + const wasm::GlobalDescVector& globals, + const wasm::ValVector& globalImportValues, const WasmGlobalObjectVector& globalObjs, HandleObject proto, UniquePtr maybeDebug); void initExportsObj(JSObject& exportsObj); diff --git a/js/src/wasm/WasmModule.cpp b/js/src/wasm/WasmModule.cpp index 13b5388abf78..1b7b63bee3a2 100644 --- a/js/src/wasm/WasmModule.cpp +++ b/js/src/wasm/WasmModule.cpp @@ -560,22 +560,22 @@ bool Module::extractCode(JSContext* cx, Tier tier, return true; } -static uint32_t EvaluateInitExpr(HandleValVector globalImportValues, +static uint32_t EvaluateInitExpr(const ValVector& globalImportValues, InitExpr initExpr) { switch (initExpr.kind()) { case InitExpr::Kind::Constant: return initExpr.val().i32(); case InitExpr::Kind::GetGlobal: - return globalImportValues[initExpr.globalIndex()].get().i32(); + return globalImportValues[initExpr.globalIndex()].i32(); } MOZ_CRASH("bad initializer expression"); } bool Module::initSegments(JSContext* cx, HandleWasmInstanceObject instanceObj, - Handle funcImports, + const JSFunctionVector& funcImports, HandleWasmMemoryObject memoryObj, - HandleValVector globalImportValues) const { + const ValVector& globalImportValues) const { MOZ_ASSERT_IF(!memoryObj, dataSegments_.empty()); Instance& instance = instanceObj->instance(); @@ -707,7 +707,7 @@ static const Import& FindImportForFuncImport(const ImportVector& imports, } bool Module::instantiateFunctions(JSContext* cx, - Handle funcImports) const { + const JSFunctionVector& funcImports) const { #ifdef DEBUG for (auto t : code().tiers()) { MOZ_ASSERT(funcImports.length() == metadata(t).funcImports.length()); @@ -721,7 +721,7 @@ bool Module::instantiateFunctions(JSContext* cx, Tier tier = code().stableTier(); for (size_t i = 0; i < metadata(tier).funcImports.length(); i++) { - HandleFunction f = funcImports[i]; + JSFunction* f = funcImports[i]; if (!IsExportedFunction(f) || ExportedFunctionToInstance(f).isAsmJS()) { continue; } @@ -904,7 +904,7 @@ bool Module::instantiateLocalTable(JSContext* cx, const TableDesc& td, } bool Module::instantiateTables(JSContext* cx, - WasmTableObjectVector& tableImports, + const WasmTableObjectVector& tableImports, MutableHandle tableObjs, SharedTableVector* tables) const { uint32_t tableIndex = 0; @@ -925,7 +925,7 @@ bool Module::instantiateTables(JSContext* cx, return true; } -static void ExtractGlobalValue(HandleValVector globalImportValues, +static void ExtractGlobalValue(const ValVector& globalImportValues, uint32_t globalIndex, const GlobalDesc& global, MutableHandleVal result) { switch (global.kind()) { @@ -954,7 +954,7 @@ static void ExtractGlobalValue(HandleValVector globalImportValues, } static bool EnsureGlobalObject(JSContext* cx, - HandleValVector globalImportValues, + const ValVector& globalImportValues, size_t globalIndex, const GlobalDesc& global, WasmGlobalObjectVector& globalObjs) { if (globalIndex < globalObjs.length() && globalObjs[globalIndex]) { @@ -980,7 +980,7 @@ static bool EnsureGlobalObject(JSContext* cx, } bool Module::instantiateGlobals(JSContext* cx, - HandleValVector globalImportValues, + const ValVector& globalImportValues, WasmGlobalObjectVector& globalObjs) const { // If there are exported globals that aren't in globalObjs because they // originate in this module or because they were immutable imports that came @@ -1083,7 +1083,7 @@ SharedCode Module::getDebugEnabledCode() const { static bool GetFunctionExport(JSContext* cx, HandleWasmInstanceObject instanceObj, - Handle funcImports, + const JSFunctionVector& funcImports, const Export& exp, MutableHandleValue val) { if (exp.funcIndex() < funcImports.length() && IsExportedWasmFunction(funcImports[exp.funcIndex()])) { @@ -1103,7 +1103,7 @@ static bool GetFunctionExport(JSContext* cx, static bool CreateExportObject(JSContext* cx, HandleWasmInstanceObject instanceObj, - Handle funcImports, + const JSFunctionVector& funcImports, const WasmTableObjectVector& tableObjs, HandleWasmMemoryObject memoryObj, const WasmGlobalObjectVector& globalObjs, @@ -1335,20 +1335,16 @@ bool Module::makeStructTypeDescrs( #endif } -bool Module::instantiate(JSContext* cx, Handle funcImports, - WasmTableObjectVector& tableImports, - HandleWasmMemoryObject memoryImport, - HandleValVector globalImportValues, - WasmGlobalObjectVector& globalObjs, +bool Module::instantiate(JSContext* cx, ImportValues& imports, HandleObject instanceProto, MutableHandleWasmInstanceObject instance) const { MOZ_RELEASE_ASSERT(cx->wasmHaveSignalHandlers); - if (!instantiateFunctions(cx, funcImports)) { + if (!instantiateFunctions(cx, imports.funcs)) { return false; } - RootedWasmMemoryObject memory(cx, memoryImport); + RootedWasmMemoryObject memory(cx, imports.memory); if (!instantiateMemory(cx, &memory)) { return false; } @@ -1358,11 +1354,11 @@ bool Module::instantiate(JSContext* cx, Handle funcImports, Rooted tableObjs(cx); SharedTableVector tables; - if (!instantiateTables(cx, tableImports, &tableObjs, &tables)) { + if (!instantiateTables(cx, imports.tables, &tableObjs, &tables)) { return false; } - if (!instantiateGlobals(cx, globalImportValues, globalObjs)) { + if (!instantiateGlobals(cx, imports.globalValues, imports.globalObjs)) { return false; } @@ -1398,15 +1394,15 @@ bool Module::instantiate(JSContext* cx, Handle funcImports, instance.set(WasmInstanceObject::create( cx, code, dataSegments_, elemSegments_, std::move(tlsData), memory, - std::move(tables), std::move(structTypeDescrs.get()), funcImports, - metadata().globals, globalImportValues, globalObjs, instanceProto, - std::move(maybeDebug))); + std::move(tables), std::move(structTypeDescrs.get()), imports.funcs, + metadata().globals, imports.globalValues, imports.globalObjs, + instanceProto, std::move(maybeDebug))); if (!instance) { return false; } - if (!CreateExportObject(cx, instance, funcImports, tableObjs.get(), memory, - globalObjs, exports_)) { + if (!CreateExportObject(cx, instance, imports.funcs, tableObjs.get(), memory, + imports.globalObjs, exports_)) { return false; } @@ -1423,7 +1419,8 @@ bool Module::instantiate(JSContext* cx, Handle funcImports, // constructed since this can make the instance live to content (even if the // start function fails). - if (!initSegments(cx, instance, funcImports, memory, globalImportValues)) { + if (!initSegments(cx, instance, imports.funcs, memory, + imports.globalValues)) { return false; } diff --git a/js/src/wasm/WasmModule.h b/js/src/wasm/WasmModule.h index c4477facd252..5a6f4d01e687 100644 --- a/js/src/wasm/WasmModule.h +++ b/js/src/wasm/WasmModule.h @@ -34,6 +34,31 @@ struct CompileArgs; typedef RefPtr Tier2Listener; +// A struct containing the typed, imported values that are harvested from the +// import object and passed to Module::instantiate(). This struct must be +// stored in a (Persistent)Rooted, not in the heap due to its use of TraceRoot() +// and complete lack of barriers. + +struct ImportValues { + JSFunctionVector funcs; + WasmTableObjectVector tables; + WasmMemoryObject* memory; + WasmGlobalObjectVector globalObjs; + ValVector globalValues; + + ImportValues() : memory(nullptr) {} + + void trace(JSTracer* trc) { + funcs.trace(trc); + tables.trace(trc); + if (memory) { + TraceRoot(trc, &memory, "import values memory"); + } + globalObjs.trace(trc); + globalValues.trace(trc); + } +}; + // Module represents a compiled wasm module and primarily provides three // operations: instantiation, tiered compilation, serialization. A Module can be // instantiated any number of times to produce new Instance objects. A Module @@ -82,7 +107,7 @@ class Module : public JS::WasmModule { mutable Atomic testingTier2Active_; bool instantiateFunctions(JSContext* cx, - Handle funcImports) const; + const JSFunctionVector& funcImports) const; bool instantiateMemory(JSContext* cx, MutableHandleWasmMemoryObject memory) const; bool instantiateImportedTable(JSContext* cx, const TableDesc& td, @@ -92,15 +117,16 @@ class Module : public JS::WasmModule { bool instantiateLocalTable(JSContext* cx, const TableDesc& td, WasmTableObjectVector* tableObjs, SharedTableVector* tables) const; - bool instantiateTables(JSContext* cx, WasmTableObjectVector& tableImports, + bool instantiateTables(JSContext* cx, + const WasmTableObjectVector& tableImports, MutableHandle tableObjs, SharedTableVector* tables) const; - bool instantiateGlobals(JSContext* cx, HandleValVector globalImportValues, + bool instantiateGlobals(JSContext* cx, const ValVector& globalImportValues, WasmGlobalObjectVector& globalObjs) const; bool initSegments(JSContext* cx, HandleWasmInstanceObject instance, - Handle funcImports, + const JSFunctionVector& funcImports, HandleWasmMemoryObject memory, - HandleValVector globalImportValues) const; + const ValVector& globalImportValues) const; SharedCode getDebugEnabledCode() const; bool makeStructTypeDescrs( JSContext* cx, @@ -144,11 +170,7 @@ class Module : public JS::WasmModule { // Instantiate this module with the given imports: - bool instantiate(JSContext* cx, Handle funcImports, - WasmTableObjectVector& tableImport, - HandleWasmMemoryObject memoryImport, - HandleValVector globalImportValues, - WasmGlobalObjectVector& globalObjs, + bool instantiate(JSContext* cx, ImportValues& imports, HandleObject instanceProto, MutableHandleWasmInstanceObject instanceObj) const; diff --git a/js/src/wasm/WasmTypes.h b/js/src/wasm/WasmTypes.h index 4840cc3ef1e2..e89986ecc7de 100644 --- a/js/src/wasm/WasmTypes.h +++ b/js/src/wasm/WasmTypes.h @@ -49,6 +49,8 @@ enum class RoundingMode; // This is a widespread header, so lets keep out the core wasm impl types. +typedef GCVector JSFunctionVector; + class WasmMemoryObject; typedef GCPtr GCPtrWasmMemoryObject; typedef Rooted RootedWasmMemoryObject; @@ -748,11 +750,10 @@ class LitVal { } }; -// A Val is a LitVal that can contain pointers to JSObjects, thanks to their -// trace implementation. Since a Val is able to store a pointer to a JSObject, -// it needs to be traced during compilation in case the pointee is moved. -// The classic shorthands for Rooted things are defined after this class, for -// easier usage. +// A Val is a LitVal that can contain (non-null) pointers to GC things. All Vals +// must be stored in Rooteds so that their trace() methods are called during +// stack marking. Vals do not implement barriers and thus may not be stored on +// the heap. class MOZ_NON_PARAM Val : public LitVal { public: @@ -773,10 +774,10 @@ typedef Rooted RootedVal; typedef Handle HandleVal; typedef MutableHandle MutableHandleVal; -typedef GCVector GCVectorVal; -typedef Rooted RootedValVector; -typedef Handle HandleValVector; -typedef MutableHandle MutableHandleValVector; +typedef GCVector ValVector; +typedef Rooted RootedValVector; +typedef Handle HandleValVector; +typedef MutableHandle MutableHandleValVector; // The FuncType class represents a WebAssembly function signature which takes a // list of value types and returns an expression type. The engine uses two From 4f1f0dbca79d2a3e66bea1c35e8eb6639b9cbbe3 Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Wed, 20 Mar 2019 13:45:04 -0500 Subject: [PATCH 09/11] Bug 1536905 - Baldr: do instantiation in a separate task from the getting of imports (r=lth) Having this OffThreadPromiseTask in place may also be useful one day if we want to do any non-trivial instantiation-time codegen (say of Web IDL Bindings stubs...). Differential Revision: https://phabricator.services.mozilla.com/D24243 --HG-- extra : rebase_source : 7a1dc29de10004e1d6c1f82ee48da3eb0321eb5f --- js/src/builtin/Promise.h | 9 ++ .../jit-test/tests/wasm/async-instantiate.js | 47 ++++++++ js/src/wasm/WasmJS.cpp | 114 +++++++++++------- 3 files changed, 127 insertions(+), 43 deletions(-) create mode 100644 js/src/jit-test/tests/wasm/async-instantiate.js diff --git a/js/src/builtin/Promise.h b/js/src/builtin/Promise.h index 1cc37936b5d4..5f7337d562a0 100644 --- a/js/src/builtin/Promise.h +++ b/js/src/builtin/Promise.h @@ -479,6 +479,15 @@ class OffThreadPromiseRuntimeState; // OffThreadPromiseTask for the promise, and let the embedding's network I/O // threads call dispatchResolveAndDestroy. // +// OffThreadPromiseTask may also be used purely on the main thread, as a way to +// "queue a task" in HTML terms. Note that a "task" is not the same as a +// "microtask" and there are separate queues for tasks and microtasks that are +// drained at separate times in the browser. The task queue is implemented by +// the browser's main event loop. The microtask queue is implemented +// by JS::JobQueue, used for promises and gets drained before returning to +// the event loop. Thus OffThreadPromiseTask can only be used when the spec +// says "queue a task", as the WebAssembly APIs do. +// // An OffThreadPromiseTask has a JSContext, and must be constructed and have its // 'init' method called on that JSContext's thread. Once initialized, its // dispatchResolveAndDestroy method may be called from any thread. This is the diff --git a/js/src/jit-test/tests/wasm/async-instantiate.js b/js/src/jit-test/tests/wasm/async-instantiate.js new file mode 100644 index 000000000000..446f89006568 --- /dev/null +++ b/js/src/jit-test/tests/wasm/async-instantiate.js @@ -0,0 +1,47 @@ +const { Module, Instance, Global, instantiate, instantiateStreaming } = WebAssembly; + +const g = new Global({value: "i32", mutable:true}, 0); + +const code = wasmTextToBinary(`(module + (global $g (import "" "g") (mut i32)) + (func $start (set_global $g (i32.add (get_global $g) (i32.const 1)))) + (start $start) +)`); +const module = new Module(code); + +const importObj = { '': { get g() { g.value++; return g } } }; + +g.value = 0; +new Instance(module, importObj); +assertEq(g.value, 2); + +g.value = 0; +instantiate(module, importObj).then(i => { + assertEq(i instanceof Instance, true); + assertEq(g.value, 2); + g.value++; +}); +assertEq(g.value, 1); +drainJobQueue(); +assertEq(g.value, 3); + +g.value = 0; +instantiate(code, importObj).then(({module,instance}) => { + assertEq(module instanceof Module, true); + assertEq(instance instanceof Instance, true); + assertEq(g.value, 2); g.value++; } +); +drainJobQueue(); +assertEq(g.value, 3); + +if (wasmStreamingIsSupported()) { + g.value = 0; + instantiateStreaming(code, importObj).then(({module,instance}) => { + assertEq(module instanceof Module, true); + assertEq(instance instanceof Instance, true); + assertEq(g.value, 2); + g.value++; + }); + drainJobQueue(); + assertEq(g.value, 3); +} diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index 2bbc422fb685..55d6fee3364f 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -2843,56 +2843,84 @@ static bool Reject(JSContext* cx, const CompileArgs& args, enum class Ret { Pair, Instance }; +class AsyncInstantiateTask : public OffThreadPromiseTask { + SharedModule module_; + PersistentRooted imports_; + Ret ret_; + + public: + AsyncInstantiateTask(JSContext* cx, const Module& module, Ret ret, + Handle promise) + : OffThreadPromiseTask(cx, promise), + module_(&module), + imports_(cx), + ret_(ret) {} + + ImportValues& imports() { return imports_.get(); } + + bool resolve(JSContext* cx, Handle promise) override { + RootedObject instanceProto( + cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); + + RootedWasmInstanceObject instanceObj(cx); + if (!module_->instantiate(cx, imports_.get(), instanceProto, + &instanceObj)) { + return RejectWithPendingException(cx, promise); + } + + RootedValue resolutionValue(cx); + if (ret_ == Ret::Instance) { + resolutionValue = ObjectValue(*instanceObj); + } else { + RootedObject resultObj(cx, JS_NewPlainObject(cx)); + if (!resultObj) { + return RejectWithPendingException(cx, promise); + } + + RootedObject moduleProto( + cx, &cx->global()->getPrototype(JSProto_WasmModule).toObject()); + RootedObject moduleObj( + cx, WasmModuleObject::create(cx, *module_, moduleProto)); + if (!moduleObj) { + return RejectWithPendingException(cx, promise); + } + + RootedValue val(cx, ObjectValue(*moduleObj)); + if (!JS_DefineProperty(cx, resultObj, "module", val, JSPROP_ENUMERATE)) { + return RejectWithPendingException(cx, promise); + } + + val = ObjectValue(*instanceObj); + if (!JS_DefineProperty(cx, resultObj, "instance", val, + JSPROP_ENUMERATE)) { + return RejectWithPendingException(cx, promise); + } + + resolutionValue = ObjectValue(*resultObj); + } + + if (!PromiseObject::resolve(cx, promise, resolutionValue)) { + return RejectWithPendingException(cx, promise); + } + + Log(cx, "async instantiate succeeded"); + return true; + } +}; + static bool AsyncInstantiate(JSContext* cx, const Module& module, HandleObject importObj, Ret ret, Handle promise) { - RootedObject instanceProto( - cx, &cx->global()->getPrototype(JSProto_WasmInstance).toObject()); + auto task = js::MakeUnique(cx, module, ret, promise); + if (!task || !task->init(cx)) { + return false; + } - Rooted imports(cx); - if (!GetImports(cx, module, importObj, imports.address())) { + if (!GetImports(cx, module, importObj, &task->imports())) { return RejectWithPendingException(cx, promise); } - RootedWasmInstanceObject instanceObj(cx); - if (!module.instantiate(cx, imports.get(), instanceProto, &instanceObj)) { - return RejectWithPendingException(cx, promise); - } - - RootedValue resolutionValue(cx); - if (ret == Ret::Instance) { - resolutionValue = ObjectValue(*instanceObj); - } else { - RootedObject resultObj(cx, JS_NewPlainObject(cx)); - if (!resultObj) { - return RejectWithPendingException(cx, promise); - } - - RootedObject proto( - cx, &cx->global()->getPrototype(JSProto_WasmModule).toObject()); - RootedObject moduleObj(cx, WasmModuleObject::create(cx, module, proto)); - if (!moduleObj) { - return RejectWithPendingException(cx, promise); - } - - RootedValue val(cx, ObjectValue(*moduleObj)); - if (!JS_DefineProperty(cx, resultObj, "module", val, JSPROP_ENUMERATE)) { - return RejectWithPendingException(cx, promise); - } - - val = ObjectValue(*instanceObj); - if (!JS_DefineProperty(cx, resultObj, "instance", val, JSPROP_ENUMERATE)) { - return RejectWithPendingException(cx, promise); - } - - resolutionValue = ObjectValue(*resultObj); - } - - if (!PromiseObject::resolve(cx, promise, resolutionValue)) { - return RejectWithPendingException(cx, promise); - } - - Log(cx, "async instantiate succeeded"); + task.release()->dispatchResolveAndDestroy(); return true; } From 51630bd49f32bd090891e46349e9e3590428b4b0 Mon Sep 17 00:00:00 2001 From: Eugen Sawin Date: Mon, 25 Mar 2019 17:28:40 +0100 Subject: [PATCH 10/11] Bug 1530789 - [4.0] Fix style. r=me --- .../src/main/java/org/mozilla/geckoview/ContentBlocking.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java index 941921a06ca1..8f5a8112774b 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ContentBlocking.java @@ -447,11 +447,11 @@ public class ContentBlocking { return builder.substring(0, builder.length() - 1); } - /* package */ static boolean catToCmPref(@Category int cat) { + /* package */ static boolean catToCmPref(@Category final int cat) { return (cat & AT_CRYPTOMINING) != 0; } - /* package */ static String catToCmListPref(@Category int cat) { + /* package */ static String catToCmListPref(@Category final int cat) { StringBuilder builder = new StringBuilder(); if ((cat & AT_CRYPTOMINING) != 0) { From 676cfa3d563bad61c7f0807f0cb73badeff1a68d Mon Sep 17 00:00:00 2001 From: Noemi Erli Date: Mon, 25 Mar 2019 19:25:13 +0200 Subject: [PATCH 11/11] Backed out changeset d0b9b3883584 (bug 1534786) for failures in browser_dbg-old-breakpoint.js --- .../debugger/new/test/mochitest/browser.ini | 1 - .../mochitest/browser_dbg-old-breakpoint.js | 90 ------------------- 2 files changed, 91 deletions(-) delete mode 100644 devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js diff --git a/devtools/client/debugger/new/test/mochitest/browser.ini b/devtools/client/debugger/new/test/mochitest/browser.ini index ef68909f67ee..8f1ee320763b 100644 --- a/devtools/client/debugger/new/test/mochitest/browser.ini +++ b/devtools/client/debugger/new/test/mochitest/browser.ini @@ -781,4 +781,3 @@ skip-if = true [browser_dbg-event-handler.js] [browser_dbg-eval-throw.js] [browser_dbg-sourceURL-breakpoint.js] -[browser_dbg-old-breakpoint.js] diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js deleted file mode 100644 index 22bbf0d682a3..000000000000 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-old-breakpoint.js +++ /dev/null @@ -1,90 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -// Test that we show a breakpoint in the UI when there is an old pending -// breakpoint with an invalid original location. -add_task(async function() { - clearDebuggerPreferences(); - - const pending = { - bp1: { - location: { - sourceId: "", - sourceUrl: EXAMPLE_URL + "nowhere2.js", - line: 5 - }, - generatedLocation: { - sourceUrl: EXAMPLE_URL + "simple1.js", - line: 4 - }, - options: {}, - disabled: false - }, - bp2: { - location: { - sourceId: "", - sourceUrl: EXAMPLE_URL + "nowhere.js", - line: 5 - }, - generatedLocation: { - sourceUrl: EXAMPLE_URL + "simple3.js", - line: 2 - }, - options: {}, - disabled: false - }, - }; - asyncStorage.setItem("debugger.pending-breakpoints", pending); - - const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + "doc-scripts.html", "jsdebugger"); - const dbg = createDebuggerContext(toolbox); - - // Pending breakpoints are installed asynchronously, keep invoking the entry - // function until the debugger pauses. - await waitUntil(() => { - invokeInTab("main"); - return isPaused(dbg); - }); - - ok(true, "paused at unmapped breakpoint"); - await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 2); - ok(true, "unmapped breakpoints shown in UI"); -}); - -// Test that if we show a breakpoint with an old generated location, it is -// removed after we load the original source and find the new generated -// location. -add_task(async function() { - clearDebuggerPreferences(); - - const pending = { - bp1: { - location: { - sourceId: "", - sourceUrl: "webpack:///entry.js", - line: 15, - column: 0 - }, - generatedLocation: { - sourceUrl: EXAMPLE_URL + "sourcemaps/bundle.js", - line: 47, - column: 16 - }, - astLocation: {}, - options: {}, - disabled: false - }, - }; - asyncStorage.setItem("debugger.pending-breakpoints", pending); - - const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + "doc-sourcemaps.html", "jsdebugger"); - const dbg = createDebuggerContext(toolbox); - - await waitForState(dbg, state => { - const bps = dbg.selectors.getBreakpointsList(state); - return bps.length == 1 - && bps[0].location.sourceUrl.includes("entry.js") - && bps[0].location.line == 15; - }); - ok(true, "removed old breakpoint during sync"); -});