Skip to content

Commit 147fa4a

Browse files
rpertarperta
committed
Revert "Bug 2017797 - Part 3: Preserve legacy structuredClone behaviour for WebExtensions, r=robwu,asuth" for causing hazard failures at js/xpconnect/src/Sandbox.cpp:X
This reverts commit 6654e59. This reverts commit b5186a6. This reverts commit 1cd2ed3.
1 parent faa084c commit 147fa4a

File tree

9 files changed

+23
-229
lines changed

9 files changed

+23
-229
lines changed

‎dom/base/nsContentUtils.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11950,9 +11950,6 @@ void nsContentUtils::StructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal,
1195011950
clonePolicy.allowSharedMemoryObjects();
1195111951
}
1195211952

11953-
// 2.7.10 Structured cloning API
11954-
// Step 1: Let serialized be
11955-
// ? StructuredSerializeWithTransfer(value, options["transfer"])
1195611953
StructuredCloneHolder holder(StructuredCloneHolder::CloningSupported,
1195711954
StructuredCloneHolder::TransferringSupported,
1195811955
JS::StructuredCloneScope::SameProcess);
@@ -11961,16 +11958,11 @@ void nsContentUtils::StructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal,
1196111958
return;
1196211959
}
1196311960

11964-
// Step 2: Let deserializeRecord be
11965-
// ? StructuredDeserializeWithTransfer(serialized, this's relevant realm).
11966-
JSAutoRealm ar(aCx, aGlobal->GetGlobalJSObject());
1196711961
holder.Read(aCx, aRetval, clonePolicy, aError);
1196811962
if (NS_WARN_IF(aError.Failed())) {
1196911963
return;
1197011964
}
1197111965

11972-
// Step 3: Return deserializeRecord.[[Deserialized]].
11973-
// (discarding deserializeRecord.[[TransferredValues]])
1197411966
nsTArray<RefPtr<MessagePort>> ports = holder.TakeTransferredPorts();
1197511967
(void)ports;
1197611968
}

‎dom/bindings/BindingUtils.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,10 @@ inline bool IsDOMObject(JSObject* obj) { return IsDOMClass(JS::GetClass(obj)); }
172172
obj, value, cx)
173173

174174
// Test whether the given object is an instance of the given interface.
175-
#define IS_INSTANCE_OF(Interface, obj) \
176-
mozilla::dom::IsInstanceOf<mozilla::dom::prototypes::id::Interface>(obj)
175+
#define IS_INSTANCE_OF(Interface, obj) \
176+
mozilla::dom::IsInstanceOf<mozilla::dom::prototypes::id::Interface, \
177+
mozilla::dom::Interface##_Binding::NativeType>( \
178+
obj)
177179

178180
// Unwrap the given non-wrapper object. This can be used with any obj that
179181
// converts to JSObject*; as long as that JSObject* is live the return value
@@ -420,11 +422,11 @@ MOZ_ALWAYS_INLINE nsresult UnwrapObjectWithCrossOriginAsserts(V&& obj,
420422
}
421423
} // namespace binding_detail
422424

