Skip to content

Commit a388a81

Browse files
committed
Revert "Bug 2017797 - Part 3: Preserve legacy structuredClone behaviour for WebExtensions, r=robwu,asuth" for causing build bustages at Sandbox.cpp
This reverts commit 08e7de1. This reverts commit 4a57bda. This reverts commit 18a34f9.
1 parent 7e7e0d5 commit a388a81

File tree

9 files changed

+23
-227
lines changed

9 files changed

+23
-227
lines changed

dom/base/nsContentUtils.cpp

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

11897-
// 2.7.10 Structured cloning API
11898-
// Step 1: Let serialized be
11899-
// ? StructuredSerializeWithTransfer(value, options["transfer"])
1190011897
StructuredCloneHolder holder(StructuredCloneHolder::CloningSupported,
1190111898
StructuredCloneHolder::TransferringSupported,
1190211899
JS::StructuredCloneScope::SameProcess);
@@ -11905,16 +11902,11 @@ void nsContentUtils::StructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal,
1190511902
return;
1190611903
}
1190711904

11908-
// Step 2: Let deserializeRecord be
11909-
// ? StructuredDeserializeWithTransfer(serialized, this's relevant realm).
11910-
JSAutoRealm ar(aCx, aGlobal->GetGlobalJSObject());
1191111905
holder.Read(aCx, aRetval, clonePolicy, aError);
1191211906
if (NS_WARN_IF(aError.Failed())) {
1191311907
return;
1191411908
}
1191511909

11916-
// Step 3: Return deserializeRecord.[[Deserialized]].
11917-
// (discarding deserializeRecord.[[TransferredValues]])
1191811910
nsTArray<RefPtr<MessagePort>> ports = holder.TakeTransferredPorts();
1191911911
(void)ports;
1192011912
}

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 & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -379,29 +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-
IS_INSTANCE_OF(RTCEncodedVideoFrame, obj) ||
396-
IS_INSTANCE_OF(RTCEncodedAudioFrame, obj) ||
397-
IS_INSTANCE_OF(MessagePort, obj) ||
398-
IS_INSTANCE_OF(OffscreenCanvas, obj) ||
399-
IS_INSTANCE_OF(ReadableStream, obj) ||
400-
IS_INSTANCE_OF(WritableStream, obj) ||
401-
IS_INSTANCE_OF(TransformStream, obj) ||
402-
IS_INSTANCE_OF(RTCDataChannel, obj);
403-
}
404-
405382
static bool SandboxStructuredClone(JSContext* cx, unsigned argc, Value* vp) {
406383
CallArgs args = CallArgsFromVp(argc, vp);
407384

@@ -410,43 +387,25 @@ static bool SandboxStructuredClone(JSContext* cx, unsigned argc, Value* vp) {
410387
}
411388

412389
RootedDictionary<dom::StructuredSerializeOptions> options(cx);
390+
BindingCallContext callCx(cx, "structuredClone");
413391
if (!options.Init(cx, args.hasDefined(1) ? args[1] : JS::NullHandleValue,
414392
"Argument 2", false)) {
415393
return false;
416394
}
417395

418-
// NOTE: A spec-compliant structuredClone should determine & use the relevant
419-
// global instead of the current global.
420-
nsCOMPtr<nsIGlobalObject> global = CurrentNativeGlobal(cx);
396+
nsIGlobalObject* global = CurrentNativeGlobal(cx);
421397
if (!global) {
422398
JS_ReportErrorASCII(cx, "structuredClone: Missing global");
423399
return false;
424400
}
425401

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

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

modules/libpref/init/StaticPrefList.yaml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6210,16 +6210,6 @@
62106210
#endif
62116211
mirror: always
62126212

6213-
# Prior to bug 2013389 and bug 2017797, calling structuredClone within a
6214-
# content script would clone most objects within the content script's realm,
6215-
# however a subset of DOM objects would be cloned into the underlying content
6216-
# window's realm. When enabled, this pref preserves this legacy behaviour for
6217-
# compatibility with existing content scripts.
6218-
- name: extensions.webextensions.legacyStructuredCloneBehavior
6219-
type: bool
6220-
value: true
6221-
mirror: always
6222-
62236213
# This pref governs whether we run webextensions in a separate process (true)
62246214
# or the parent/main process (false)
62256215
- name: extensions.webextensions.remote
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)