From 864f723c0c7372dd1f95de458d735f15e810d2de Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 28 Apr 2015 12:25:55 -0400 Subject: [PATCH] Bug 1155946 part 2. Add mayResolve methods to DOM classes with resolve hooks. r=peterv --- dom/base/Navigator.cpp | 23 ++++++++++ dom/base/Navigator.h | 3 ++ dom/base/nsGlobalWindow.cpp | 40 +++++++++++++++++ dom/base/nsGlobalWindow.h | 3 ++ dom/base/nsJSEnvironment.cpp | 6 +++ dom/base/nsJSEnvironment.h | 3 ++ dom/base/nsObjectLoadingContent.cpp | 8 ++++ dom/base/nsObjectLoadingContent.h | 4 ++ dom/bindings/BindingUtils.cpp | 6 +++ dom/bindings/BindingUtils.h | 3 ++ dom/bindings/Codegen.py | 50 +++++++++++++++++---- js/src/devtools/rootAnalysis/annotations.js | 6 +++ 12 files changed, 146 insertions(+), 9 deletions(-) diff --git a/dom/base/Navigator.cpp b/dom/base/Navigator.cpp index 276c80736052..64c638d71d62 100644 --- a/dom/base/Navigator.cpp +++ b/dom/base/Navigator.cpp @@ -2087,6 +2087,7 @@ Navigator::DoResolve(JSContext* aCx, JS::Handle aObject, JS::Handle aId, JS::MutableHandle aDesc) { + // Note: Keep this in sync with MayResolve. if (!JSID_IS_STRING(aId)) { return true; } @@ -2238,6 +2239,28 @@ Navigator::DoResolve(JSContext* aCx, JS::Handle aObject, return true; } +/* static */ +bool +Navigator::MayResolve(jsid aId) +{ + // Note: This function does not fail and may not have any side-effects. + // Note: Keep this in sync with DoResolve. + if (!JSID_IS_STRING(aId)) { + return false; + } + + nsScriptNameSpaceManager *nameSpaceManager = PeekNameSpaceManager(); + if (!nameSpaceManager) { + // Really shouldn't happen here. Fail safe. + return true; + } + + nsAutoString name; + AssignJSFlatString(name, JSID_TO_FLAT_STRING(aId)); + + return nameSpaceManager->LookupNavigatorName(name); +} + struct NavigatorNameEnumeratorClosure { NavigatorNameEnumeratorClosure(JSContext* aCx, JSObject* aWrapper, diff --git a/dom/base/Navigator.h b/dom/base/Navigator.h index 91fa175c3818..38cd75d35eed 100644 --- a/dom/base/Navigator.h +++ b/dom/base/Navigator.h @@ -281,6 +281,9 @@ public: bool DoResolve(JSContext* aCx, JS::Handle aObject, JS::Handle aId, JS::MutableHandle aDesc); + // The return value is whether DoResolve might end up resolving the given id. + // If in doubt, return true. + static bool MayResolve(jsid aId); void GetOwnPropertyNames(JSContext* aCx, nsTArray& aNames, ErrorResult& aRv); void GetLanguages(nsTArray& aLanguages); diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 5a0670bb0f22..ac77462b844d 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -4266,6 +4266,8 @@ nsGlobalWindow::DoResolve(JSContext* aCx, JS::Handle aObj, { MOZ_ASSERT(IsInnerWindow()); + // Note: Keep this in sync with MayResolve. + // Note: The infallibleInit call in GlobalResolve depends on this check. if (!JSID_IS_STRING(aId)) { return true; @@ -4279,6 +4281,44 @@ nsGlobalWindow::DoResolve(JSContext* aCx, JS::Handle aObj, return true; } +/* static */ +bool +nsGlobalWindow::MayResolve(jsid aId) +{ + // Note: This function does not fail and may not have any side-effects. + // Note: Keep this in sync with DoResolve. + if (!JSID_IS_STRING(aId)) { + return false; + } + + if (aId == XPCJSRuntime::Get()->GetStringID(XPCJSRuntime::IDX_COMPONENTS)) { + return true; + } + + if (aId == XPCJSRuntime::Get()->GetStringID(XPCJSRuntime::IDX_CONTROLLERS)) { + // We only resolve .controllers in release builds and on non-chrome windows, + // but let's not worry about any of that stuff. + return true; + } + + nsScriptNameSpaceManager *nameSpaceManager = PeekNameSpaceManager(); + if (!nameSpaceManager) { + // Really shouldn't happen. Fail safe. + return true; + } + + nsAutoString name; + AssignJSFlatString(name, JSID_TO_FLAT_STRING(aId)); + + const nsGlobalNameStruct *name_struct = + nameSpaceManager->LookupName(name); + + // LookupName only returns structs for the global. + MOZ_ASSERT_IF(name_struct, + name_struct->mType != nsGlobalNameStruct::eTypeNavigatorProperty); + return name_struct; +} + struct GlobalNameEnumeratorClosure { GlobalNameEnumeratorClosure(JSContext* aCx, nsGlobalWindow* aWindow, diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 9570665638c9..cff6c4d9b8c5 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -502,6 +502,9 @@ public: bool DoResolve(JSContext* aCx, JS::Handle aObj, JS::Handle aId, JS::MutableHandle aDesc); + // The return value is whether DoResolve might end up resolving the given id. + // If in doubt, return true. + static bool MayResolve(jsid aId); void GetOwnPropertyNames(JSContext* aCx, nsTArray& aNames, mozilla::ErrorResult& aRv); diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index cb1d8218d5af..56d2dff3c70b 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -2826,6 +2826,12 @@ mozilla::dom::GetNameSpaceManager() return gNameSpaceManager; } +nsScriptNameSpaceManager* +mozilla::dom::PeekNameSpaceManager() +{ + return gNameSpaceManager; +} + void mozilla::dom::ShutdownJSEnvironment() { diff --git a/dom/base/nsJSEnvironment.h b/dom/base/nsJSEnvironment.h index e87192a52379..4f2dfadf9231 100644 --- a/dom/base/nsJSEnvironment.h +++ b/dom/base/nsJSEnvironment.h @@ -182,6 +182,9 @@ void ShutdownJSEnvironment(); // Get the NameSpaceManager, creating if necessary nsScriptNameSpaceManager* GetNameSpaceManager(); +// Peek the NameSpaceManager, without creating it. +nsScriptNameSpaceManager* PeekNameSpaceManager(); + // Runnable that's used to do async error reporting class AsyncErrorReporter : public nsRunnable { diff --git a/dom/base/nsObjectLoadingContent.cpp b/dom/base/nsObjectLoadingContent.cpp index 32d81f3fe429..c89456ed27ea 100644 --- a/dom/base/nsObjectLoadingContent.cpp +++ b/dom/base/nsObjectLoadingContent.cpp @@ -3667,6 +3667,14 @@ nsObjectLoadingContent::DoResolve(JSContext* aCx, JS::Handle aObject, return true; } +/* static */ +bool +nsObjectLoadingContent::MayResolve(jsid aId) +{ + // We can resolve anything, really. + return true; +} + void nsObjectLoadingContent::GetOwnPropertyNames(JSContext* aCx, nsTArray& /* unused */, diff --git a/dom/base/nsObjectLoadingContent.h b/dom/base/nsObjectLoadingContent.h index 535547375a58..05aa20306344 100644 --- a/dom/base/nsObjectLoadingContent.h +++ b/dom/base/nsObjectLoadingContent.h @@ -171,6 +171,10 @@ class nsObjectLoadingContent : public nsImageLoadingContent bool DoResolve(JSContext* aCx, JS::Handle aObject, JS::Handle aId, JS::MutableHandle aDesc); + // The return value is whether DoResolve might end up resolving the given + // id. If in doubt, return true. + static bool MayResolve(jsid aId); + // Helper for WebIDL enumeration void GetOwnPropertyNames(JSContext* aCx, nsTArray& /* unused */, mozilla::ErrorResult& aRv); diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index be9c89e3aca0..29b0d09f6c33 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -2436,6 +2436,12 @@ ResolveGlobal(JSContext* aCx, JS::Handle aObj, return JS_ResolveStandardClass(aCx, aObj, aId, aResolvedp); } +bool +MayResolveGlobal(const JSAtomState& aNames, jsid aId, JSObject* aMaybeObj) +{ + return JS_MayResolveStandardClass(aNames, aId, aMaybeObj); +} + bool EnumerateGlobal(JSContext* aCx, JS::Handle aObj) { diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index 0adba74d687c..5f29d3d09d14 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -2968,6 +2968,9 @@ bool ResolveGlobal(JSContext* aCx, JS::Handle aObj, JS::Handle aId, bool* aResolvedp); +bool +MayResolveGlobal(const JSAtomState& aNames, jsid aId, JSObject* aMaybeObj); + bool EnumerateGlobal(JSContext* aCx, JS::Handle aObj); diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 4db8650ef701..5c8b5d19d2ec 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -23,6 +23,7 @@ CONSTRUCT_HOOK_NAME = '_constructor' LEGACYCALLER_HOOK_NAME = '_legacycaller' HASINSTANCE_HOOK_NAME = '_hasInstance' RESOLVE_HOOK_NAME = '_resolve' +MAY_RESOLVE_HOOK_NAME = '_mayResolve' ENUMERATE_HOOK_NAME = '_enumerate' ENUM_ENTRY_VARIABLE_NAME = 'strings' INSTANCE_RESERVED_SLOTS = 1 @@ -457,12 +458,15 @@ class CGDOMJSClass(CGThing): reservedSlots = slotCount if self.descriptor.interface.getExtendedAttribute("NeedResolve"): resolveHook = RESOLVE_HOOK_NAME + mayResolveHook = MAY_RESOLVE_HOOK_NAME enumerateHook = ENUMERATE_HOOK_NAME elif self.descriptor.isGlobal(): resolveHook = "mozilla::dom::ResolveGlobal" + mayResolveHook = "mozilla::dom::MayResolveGlobal" enumerateHook = "mozilla::dom::EnumerateGlobal" else: resolveHook = "nullptr" + mayResolveHook = "nullptr" enumerateHook = "nullptr" return fill( @@ -476,7 +480,7 @@ class CGDOMJSClass(CGThing): nullptr, /* setProperty */ ${enumerate}, /* enumerate */ ${resolve}, /* resolve */ - nullptr, /* mayResolve */ + ${mayResolve}, /* mayResolve */ nullptr, /* convert */ ${finalize}, /* finalize */ ${call}, /* call */ @@ -498,6 +502,7 @@ class CGDOMJSClass(CGThing): addProperty=ADDPROPERTY_HOOK_NAME if wantsAddProperty(self.descriptor) else 'nullptr', enumerate=enumerateHook, resolve=resolveHook, + mayResolve=mayResolveHook, finalize=FINALIZE_HOOK_NAME, call=callHook, trace=traceHook, @@ -7769,7 +7774,7 @@ class CGLegacyCallHook(CGAbstractBindingMethod): self._legacycaller) -class CGResolveHook(CGAbstractBindingMethod): +class CGResolveHook(CGAbstractClassHook): """ Resolve hook for objects that have the NeedResolve extended attribute. """ @@ -7780,13 +7785,11 @@ class CGResolveHook(CGAbstractBindingMethod): Argument('JS::Handle', 'obj'), Argument('JS::Handle', 'id'), Argument('bool*', 'resolvedp')] - # Our "self" is actually the "obj" argument in this case, not the thisval. - CGAbstractBindingMethod.__init__( - self, descriptor, RESOLVE_HOOK_NAME, - args, getThisObj="", callArgs="") + CGAbstractClassHook.__init__(self, descriptor, RESOLVE_HOOK_NAME, + "bool", args) def generate_code(self): - return CGGeneric(dedent(""" + return dedent(""" JS::Rooted desc(cx); if (!self->DoResolve(cx, obj, id, &desc)) { return false; @@ -7803,7 +7806,7 @@ class CGResolveHook(CGAbstractBindingMethod): } *resolvedp = true; return true; - """)) + """) def definition_body(self): if self.descriptor.isGlobal(): @@ -7819,7 +7822,35 @@ class CGResolveHook(CGAbstractBindingMethod): """) else: prefix = "" - return prefix + CGAbstractBindingMethod.definition_body(self) + return prefix + CGAbstractClassHook.definition_body(self) + + +class CGMayResolveHook(CGAbstractStaticMethod): + """ + Resolve hook for objects that have the NeedResolve extended attribute. + """ + def __init__(self, descriptor): + assert descriptor.interface.getExtendedAttribute("NeedResolve") + + args = [Argument('const JSAtomState&', 'names'), + Argument('jsid', 'id'), + Argument('JSObject*', 'maybeObj')] + CGAbstractStaticMethod.__init__(self, descriptor, MAY_RESOLVE_HOOK_NAME, + "bool", args) + + def definition_body(self): + if self.descriptor.isGlobal(): + # Check whether this would resolve as a standard class. + prefix = dedent(""" + if (MayResolveGlobal(names, id, maybeObj)) { + return true; + } + + """) + else: + prefix = "" + return (prefix + + "return %s::MayResolve(id);\n" % self.descriptor.nativeType) class CGEnumerateHook(CGAbstractBindingMethod): @@ -11348,6 +11379,7 @@ class CGDescriptor(CGThing): cgThings.append(CGLegacyCallHook(descriptor)) if descriptor.interface.getExtendedAttribute("NeedResolve"): cgThings.append(CGResolveHook(descriptor)) + cgThings.append(CGMayResolveHook(descriptor)) cgThings.append(CGEnumerateHook(descriptor)) if descriptor.hasNamedPropertiesObject: diff --git a/js/src/devtools/rootAnalysis/annotations.js b/js/src/devtools/rootAnalysis/annotations.js index effcd4ca9a87..e99c354b0a56 100644 --- a/js/src/devtools/rootAnalysis/annotations.js +++ b/js/src/devtools/rootAnalysis/annotations.js @@ -174,6 +174,12 @@ var ignoreFunctions = { // And these are workarounds to avoid even more analysis work, // which would sadly still be needed even with bug 898815. "void js::AutoCompartment::AutoCompartment(js::ExclusiveContext*, JSCompartment*)": true, + + // The nsScriptNameSpaceManager functions can't actually GC. They + // just use a pldhash which has function pointers, which makes the + // analysis think maybe they can. + "nsGlobalNameStruct* nsScriptNameSpaceManager::LookupNavigatorName(nsAString_internal*)": true, + "nsGlobalNameStruct* nsScriptNameSpaceManager::LookupName(nsAString_internal*, uint16**)": true, }; function ignoreGCFunction(mangled)