423-
template <prototypes::ID PrototypeID>
425+
template <prototypes::ID PrototypeID, class T>
424426
MOZ_ALWAYS_INLINE bool IsInstanceOf(JSObject* obj) {
425427
AssertStaticUnwrapOK<PrototypeID>();
426428
void* ignored;
427-
nsresult unwrapped = binding_detail::UnwrapObjectInternal<void, true>(
429+
nsresult unwrapped = binding_detail::UnwrapObjectInternal<T, true>(
428430
obj, ignored, PrototypeID, PrototypeTraits<PrototypeID>::Depth, nullptr);
429431
return NS_SUCCEEDED(unwrapped);
430432
}

‎js/xpconnect/src/Sandbox.cpp‎

Lines changed: 2 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -379,31 +379,6 @@ static bool SandboxCreateStorage(JSContext* cx, JS::HandleObject obj) {
379379
return JS_DefineProperty(cx, obj, "storage", wrapped, JSPROP_ENUMERATE);
380380
}
381381

382-
// Prior to bug 2013389, the following DOM objects would be structured-cloned
383-
// into the inner window's realm when `structuredClone` is called from an
384-
// extension content script. All other objects would remain within the content
385-
// script's realm. This method is used to retain this historic behaviour.
386-
//
387-
// See bug 2017797 for discussion about this behaviour.
388-
static bool LegacyShouldCloneIntoWindow(JSObject* obj) {
389-
return IS_INSTANCE_OF(Blob, obj) || IS_INSTANCE_OF(Directory, obj) ||
390-
IS_INSTANCE_OF(FileList, obj) || IS_INSTANCE_OF(FormData, obj) ||
391-
IS_INSTANCE_OF(ImageBitmap, obj) || IS_INSTANCE_OF(VideoFrame, obj) ||
392-
IS_INSTANCE_OF(EncodedVideoChunk, obj) ||
393-
IS_INSTANCE_OF(AudioData, obj) ||
394-
IS_INSTANCE_OF(EncodedAudioChunk, obj) ||
395-
#ifdef MOZ_WEBRTC
396-
IS_INSTANCE_OF(RTCEncodedVideoFrame, obj) ||
397-
IS_INSTANCE_OF(RTCEncodedAudioFrame, obj) ||
398-
IS_INSTANCE_OF(RTCDataChannel, obj) ||
399-
#endif
400-
IS_INSTANCE_OF(MessagePort, obj) ||
401-
IS_INSTANCE_OF(OffscreenCanvas, obj) ||
402-
IS_INSTANCE_OF(ReadableStream, obj) ||
403-
IS_INSTANCE_OF(WritableStream, obj) ||
404-
IS_INSTANCE_OF(TransformStream, obj);
405-
}
406-
407382
static bool SandboxStructuredClone(JSContext* cx, unsigned argc, Value* vp) {
408383
CallArgs args = CallArgsFromVp(argc, vp);
409384

@@ -412,43 +387,25 @@ static bool SandboxStructuredClone(JSContext* cx, unsigned argc, Value* vp) {
412387
}
413388

414389
RootedDictionary<dom::StructuredSerializeOptions> options(cx);
390+
BindingCallContext callCx(cx, "structuredClone");
415391
if (!options.Init(cx, args.hasDefined(1) ? args[1] : JS::NullHandleValue,
416392
"Argument 2", false)) {
417393
return false;
418394
}
419395

420-
// NOTE: A spec-compliant structuredClone should determine & use the relevant
421-
// global instead of the current global.
422-
nsCOMPtr<nsIGlobalObject> global = CurrentNativeGlobal(cx);
396+
nsIGlobalObject* global = CurrentNativeGlobal(cx);
423397
if (!global) {
424398
JS_ReportErrorASCII(cx, "structuredClone: Missing global");
425399
return false;
426400
}
427401

428-
// If this is a content script, we may want to clone into that window instead.
429-
// See the comment on LegacyShouldCloneIntoWindow for details.
430-
if (IsWebExtensionContentScriptSandbox(global->GetGlobalJSObject()) &&
431-
StaticPrefs::extensions_webextensions_legacyStructuredCloneBehavior() &&
432-
args[0].isObject() && LegacyShouldCloneIntoWindow(&args[0].toObject())) {
433-
if (nsGlobalWindowInner* window =
434-
SandboxWindowOrNull(global->GetGlobalJSObject(), cx)) {
435-
global = window;
436-
}
437-
}
438-
439402
JS::Rooted<JS::Value> result(cx);
440403
ErrorResult rv;
441404
nsContentUtils::StructuredClone(cx, global, args[0], options, &result, rv);
442405
if (rv.MaybeSetPendingException(cx)) {
443406
return false;
444407
}
445408

446-
// Because we specified a custom `global`, the returned value may not be in
447-
// our realm.
448-
if (!mozilla::dom::MaybeWrapValue(cx, &result)) {
449-
return false;
450-
}
451-
452409
MOZ_ASSERT_IF(result.isGCThing(),
453410
!JS::GCThingIsMarkedGray(result.toGCCellPtr()));
454411
args.rval().set(result);

‎modules/libpref/init/StaticPrefList.yaml‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6217,16 +6217,6 @@
62176217
#endif
62186218
mirror: always
62196219

6220-
# Prior to bug 2013389 and bug 2017797, calling structuredClone within a
6221-
# content script would clone most objects within the content script's realm,
6222-
# however a subset of DOM objects would be cloned into the underlying content
6223-
# window's realm. When enabled, this pref preserves this legacy behaviour for
6224-
# compatibility with existing content scripts.
6225-
- name: extensions.webextensions.legacyStructuredCloneBehavior
6226-
type: bool
6227-
value: true
6228-
mirror: always
6229-
62306220
# This pref governs whether we run webextensions in a separate process (true)
62316221
# or the parent/main process (false)
62326222
- name: extensions.webextensions.remote

‎testing/web-platform/meta/html/webappapis/structured-clone/structured-clone-cross-realm-method.html.ini‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[structured-clone-cross-realm-method.html]
2+
[Object instance]
3+
expected: FAIL
4+
5+
[Array instance]
6+
expected: FAIL
7+
8+
[Date instance]
9+
expected: FAIL
10+
11+
[RegExp instance]
12+
expected: FAIL

‎toolkit/components/extensions/ExtensionContent.sys.mjs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,20 +1039,16 @@ export class ContentScriptContextChild extends BaseContext {
10391039
addonId: extensionPrincipal.addonId,
10401040
};
10411041

1042-
// Within a content script, we want the 'structuredClone' method to clone
1043-
// the object within the content script's global, rather than cloning it
1044-
// into the the embedding scope. (see bug 2017797)
1045-
let wantGlobalProperties = ["structuredClone"];
1046-
10471042
let isMV2 = extension.manifestVersion == 2;
1043+
let wantGlobalProperties;
10481044
let sandboxContentSecurityPolicy;
10491045
if (isMV2) {
10501046
// In MV2, fetch/XHR support cross-origin requests.
10511047
// WebSocket was also included to avoid CSP effects (bug 1676024).
1052-
wantGlobalProperties.push("XMLHttpRequest", "fetch", "WebSocket");
1048+
wantGlobalProperties = ["XMLHttpRequest", "fetch", "WebSocket"];
10531049
} else {
10541050
// In MV3, fetch/XHR have the same capabilities as the web page.
1055-
1051+
wantGlobalProperties = [];
10561052
// In MV3, the base CSP is enforced for content scripts. Overrides are
10571053
// currently not supported, but this was considered at some point, see
10581054
// https://bugzilla.mozilla.org/show_bug.cgi?id=1581611#c10

‎toolkit/components/extensions/ExtensionUserScriptsContent.sys.mjs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,6 @@ class WorldCollection {
258258
wantXrays: true,
259259
isWebExtensionContentScript: true,
260260
wantExportHelpers: true,
261-
// The 'structuredClone' method in content scripts should clone within the
262-
// content script global by default, rather than cloning into the target
263-
// contentWindow.
264-
wantGlobalProperties: ["structuredClone"],
265261
originAttributes: docPrincipal.originAttributes,
266262
});
267263

‎toolkit/components/extensions/test/xpcshell/test_ext_contentscript_structured_clone.js‎

Lines changed: 0 additions & 149 deletions
This file was deleted.

‎toolkit/components/extensions/test/xpcshell/xpcshell-common.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,6 @@ skip-if = [
191191

192192
["test_ext_contentscript_slow_frame.js"]
193193

194-
["test_ext_contentscript_structured_clone.js"]
195-
196194
["test_ext_contentscript_teardown.js"]
197195
skip-if = [
198196
"tsan", # Bug 1683730

0 commit comments

Comments
 (0)