From 46024cd87255b400884624e972795d0fb30660f7 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 16 Oct 2023 01:10:26 -0500 Subject: [PATCH 1/3] Issue #1442 Follow-up: Stop pretending proxies have a JSNative call/construct hook. https://bugzilla.mozilla.org/show_bug.cgi?id=1471924 Part 1 Also remove an erroneous debug assert and guard against future issues. This fixes crashes on vk.com but still does not behave correctly. --- dom/workers/WorkerPrivate.cpp | 2 -- js/src/builtin/Stream.cpp | 2 +- js/src/jit/BaselineIC.cpp | 3 --- js/src/jscntxtinlines.h | 17 ++--------------- js/src/jsobj.cpp | 32 ++++++++++---------------------- js/src/proxy/Proxy.cpp | 18 ------------------ js/src/vm/Interpreter.cpp | 21 +++++++++++++++++---- 7 files changed, 30 insertions(+), 65 deletions(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 3b3de7e3b5..51ab67f708 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -6226,8 +6226,6 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) // Guard against recursion. mRunningExpiredTimeouts = true; - MOZ_DIAGNOSTIC_ASSERT(data->mCurrentTimerNestingLevel == 0); - // Run expired timeouts. for (uint32_t index = 0; index < expiredTimeouts.Length(); index++) { TimeoutInfo*& info = expiredTimeouts[index]; diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp index c8d8e3e324..26457709d8 100644 --- a/js/src/builtin/Stream.cpp +++ b/js/src/builtin/Stream.cpp @@ -2790,7 +2790,7 @@ ReadableStreamControllerCallPullIfNeeded(JSContext* cx, HandleNativeObject contr } else { pullPromise = PromiseInvokeOrNoop(cx, underlyingSource, cx->names().pull, controllerVal); } - if (!pullPromise) + if (!pullPromise || !pullPromise->is()) return false; RootedObject onPullFulfilled(cx, NewHandler(cx, ControllerPullHandler, controller)); diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index b657359d3e..a6adc122c7 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -5593,9 +5593,6 @@ TryAttachCallStub(JSContext* cx, ICCall_Fallback* stub, HandleScript script, jsb RootedObject obj(cx, &callee.toObject()); if (!obj->is()) { // Try to attach a stub for a call/construct hook on the object. - // Ignore proxies, which are special cased by callHook/constructHook. - if (obj->is()) - return true; if (JSNative hook = constructing ? obj->constructHook() : obj->callHook()) { if (op != JSOP_FUNAPPLY && !isSpread && !createSingleton) { RootedObject templateObject(cx); diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h index c89ef86ec2..d5bd0dc46e 100644 --- a/js/src/jscntxtinlines.h +++ b/js/src/jscntxtinlines.h @@ -278,22 +278,9 @@ CallJSNativeConstructor(JSContext* cx, Native native, const CallArgs& args) * constructor to return the callee, the assertion can be removed or * (another) conjunct can be added to the antecedent. * - * Exceptions: - * - * - Proxies are exceptions to both rules: they can return primitives and - * they allow content to return the callee. - * - * - CallOrConstructBoundFunction is an exception as well because we might - * have used bind on a proxy function. - * - * - new Iterator(x) is user-hookable; it returns x.__iterator__() which - * could be any object. - * - * - (new Object(Object)) returns the callee. + * Exception: (new Object(Object)) returns the callee. */ - MOZ_ASSERT_IF(native != js::proxy_Construct && - native != js::IteratorConstructor && - (!callee->is() || callee->as().native() != obj_construct), + MOZ_ASSERT_IF((!callee->is() || callee->as().native() != obj_construct), args.rval().isObject() && callee != &args.rval().toObject()); return true; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 730805e038..46f89e547e 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -2066,6 +2066,10 @@ JSObject::isCallable() const { if (is()) return true; + if (is()) { + const js::ProxyObject& p = as(); + return p.handler()->isCallable(const_cast(this)); + } return callHook() != nullptr; } @@ -2076,39 +2080,23 @@ JSObject::isConstructor() const const JSFunction& fun = as(); return fun.isConstructor(); } + if (is()) { + const js::ProxyObject& p = as(); + return p.handler()->isConstructor(const_cast(this)); + } return constructHook() != nullptr; } JSNative JSObject::callHook() const { - const js::Class* clasp = getClass(); - - if (JSNative call = clasp->getCall()) - return call; - - if (is()) { - const js::ProxyObject& p = as(); - if (p.handler()->isCallable(const_cast(this))) - return js::proxy_Call; - } - return nullptr; + return getClass()->getCall(); } JSNative JSObject::constructHook() const { - const js::Class* clasp = getClass(); - - if (JSNative construct = clasp->getConstruct()) - return construct; - - if (is()) { - const js::ProxyObject& p = as(); - if (p.handler()->isConstructor(const_cast(this))) - return js::proxy_Construct; - } - return nullptr; + return getClass()->getConstruct(); } bool diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 8f6a262106..984e1f411d 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -665,24 +665,6 @@ js::proxy_HasInstance(JSContext* cx, HandleObject proxy, MutableHandleValue v, b return Proxy::hasInstance(cx, proxy, v, bp); } -bool -js::proxy_Call(JSContext* cx, unsigned argc, Value* vp) -{ - CallArgs args = CallArgsFromVp(argc, vp); - RootedObject proxy(cx, &args.callee()); - MOZ_ASSERT(proxy->is()); - return Proxy::call(cx, proxy, args); -} - -bool -js::proxy_Construct(JSContext* cx, unsigned argc, Value* vp) -{ - CallArgs args = CallArgsFromVp(argc, vp); - RootedObject proxy(cx, &args.callee()); - MOZ_ASSERT(proxy->is()); - return Proxy::construct(cx, proxy, args); -} - bool js::proxy_GetElements(JSContext* cx, HandleObject proxy, uint32_t begin, uint32_t end, ElementAdder* adder) diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index f97ba99272..ad52234a31 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -457,10 +457,18 @@ js::InternalCallOrConstruct(JSContext* cx, const CallArgs& args, MaybeConstruct /* Invoke non-functions. */ if (MOZ_UNLIKELY(!args.callee().is())) { - MOZ_ASSERT_IF(construct, !args.callee().constructHook()); - JSNative call = args.callee().callHook(); - if (!call) + MOZ_ASSERT_IF(construct, !args.callee().isConstructor()); + + if (!args.callee().isCallable()) return ReportIsNotFunction(cx, args.calleev(), skipForCallee, construct); + + if (args.callee().is()) { + RootedObject proxy(cx, &args.callee()); + return Proxy::call(cx, proxy, args); + } + + JSNative call = args.callee().callHook(); + MOZ_ASSERT(call, "isCallable without a callHook?"); return CallJSNative(cx, call, args); } @@ -579,6 +587,11 @@ InternalConstruct(JSContext* cx, const AnyConstructArgs& args) return true; } + if (callee.is()) { + RootedObject proxy(cx, &callee); + return Proxy::construct(cx, proxy, args); + } + JSNative construct = callee.constructHook(); MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?"); @@ -4846,7 +4859,7 @@ js::SpreadCallOperation(JSContext* cx, HandleScript script, jsbytecode* pc, Hand constructing ? CONSTRUCT : NO_CONSTRUCT); } - if (MOZ_UNLIKELY(!callee.toObject().is()) && !callee.toObject().callHook()) { + if (!callee.toObject().isCallable()) { return ReportIsNotFunction(cx, callee, 2 + constructing, constructing ? CONSTRUCT : NO_CONSTRUCT); } From 84ca5fdfc71e696caf0c3525e4ca1a480fbe6bd9 Mon Sep 17 00:00:00 2001 From: Basilisk-Dev Date: Mon, 16 Oct 2023 10:13:10 -0400 Subject: [PATCH 2/3] PR #2347 - Backport of Mozilla tooltip bugs: Bug 148624 - only show tooltip when document has focus Partial Bug 1857513 - Use window activeness rather than document.hasFocus() to display chrome-only tooltips --- layout/xul/nsXULTooltipListener.cpp | 79 +++++++++++++++-------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/layout/xul/nsXULTooltipListener.cpp b/layout/xul/nsXULTooltipListener.cpp index 110f8c0e09..20e105dd55 100644 --- a/layout/xul/nsXULTooltipListener.cpp +++ b/layout/xul/nsXULTooltipListener.cpp @@ -390,51 +390,54 @@ nsXULTooltipListener::ShowTooltip() return NS_ERROR_FAILURE; // the target node doesn't need a tooltip // set the node in the document that triggered the tooltip and show it - nsCOMPtr xulDoc = - do_QueryInterface(tooltipNode->GetComposedDoc()); - if (xulDoc) { - // Make sure the target node is still attached to some document. - // It might have been deleted. - if (sourceNode->IsInComposedDoc()) { + // Make sure the document still has focus. + nsIDocument* doc = tooltipNode->GetComposedDoc(); + if (!doc || !nsContentUtils::IsChromeDoc(doc) || + doc->GetDocumentState().HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE)) { + return NS_OK; + } + + // Make sure the target node is still attached to some document. + // It might have been deleted. + if (sourceNode->IsInComposedDoc()) { #ifdef MOZ_XUL - if (!mIsSourceTree) { - mLastTreeRow = -1; - mLastTreeCol = nullptr; - } + if (!mIsSourceTree) { + mLastTreeRow = -1; + mLastTreeCol = nullptr; + } #endif - mCurrentTooltip = do_GetWeakReference(tooltipNode); - LaunchTooltip(); - mTargetNode = nullptr; + mCurrentTooltip = do_GetWeakReference(tooltipNode); + LaunchTooltip(); + mTargetNode = nullptr; - nsCOMPtr currentTooltip = do_QueryReferent(mCurrentTooltip); - if (!currentTooltip) - return NS_OK; + nsCOMPtr currentTooltip = do_QueryReferent(mCurrentTooltip); + if (!currentTooltip) + return NS_OK; - // listen for popuphidden on the tooltip node, so that we can - // be sure DestroyPopup is called even if someone else closes the tooltip - currentTooltip->AddSystemEventListener(NS_LITERAL_STRING("popuphiding"), - this, false, false); + // listen for popuphidden on the tooltip node, so that we can + // be sure DestroyPopup is called even if someone else closes the tooltip + currentTooltip->AddSystemEventListener(NS_LITERAL_STRING("popuphiding"), + this, false, false); - // listen for mousedown, mouseup, keydown, and DOMMouseScroll events at document level - nsIDocument* doc = sourceNode->GetComposedDoc(); - if (doc) { - // Probably, we should listen to untrusted events for hiding tooltips - // on content since tooltips might disturb something of web - // applications. If we don't specify the aWantsUntrusted of - // AddSystemEventListener(), the event target sets it to TRUE if the - // target is in content. - doc->AddSystemEventListener(NS_LITERAL_STRING("DOMMouseScroll"), - this, true); - doc->AddSystemEventListener(NS_LITERAL_STRING("mousedown"), - this, true); - doc->AddSystemEventListener(NS_LITERAL_STRING("mouseup"), - this, true); - doc->AddSystemEventListener(NS_LITERAL_STRING("keydown"), - this, true); - } - mSourceNode = nullptr; + // listen for mousedown, mouseup, keydown, and DOMMouseScroll events at document level + nsIDocument* doc = sourceNode->GetComposedDoc(); + if (doc) { + // Probably, we should listen to untrusted events for hiding tooltips + // on content since tooltips might disturb something of web + // applications. If we don't specify the aWantsUntrusted of + // AddSystemEventListener(), the event target sets it to TRUE if the + // target is in content. + doc->AddSystemEventListener(NS_LITERAL_STRING("DOMMouseScroll"), + this, true); + doc->AddSystemEventListener(NS_LITERAL_STRING("mousedown"), + this, true); + doc->AddSystemEventListener(NS_LITERAL_STRING("mouseup"), + this, true); + doc->AddSystemEventListener(NS_LITERAL_STRING("keydown"), + this, true); } + mSourceNode = nullptr; } return NS_OK; From 0315aeee7351efa84836138ebeaac74ac6345c00 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 17 Oct 2023 16:26:18 +0200 Subject: [PATCH 3/3] Issue #1721 - Follow-up: Properly protect GPC header Make it a singleton so it cannot be overwritten. --- netwerk/protocol/http/nsHttpHeaderArray.h | 1 + 1 file changed, 1 insertion(+) diff --git a/netwerk/protocol/http/nsHttpHeaderArray.h b/netwerk/protocol/http/nsHttpHeaderArray.h index b65b36fcdd..0a93e94ff6 100644 --- a/netwerk/protocol/http/nsHttpHeaderArray.h +++ b/netwerk/protocol/http/nsHttpHeaderArray.h @@ -234,6 +234,7 @@ nsHttpHeaderArray::IsSingletonHeader(nsHttpAtom header) header == nsHttp::From || header == nsHttp::Location || header == nsHttp::Max_Forwards || + header == nsHttp::GlobalPrivacyControl || // Ignore-multiple-headers are singletons in the sense that they // shouldn't be merged. IsIgnoreMultipleHeader(header);