From e24465fc4babd05e45eb8ebd1b7f8997788e3cd2 Mon Sep 17 00:00:00 2001 From: Gene Lian Date: Thu, 8 Aug 2013 16:16:50 +0800 Subject: [PATCH 01/19] Bug 901887 - [RIL][Contacts] Saving to SIM doesn't work. r=allstars.chh --- dom/system/gonk/ril_worker.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dom/system/gonk/ril_worker.js b/dom/system/gonk/ril_worker.js index 734fc425cdfe..0a96da671655 100644 --- a/dom/system/gonk/ril_worker.js +++ b/dom/system/gonk/ril_worker.js @@ -12591,10 +12591,11 @@ let ICCContactHelper = { return; } - // Check if contact has additional properties (email, anr, ...etc) need - // to be updated as well. + // Check if contact has additional properties (email, anr, ...etc) that + // need to be updated as well. if ((field === USIM_PBR_EMAIL && !contact.email) || - (field === USIM_PBR_ANR0 && !contact.anr[0])) { + (field === USIM_PBR_ANR0 && (!Array.isArray(contact.anr) || + !contact.anr[0]))) { updateField(); return; } From e5cd57f08c5daf045d55e14dce34f09e908a7222 Mon Sep 17 00:00:00 2001 From: Gene Lian Date: Thu, 8 Aug 2013 16:36:09 +0800 Subject: [PATCH 02/19] Bug 901992 - B2G MMS: Retry Mechanism for Downloading MMS is Buggy. r=vicamo a=leo+ --- dom/mobilemessage/src/gonk/MmsService.js | 31 ++++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/dom/mobilemessage/src/gonk/MmsService.js b/dom/mobilemessage/src/gonk/MmsService.js index 0e7ed3f3acc7..e5d59b4d05eb 100644 --- a/dom/mobilemessage/src/gonk/MmsService.js +++ b/dom/mobilemessage/src/gonk/MmsService.js @@ -817,24 +817,25 @@ RetrieveTransaction.prototype = Object.create(CancellableTransaction.prototype, this.registerRunCallback(callback); this.retryCount = 0; - let that = this; - this.retrieve((function retryCallback(mmsStatus, msg) { + let retryCallback = (function (mmsStatus, msg) { if (MMS.MMS_PDU_STATUS_DEFERRED == mmsStatus && - that.retryCount < PREF_RETRIEVAL_RETRY_COUNT) { - if (that.timer == null) { - that.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); + this.retryCount < PREF_RETRIEVAL_RETRY_COUNT) { + let time = PREF_RETRIEVAL_RETRY_INTERVALS[this.retryCount]; + if (DEBUG) debug("Fail to retrieve. Will retry after: " + time); + + if (this.timer == null) { + this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); } - that.timer.initWithCallback((function (){ - this.retrieve(retryCallback); - }).bind(that), - PREF_RETRIEVAL_RETRY_INTERVALS[that.retryCount], - Ci.nsITimer.TYPE_ONE_SHOT); - that.retryCount++; + this.timer.initWithCallback(this.retrieve.bind(this, retryCallback), + time, Ci.nsITimer.TYPE_ONE_SHOT); + this.retryCount++; return; } this.runCallbackIfValid(mmsStatus, msg); - }).bind(this)); + }).bind(this); + + this.retrieve(retryCallback); }, enumerable: true, configurable: true, @@ -1043,6 +1044,10 @@ SendTransaction.prototype = Object.create(CancellableTransaction.prototype, { if ((MMS.MMS_PDU_ERROR_TRANSIENT_FAILURE == mmsStatus || MMS.MMS_PDU_ERROR_PERMANENT_FAILURE == mmsStatus) && this.retryCount < PREF_SEND_RETRY_COUNT) { + if (DEBUG) { + debug("Fail to send. Will retry after: " + PREF_SEND_RETRY_INTERVAL); + } + if (this.timer == null) { this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); } @@ -1400,7 +1405,7 @@ MmsService.prototype = { // Retrieved fail after retry, so we update the delivery status in DB and // notify this domMessage that error happen. gMobileMessageDatabaseService - .setMessageDeliveryByMessageId(id, + .setMessageDeliveryByMessageId(savableMessage.id, null, null, DELIVERY_STATUS_ERROR, From 8b81c4a1452a48e9f2f3ada615aeca056a1eeb3a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 7 Aug 2013 17:40:00 -0400 Subject: [PATCH 03/19] Bug 901816. Include Nullable and TypedArray headers in binding header files if we have dictionaries that have those as members. r=smaug --- dom/bindings/Codegen.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index e4827d9d78a0..b818d46b17f0 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -597,6 +597,9 @@ class CGHeaders(CGWrapper): dictionary, if passed, to decide what to do with interface types. """ assert not descriptor or not dictionary + if t.nullable() and dictionary: + # Need to make sure that Nullable as a dictionary member works + declareIncludes.add("mozilla/dom/Nullable.h") unrolled = t.unroll() if unrolled.isUnion(): # UnionConversions.h includes UnionTypes.h @@ -604,7 +607,11 @@ class CGHeaders(CGWrapper): elif unrolled.isInterface(): if unrolled.isSpiderMonkeyInterface(): bindingHeaders.add("jsfriendapi.h") - bindingHeaders.add("mozilla/dom/TypedArray.h") + if dictionary: + headerSet = declareIncludes + else: + headerSet = bindingHeaders + headerSet.add("mozilla/dom/TypedArray.h") else: providers = getRelevantProviders(descriptor, dictionary, config) From 0404e39cd69cfcceb6e17df1e8092e97aa0b5254 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 7 Aug 2013 17:40:00 -0400 Subject: [PATCH 04/19] Bug 902485. Disallow copy constructors and operator= on WebIDL union structs, because those wouldn't do what you think they should. r=dzbarsky --- dom/bindings/Codegen.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index b818d46b17f0..3b701803f8f9 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -6105,6 +6105,10 @@ ${methods} private: friend class ${structName}Argument; + // Disallow copy-construction and assignment + ${structName}(const ${structName}&) MOZ_DELETE; + void operator=(const ${structName}&) MOZ_DELETE; + ${destructors} enum Type { @@ -6228,6 +6232,10 @@ ${methods} JS::MutableHandle rval) const; private: + // Disallow copy-construction and assignment + ${structName}ReturnValue(const ${structName}ReturnValue&) MOZ_DELETE; + void operator=(const ${structName}ReturnValue&) MOZ_DELETE; + enum Type { eUninitialized, ${enumValues} From 443795d1eb9e84a17f91671585d5f27090970b4b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 7 Aug 2013 17:40:00 -0400 Subject: [PATCH 05/19] Bug 897913 part 1. Don't assert that app id exists when asked for app status; just claim not installed if there is no app id. r=sicking --- caps/src/nsPrincipal.cpp | 6 ++---- content/base/src/nsDocument.cpp | 37 ++++++++++++--------------------- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/caps/src/nsPrincipal.cpp b/caps/src/nsPrincipal.cpp index be7449fff440..d592e8955efa 100644 --- a/caps/src/nsPrincipal.cpp +++ b/caps/src/nsPrincipal.cpp @@ -442,8 +442,6 @@ nsPrincipal::GetExtendedOrigin(nsACString& aExtendedOrigin) NS_IMETHODIMP nsPrincipal::GetAppStatus(uint16_t* aAppStatus) { - MOZ_ASSERT(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID); - *aAppStatus = GetAppStatus(); return NS_OK; } @@ -569,8 +567,8 @@ nsPrincipal::Write(nsIObjectOutputStream* aStream) uint16_t nsPrincipal::GetAppStatus() { - MOZ_ASSERT(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID); - + NS_WARN_IF_FALSE(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID, + "Asking for app status on a principal with an unknown app id"); // Installed apps have a valid app id (not NO_APP_ID or UNKNOWN_APP_ID) // and they are not inside a mozbrowser. if (mAppId == nsIScriptSecurityManager::NO_APP_ID || diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index d56e2cdd73c8..6aedc85cb848 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -2545,37 +2545,26 @@ nsDocument::InitCSP(nsIChannel* aChannel) } // Figure out if we need to apply an app default CSP or a CSP from an app manifest - bool applyAppDefaultCSP = false; - bool applyAppManifestCSP = false; - nsIPrincipal* principal = NodePrincipal(); - bool unknownAppId; - uint16_t appStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED; - nsAutoString appManifestCSP; - if (NS_SUCCEEDED(principal->GetUnknownAppId(&unknownAppId)) && - !unknownAppId && - NS_SUCCEEDED(principal->GetAppStatus(&appStatus))) { - applyAppDefaultCSP = ( appStatus == nsIPrincipal::APP_STATUS_PRIVILEGED || - appStatus == nsIPrincipal::APP_STATUS_CERTIFIED); + uint16_t appStatus = principal->GetAppStatus(); + bool applyAppDefaultCSP = appStatus == nsIPrincipal::APP_STATUS_PRIVILEGED || + appStatus == nsIPrincipal::APP_STATUS_CERTIFIED; + bool applyAppManifestCSP = false; - if (appStatus != nsIPrincipal::APP_STATUS_NOT_INSTALLED) { - nsCOMPtr appsService = do_GetService(APPS_SERVICE_CONTRACTID); - if (appsService) { - uint32_t appId = 0; - if (NS_SUCCEEDED(principal->GetAppId(&appId))) { - appsService->GetCSPByLocalId(appId, appManifestCSP); - if (!appManifestCSP.IsEmpty()) { - applyAppManifestCSP = true; - } + nsAutoString appManifestCSP; + if (appStatus != nsIPrincipal::APP_STATUS_NOT_INSTALLED) { + nsCOMPtr appsService = do_GetService(APPS_SERVICE_CONTRACTID); + if (appsService) { + uint32_t appId = 0; + if (NS_SUCCEEDED(principal->GetAppId(&appId))) { + appsService->GetCSPByLocalId(appId, appManifestCSP); + if (!appManifestCSP.IsEmpty()) { + applyAppManifestCSP = true; } } } } -#ifdef PR_LOGGING - else - PR_LOG(gCspPRLog, PR_LOG_DEBUG, ("Failed to get app status from principal")); -#endif // If there's no CSP to apply, go ahead and return early if (!applyAppDefaultCSP && From a91edf513822b027f3bfd2aa6f52c562d0467376 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 7 Aug 2013 17:40:00 -0400 Subject: [PATCH 06/19] Bug 897913 part 2. Allow touching interface objects via an Xray even if the page they're in can't touch them. r=bholley,smaug --- dom/base/nsDOMClassInfo.cpp | 54 ++++++++++++++++++++--------------- dom/bindings/BindingUtils.cpp | 14 +++++---- dom/bindings/BindingUtils.h | 8 +++++- dom/bindings/Codegen.py | 36 ++++++++++++----------- js/xpconnect/src/xpcpublic.h | 2 +- 5 files changed, 66 insertions(+), 48 deletions(-) diff --git a/dom/base/nsDOMClassInfo.cpp b/dom/base/nsDOMClassInfo.cpp index 8dea4d77ed88..06a756e92067 100644 --- a/dom/base/nsDOMClassInfo.cpp +++ b/dom/base/nsDOMClassInfo.cpp @@ -3540,6 +3540,14 @@ nsWindowSH::GlobalResolve(nsGlobalWindow *aWin, JSContext *cx, JS::Rooted global(cx); bool defineOnXray = xpc::WrapperFactory::IsXrayWrapper(obj); if (defineOnXray) { + // Check whether to define this property on the Xray first. This allows + // consumers to opt in to defining on the xray even if they don't want + // to define on the underlying global. + if (name_struct->mConstructorEnabled && + !(*name_struct->mConstructorEnabled)(cx, obj)) { + return NS_OK; + } + global = js::CheckedUnwrap(obj, /* stopAtOuter = */ false); if (!global) { return NS_ERROR_DOM_SECURITY_ERR; @@ -3549,36 +3557,36 @@ nsWindowSH::GlobalResolve(nsGlobalWindow *aWin, JSContext *cx, global = obj; } - // Check whether our constructor is enabled after we unwrap Xrays, since - // we don't want to define an interface on the Xray if it's disabled in - // the target global, even if it's enabled in the Xray's global. - if (name_struct->mConstructorEnabled && - !(*name_struct->mConstructorEnabled)(cx, global)) { + // Check whether to define on the global too. Note that at this point cx + // is in the compartment of global even if we were coming in via an Xray. + bool defineOnGlobal = !name_struct->mConstructorEnabled || + (*name_struct->mConstructorEnabled)(cx, global); + + if (!defineOnGlobal && !defineOnXray) { return NS_OK; } - bool enabled; - JS::Rooted interfaceObject(cx, define(cx, global, id, &enabled)); - if (enabled) { - if (!interfaceObject) { + JS::Rooted interfaceObject(cx, define(cx, global, id, + defineOnGlobal)); + if (!interfaceObject) { + return NS_ERROR_FAILURE; + } + + if (defineOnXray) { + // This really should be handled by the Xray for the window. + ac.destroy(); + if (!JS_WrapObject(cx, interfaceObject.address()) || + !JS_DefinePropertyById(cx, obj, id, + JS::ObjectValue(*interfaceObject), JS_PropertyStub, + JS_StrictPropertyStub, 0)) { return NS_ERROR_FAILURE; } - if (defineOnXray) { - // This really should be handled by the Xray for the window. - ac.destroy(); - if (!JS_WrapObject(cx, interfaceObject.address()) || - !JS_DefinePropertyById(cx, obj, id, - JS::ObjectValue(*interfaceObject), JS_PropertyStub, - JS_StrictPropertyStub, 0)) { - return NS_ERROR_FAILURE; - } - } - - *did_resolve = true; - - return NS_OK; } + + *did_resolve = true; + + return NS_OK; } } diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index 7d4ffbca03d7..348875d34508 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -371,7 +371,7 @@ CreateInterfaceObject(JSContext* cx, JS::Handle global, JS::Handle proto, const NativeProperties* properties, const NativeProperties* chromeOnlyProperties, - const char* name) + const char* name, bool defineOnGlobal) { JS::Rooted constructor(cx); if (constructorClass) { @@ -455,7 +455,7 @@ CreateInterfaceObject(JSContext* cx, JS::Handle global, return NULL; } - if (!DefineConstructor(cx, global, name, constructor)) { + if (defineOnGlobal && !DefineConstructor(cx, global, name, constructor)) { return nullptr; } @@ -471,8 +471,9 @@ CreateInterfaceObject(JSContext* cx, JS::Handle global, JS::ObjectValue(*proto), JS_PropertyStub, JS_StrictPropertyStub, JSPROP_PERMANENT | JSPROP_READONLY) || - !DefineConstructor(cx, global, namedConstructors->mName, - namedConstructor)) { + (defineOnGlobal && + !DefineConstructor(cx, global, namedConstructors->mName, + namedConstructor))) { return nullptr; } js::SetReservedSlot(constructor, namedConstructorSlot++, @@ -562,7 +563,7 @@ CreateInterfaceObjects(JSContext* cx, JS::Handle global, JS::Heap* constructorCache, const DOMClass* domClass, const NativeProperties* properties, const NativeProperties* chromeOnlyProperties, - const char* name) + const char* name, bool defineOnGlobal) { MOZ_ASSERT(protoClass || constructorClass || constructor, "Need at least one class or a constructor!"); @@ -612,7 +613,8 @@ CreateInterfaceObjects(JSContext* cx, JS::Handle global, interface = CreateInterfaceObject(cx, global, constructorProto, constructorClass, constructor, ctorNargs, namedConstructors, proto, - properties, chromeOnlyProperties, name); + properties, chromeOnlyProperties, name, + defineOnGlobal); if (!interface) { if (protoCache) { // If we fail we need to make sure to clear the value of protoCache we diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index f0be93b2c183..7ee25bb63aea 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -364,6 +364,12 @@ struct NamedConstructor * on objects in chrome compartments. This must be null if the * interface doesn't have any ChromeOnly properties or if the * object is being created in non-chrome compartment. + * defineOnGlobal controls whether properties should be defined on the given + * global for the interface object (if any) and named + * constructors (if any) for this interface. This can be + * false in situations where we want the properties to only + * appear on privileged Xrays but not on the unprivileged + * underlying global. * * At least one of protoClass, constructorClass or constructor should be * non-null. If constructorClass or constructor are non-null, the resulting @@ -380,7 +386,7 @@ CreateInterfaceObjects(JSContext* cx, JS::Handle global, JS::Heap* constructorCache, const DOMClass* domClass, const NativeProperties* regularProperties, const NativeProperties* chromeOnlyProperties, - const char* name); + const char* name, bool defineOnGlobal); /* * Define the unforgeable attributes on an object. diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 3b701803f8f9..a5d688c4c244 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -1664,7 +1664,8 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod): def __init__(self, descriptor, properties): args = [Argument('JSContext*', 'aCx'), Argument('JS::Handle', 'aGlobal'), - Argument('JS::Heap*', 'protoAndIfaceArray')] + Argument('JS::Heap*', 'aProtoAndIfaceArray'), + Argument('bool', 'aDefineOnGlobal')] CGAbstractMethod.__init__(self, descriptor, 'CreateInterfaceObjects', 'void', args) self.properties = properties def definition_body(self): @@ -1791,13 +1792,13 @@ if (!unforgeableHolder) { if needInterfacePrototypeObject: protoClass = "&PrototypeClass.mBase" - protoCache = "&protoAndIfaceArray[prototypes::id::%s]" % self.descriptor.name + protoCache = "&aProtoAndIfaceArray[prototypes::id::%s]" % self.descriptor.name else: protoClass = "nullptr" protoCache = "nullptr" if needInterfaceObject: interfaceClass = "&InterfaceObjectClass.mBase" - interfaceCache = "&protoAndIfaceArray[constructors::id::%s]" % self.descriptor.name + interfaceCache = "&aProtoAndIfaceArray[constructors::id::%s]" % self.descriptor.name else: # We don't have slots to store the named constructors. assert len(self.descriptor.interface.namedConstructors) == 0 @@ -1828,7 +1829,7 @@ if (!unforgeableHolder) { " %s,\n" " %s,\n" " %s,\n" - " %s);" % ( + " %s, aDefineOnGlobal);" % ( protoClass, protoCache, interfaceClass, constructHookHolder, constructArgs, namedConstructors, @@ -1840,7 +1841,7 @@ if (!unforgeableHolder) { if UseHolderForUnforgeable(self.descriptor): assert needInterfacePrototypeObject setUnforgeableHolder = CGGeneric( - "JSObject* proto = protoAndIfaceArray[prototypes::id::%s];\n" + "JSObject* proto = aProtoAndIfaceArray[prototypes::id::%s];\n" "if (proto) {\n" " js::SetReservedSlot(proto, DOM_INTERFACE_PROTO_SLOTS_BASE,\n" " JS::ObjectValue(*unforgeableHolder));\n" @@ -1858,9 +1859,9 @@ class CGGetPerInterfaceObject(CGAbstractMethod): A method for getting a per-interface object (a prototype object or interface constructor object). """ - def __init__(self, descriptor, name, idPrefix=""): + def __init__(self, descriptor, name, idPrefix="", extraArgs=[]): args = [Argument('JSContext*', 'aCx'), - Argument('JS::Handle', 'aGlobal')] + Argument('JS::Handle', 'aGlobal')] + extraArgs CGAbstractMethod.__init__(self, descriptor, name, 'JS::Handle', args, inline=True) self.id = idPrefix + "id::" + self.descriptor.name @@ -1874,7 +1875,7 @@ class CGGetPerInterfaceObject(CGAbstractMethod): /* Check to see whether the interface objects are already installed */ JS::Heap* protoAndIfaceArray = GetProtoAndIfaceArray(aGlobal); if (!protoAndIfaceArray[%s]) { - CreateInterfaceObjects(aCx, aGlobal, protoAndIfaceArray); + CreateInterfaceObjects(aCx, aGlobal, protoAndIfaceArray, aDefineOnGlobal); } /* @@ -1897,15 +1898,18 @@ class CGGetProtoObjectMethod(CGGetPerInterfaceObject): def definition_body(self): return """ /* Get the interface prototype object for this class. This will create the - object as needed. */""" + CGGetPerInterfaceObject.definition_body(self) + object as needed. */ + bool aDefineOnGlobal = true;""" + CGGetPerInterfaceObject.definition_body(self) class CGGetConstructorObjectMethod(CGGetPerInterfaceObject): """ A method for getting the interface constructor object. """ def __init__(self, descriptor): - CGGetPerInterfaceObject.__init__(self, descriptor, "GetConstructorObject", - "constructors::") + CGGetPerInterfaceObject.__init__( + self, descriptor, "GetConstructorObject", + "constructors::", + extraArgs=[Argument("bool", "aDefineOnGlobal", "true")]) def definition_body(self): return """ /* Get the interface object for this class. This will create the object as @@ -1920,7 +1924,7 @@ class CGDefineDOMInterfaceMethod(CGAbstractMethod): args = [Argument('JSContext*', 'aCx'), Argument('JS::Handle', 'aGlobal'), Argument('JS::Handle', 'id'), - Argument('bool*', 'aEnabled')] + Argument('bool', 'aDefineOnGlobal')] CGAbstractMethod.__init__(self, descriptor, 'DefineDOMInterface', 'JSObject*', args) def declare(self): @@ -1935,7 +1939,7 @@ class CGDefineDOMInterfaceMethod(CGAbstractMethod): def definition_body(self): if len(self.descriptor.interface.namedConstructors) > 0: - getConstructor = """ JSObject* interfaceObject = GetConstructorObject(aCx, aGlobal); + getConstructor = """ JSObject* interfaceObject = GetConstructorObject(aCx, aGlobal, aDefineOnGlobal); if (!interfaceObject) { return nullptr; } @@ -1947,10 +1951,8 @@ class CGDefineDOMInterfaceMethod(CGAbstractMethod): } return interfaceObject;""" else: - getConstructor = " return GetConstructorObject(aCx, aGlobal);" - return (""" *aEnabled = true; - -""" + getConstructor) + getConstructor = " return GetConstructorObject(aCx, aGlobal, aDefineOnGlobal);" + return getConstructor class CGConstructorEnabledViaPrefEnabled(CGAbstractMethod): """ diff --git a/js/xpconnect/src/xpcpublic.h b/js/xpconnect/src/xpcpublic.h index dedaded837b0..5a4f1a35d99a 100644 --- a/js/xpconnect/src/xpcpublic.h +++ b/js/xpconnect/src/xpcpublic.h @@ -457,7 +457,7 @@ inline bool IsDOMProxy(JSObject *obj) typedef JSObject* (*DefineInterface)(JSContext *cx, JS::Handle global, - JS::Handle id, bool *enabled); + JS::Handle id, bool defineOnGlobal); typedef JSObject* (*ConstructNavigatorProperty)(JSContext *cx, JS::Handle naviObj); From 3c1b29f83503c40d6115a48de5b26dd07d008226 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 7 Aug 2013 17:40:20 -0400 Subject: [PATCH 07/19] Bug 897913 part 3. Enable Promise in chrome and certified apps, even when preffed off. r=bholley, pending review from baku --- dom/promise/Makefile.in | 5 +++++ dom/promise/Promise.cpp | 28 ++++++++++++++++++++++------ dom/promise/Promise.h | 1 + dom/webidl/Promise.webidl | 13 ++++++++++--- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/dom/promise/Makefile.in b/dom/promise/Makefile.in index c5c17c6d1b6c..18cb01cb2fbe 100644 --- a/dom/promise/Makefile.in +++ b/dom/promise/Makefile.in @@ -13,4 +13,9 @@ LIBRARY_NAME = dompromise_s LIBXUL_LIBRARY = 1 FAIL_ON_WARNINGS := 1 +LOCAL_INCLUDES += \ + -I$(topsrcdir)/dom/workers \ + -I$(topsrcdir)/dom/base \ + $(NULL) + include $(topsrcdir)/config/rules.mk diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index 88d88b2c65c8..805b2e6f115c 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -11,6 +11,8 @@ #include "PromiseCallback.h" #include "nsContentUtils.h" #include "nsPIDOMWindow.h" +#include "WorkerPrivate.h" +#include "nsJSPrincipals.h" namespace mozilla { namespace dom { @@ -110,6 +112,26 @@ Promise::PrefEnabled() return Preferences::GetBool("dom.promise.enabled", false); } +/* static */ bool +Promise::EnabledForScope(JSContext* aCx, JSObject* /* unused */) +{ + // Enable if the pref is enabled or if we're chrome or if we're a + // certified app. + if (PrefEnabled()) { + return true; + } + + // Note that we have no concept of a certified app in workers. + // XXXbz well, why not? + if (!NS_IsMainThread()) { + return workers::GetWorkerPrivateFromContext(aCx)->IsChromeWorker(); + } + + nsIPrincipal* prin = nsContentUtils::GetSubjectPrincipal(); + return nsContentUtils::IsSystemPrincipal(prin) || + prin->GetAppStatus() == nsIPrincipal::APP_STATUS_CERTIFIED; +} + static void EnterCompartment(Maybe& aAc, JSContext* aCx, const Optional >& aValue) @@ -125,8 +147,6 @@ EnterCompartment(Maybe& aAc, JSContext* aCx, Promise::Constructor(const GlobalObject& aGlobal, JSContext* aCx, PromiseInit& aInit, ErrorResult& aRv) { - MOZ_ASSERT(PrefEnabled()); - nsCOMPtr window = do_QueryInterface(aGlobal.Get()); if (!window) { aRv.Throw(NS_ERROR_UNEXPECTED); @@ -155,8 +175,6 @@ Promise::Constructor(const GlobalObject& aGlobal, JSContext* aCx, Promise::Resolve(const GlobalObject& aGlobal, JSContext* aCx, JS::Handle aValue, ErrorResult& aRv) { - MOZ_ASSERT(PrefEnabled()); - nsCOMPtr window = do_QueryInterface(aGlobal.Get()); if (!window) { aRv.Throw(NS_ERROR_UNEXPECTED); @@ -174,8 +192,6 @@ Promise::Resolve(const GlobalObject& aGlobal, JSContext* aCx, Promise::Reject(const GlobalObject& aGlobal, JSContext* aCx, JS::Handle aValue, ErrorResult& aRv) { - MOZ_ASSERT(PrefEnabled()); - nsCOMPtr window = do_QueryInterface(aGlobal.Get()); if (!window) { aRv.Throw(NS_ERROR_UNEXPECTED); diff --git a/dom/promise/Promise.h b/dom/promise/Promise.h index cf1ba6adfbe3..d3e2e3f2a576 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -41,6 +41,7 @@ public: ~Promise(); static bool PrefEnabled(); + static bool EnabledForScope(JSContext* aCx, JSObject* /* unused */); // WebIDL diff --git a/dom/webidl/Promise.webidl b/dom/webidl/Promise.webidl index e974c85bd16d..3b5f5d4efff9 100644 --- a/dom/webidl/Promise.webidl +++ b/dom/webidl/Promise.webidl @@ -7,6 +7,7 @@ * http://dom.spec.whatwg.org/#promises */ +[Func="mozilla::dom::Promise::EnabledForScope"] interface PromiseResolver { // TODO bug 875289 - void fulfill(optional any value); void resolve(optional any value); @@ -16,12 +17,18 @@ interface PromiseResolver { callback PromiseInit = void (PromiseResolver resolver); callback AnyCallback = any (optional any value); -[PrefControlled, Constructor(PromiseInit init)] +[Func="mozilla::dom::Promise::EnabledForScope", Constructor(PromiseInit init)] interface Promise { // TODO bug 875289 - static Promise fulfill(any value); - [Creator, Throws] + + // Disable the static methods when the interface object is supposed to be + // disabled, just in case some code decides to walk over to .constructor from + // the proto of a promise object or someone screws up and manages to create a + // Promise object in this scope without having resolved the interface object + // first. + [Creator, Throws, Func="mozilla::dom::Promise::EnabledForScope"] static Promise resolve(any value); // same as any(value) - [Creator, Throws] + [Creator, Throws, Func="mozilla::dom::Promise::EnabledForScope"] static Promise reject(any value); [Creator] From 070be8a915c7843d805ca7237f2fade37b569caa Mon Sep 17 00:00:00 2001 From: Gabor Krizsanits Date: Thu, 8 Aug 2013 14:23:33 +0200 Subject: [PATCH 08/19] Bug 877164 - nsDocument should call GetJunkScope later. r=smaug --- content/base/src/nsDocument.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index 6aedc85cb848..720559fc0235 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -1953,15 +1953,6 @@ nsDocument::Init() mRadioGroups.Init(); mCustomPrototypes.Init(); - // If after creation the owner js global is not set for a document - // we use the default compartment for this document, instead of creating - // wrapper in some random compartment when the document is exposed to js - // via some events. - nsCOMPtr global = xpc::GetJunkScopeGlobal(); - NS_ENSURE_TRUE(global, NS_ERROR_FAILURE); - mScopeObject = do_GetWeakReference(global); - MOZ_ASSERT(mScopeObject); - // Force initialization. nsINode::nsSlots* slots = Slots(); @@ -1992,6 +1983,15 @@ nsDocument::Init() NS_ASSERTION(OwnerDoc() == this, "Our nodeinfo is busted!"); + // If after creation the owner js global is not set for a document + // we use the default compartment for this document, instead of creating + // wrapper in some random compartment when the document is exposed to js + // via some events. + nsCOMPtr global = xpc::GetJunkScopeGlobal(); + NS_ENSURE_TRUE(global, NS_ERROR_FAILURE); + mScopeObject = do_GetWeakReference(global); + MOZ_ASSERT(mScopeObject); + mScriptLoader = new nsScriptLoader(this); mImageTracker.Init(); From cac2f1bc2fa0976b25c279cccf2663443e6228f1 Mon Sep 17 00:00:00 2001 From: Jan Beich Date: Thu, 8 Aug 2013 14:54:51 +0200 Subject: [PATCH 09/19] Bug 902765 - fix build after bug 894331, use mozilla::TimeStamp instead of TimeStamp r=mak --- toolkit/components/places/nsNavHistoryResult.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/places/nsNavHistoryResult.h b/toolkit/components/places/nsNavHistoryResult.h index 8c62469c04b9..b12534087a48 100644 --- a/toolkit/components/places/nsNavHistoryResult.h +++ b/toolkit/components/places/nsNavHistoryResult.h @@ -176,7 +176,7 @@ public: bool mBatchInProgress; int32_t mRelatedNotificationsCount; - TimeStamp mLastNotificationTimeStamp; + mozilla::TimeStamp mLastNotificationTimeStamp; nsCOMPtr mEndBatchTimer; void MaybeBeginBatch(); From 0cf84ed53a3f46906caa304d87334360aeedb886 Mon Sep 17 00:00:00 2001 From: Brad Lassey Date: Thu, 8 Aug 2013 09:02:02 -0400 Subject: [PATCH 10/19] bug 894313 - GeckoThread should own its own static instance r=kats --HG-- extra : rebase_source : bf7fdb0a4bd399c4edcc2046ed4c232b41a3827b --- mobile/android/base/GeckoApp.java | 11 ++++--- mobile/android/base/GeckoThread.java | 48 ++++++++++++++++++++++++---- mobile/android/base/GeckoView.java | 12 +++---- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index 65197642f228..207d0c8df6a9 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -183,7 +183,6 @@ abstract public class GeckoApp private static GeckoApp sAppContext; protected MenuPanel mMenuPanel; protected Menu mMenu; - private static GeckoThread sGeckoThread; protected GeckoProfile mProfile; public static int mOrientation; protected boolean mIsRestoringActivity; @@ -1213,7 +1212,7 @@ abstract public class GeckoApp return; } - if (sGeckoThread != null) { + if (GeckoThread.isCreated()) { // This happens when the GeckoApp activity is destroyed by Android // without killing the entire application (see Bug 769269). mIsRestoringActivity = true; @@ -1438,18 +1437,20 @@ abstract public class GeckoApp Telemetry.HistogramAdd("FENNEC_STARTUP_GECKOAPP_ACTION", startupAction.ordinal()); if (!mIsRestoringActivity) { - sGeckoThread = new GeckoThread(intent, passedUri); + GeckoThread.setArgs(intent.getStringExtra("args")); + GeckoThread.setAction(intent.getAction()); + GeckoThread.setUri(passedUri); } if (!ACTION_DEBUG.equals(action) && GeckoThread.checkAndSetLaunchState(GeckoThread.LaunchState.Launching, GeckoThread.LaunchState.Launched)) { - sGeckoThread.start(); + GeckoThread.createAndStart(); } else if (ACTION_DEBUG.equals(action) && GeckoThread.checkAndSetLaunchState(GeckoThread.LaunchState.Launching, GeckoThread.LaunchState.WaitForDebugger)) { ThreadUtils.getUiHandler().postDelayed(new Runnable() { @Override public void run() { GeckoThread.setLaunchState(GeckoThread.LaunchState.Launching); - sGeckoThread.start(); + GeckoThread.createAndStart(); } }, 1000 * 5 /* 5 seconds */); } diff --git a/mobile/android/base/GeckoThread.java b/mobile/android/base/GeckoThread.java index 7e1beb926375..e0e66294404c 100644 --- a/mobile/android/base/GeckoThread.java +++ b/mobile/android/base/GeckoThread.java @@ -38,16 +38,53 @@ public class GeckoThread extends Thread implements GeckoEventListener { private static LaunchState sLaunchState = LaunchState.Launching; - private Intent mIntent; + private static GeckoThread sGeckoThread; + + private final String mArgs; + private final String mAction; private final String mUri; - GeckoThread(Intent intent, String uri) { - mIntent = intent; + public static boolean ensureInit() { + ThreadUtils.assertOnUiThread(); + if (isCreated()) + return false; + sGeckoThread = new GeckoThread(sArgs, sAction, sUri); + return true; + } + + public static String sArgs; + public static String sAction; + public static String sUri; + + public static void setArgs(String args) { + sArgs = args; + } + + public static void setAction(String action) { + sAction = action; + } + + public static void setUri(String uri) { + sUri = uri; + } + + GeckoThread(String args, String action, String uri) { + mArgs = args; + mAction = action; mUri = uri; setName("Gecko"); GeckoAppShell.getEventDispatcher().registerEventListener("Gecko:Ready", this); } + public static boolean isCreated() { + return sGeckoThread != null; + } + + public static void createAndStart() { + if (ensureInit()) + sGeckoThread.start(); + } + private String initGeckoEnvironment() { // At some point while loading the gecko libs our default locale gets set // so just save it to locale here and reset it as default after the join @@ -123,9 +160,8 @@ public class GeckoThread extends Thread implements GeckoEventListener { Log.w(LOGTAG, "zerdatime " + SystemClock.uptimeMillis() + " - runGecko"); - String args = addCustomProfileArg(mIntent.getStringExtra("args")); - String type = getTypeFromAction(mIntent.getAction()); - mIntent = null; + String args = addCustomProfileArg(mArgs); + String type = getTypeFromAction(mAction); // and then fire us up Log.i(LOGTAG, "RunGecko - args = " + args); diff --git a/mobile/android/base/GeckoView.java b/mobile/android/base/GeckoView.java index a9008386bec7..cb5b7ac0b619 100644 --- a/mobile/android/base/GeckoView.java +++ b/mobile/android/base/GeckoView.java @@ -28,7 +28,6 @@ import android.os.Handler; public class GeckoView extends LayerView implements GeckoEventListener, ContextGetter { - static GeckoThread sGeckoThread; public GeckoView(Context context, AttributeSet attrs) { super(context, attrs); @@ -37,11 +36,9 @@ public class GeckoView extends LayerView String url = a.getString(R.styleable.GeckoView_url); a.recycle(); - Intent intent; - if (url == null) { - intent = new Intent(Intent.ACTION_MAIN); - } else { - intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url)); + if (url != null) { + GeckoThread.setUri(url); + GeckoThread.setAction(Intent.ACTION_VIEW); GeckoAppShell.sendEventToGecko(GeckoEvent.createURILoadEvent(url)); } GeckoAppShell.setContextGetter(this); @@ -53,12 +50,11 @@ public class GeckoView extends LayerView BrowserDB.initialize(profile.getName()); GeckoAppShell.registerEventListener("Gecko:Ready", this); - sGeckoThread = new GeckoThread(intent, url); ThreadUtils.setUiThread(Thread.currentThread(), new Handler()); initializeView(GeckoAppShell.getEventDispatcher()); if (GeckoThread.checkAndSetLaunchState(GeckoThread.LaunchState.Launching, GeckoThread.LaunchState.Launched)) { GeckoAppShell.setLayerView(this); - sGeckoThread.start(); + GeckoThread.createAndStart(); } } From e7fe914e75a314a66f583febf8b1ab24d05376e6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 8 Aug 2013 06:33:49 -0700 Subject: [PATCH 11/19] Bug 902820 - Fix a bunch of compile warnings in SpiderMonkey. r=till. --HG-- extra : rebase_source : 0306d26443640104bae575a60fec7a693f7b43c5 --- js/public/RootingAPI.h | 4 ++++ js/src/builtin/BinaryData.cpp | 4 +++- js/src/gc/Zone.cpp | 2 ++ js/src/ion/shared/CodeGenerator-x86-shared.cpp | 3 ++- js/src/jscntxtinlines.h | 4 ++++ js/src/jsfun.cpp | 2 +- js/src/jsobj.cpp | 2 +- js/src/jsproxy.cpp | 6 +++--- js/src/jsworkers.h | 2 ++ js/src/vm/ThreadPool.cpp | 5 ++++- js/src/vm/ThreadPool.h | 2 ++ 11 files changed, 28 insertions(+), 8 deletions(-) diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index 99295f123882..67c26f0267e7 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -856,6 +856,10 @@ class SkipRoot void init(js::ContextFriendFields *cx, const T *ptr, size_t count) {} public: + ~SkipRoot() { + // An empty destructor is needed to avoid warnings from clang about + // unused local variables of this type. + } #endif /* DEBUG && JSGC_ROOT_ANALYSIS */ diff --git a/js/src/builtin/BinaryData.cpp b/js/src/builtin/BinaryData.cpp index f0155af8463c..344b1487fbce 100644 --- a/js/src/builtin/BinaryData.cpp +++ b/js/src/builtin/BinaryData.cpp @@ -23,6 +23,8 @@ #include "jsatominlines.h" #include "jsobjinlines.h" +using mozilla::DebugOnly; + using namespace js; /* @@ -1552,7 +1554,7 @@ StructType::layout(JSContext *cx, HandleObject structType, HandleObject fields) // If the field type is a BinaryType and we can't get its bytes, we have a problem. RootedValue fieldTypeBytes(cx); - bool r = JSObject::getProperty(cx, fieldType, fieldType, cx->names().bytes, &fieldTypeBytes); + DebugOnly r = JSObject::getProperty(cx, fieldType, fieldType, cx->names().bytes, &fieldTypeBytes); JS_ASSERT(r); JS_ASSERT(fieldTypeBytes.isInt32()); diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index c3f07e58e725..cc4ca41c80d2 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -18,6 +18,8 @@ #include "jsgcinlines.h" +#include "vm/ObjectImpl-inl.h" + using namespace js; using namespace js::gc; diff --git a/js/src/ion/shared/CodeGenerator-x86-shared.cpp b/js/src/ion/shared/CodeGenerator-x86-shared.cpp index 3d65ec41bcdf..5a3fb234164a 100644 --- a/js/src/ion/shared/CodeGenerator-x86-shared.cpp +++ b/js/src/ion/shared/CodeGenerator-x86-shared.cpp @@ -376,9 +376,10 @@ CodeGeneratorX86Shared::visitMinMaxD(LMinMaxD *ins) { FloatRegister first = ToFloatRegister(ins->first()); FloatRegister second = ToFloatRegister(ins->second()); +#ifdef DEBUG FloatRegister output = ToFloatRegister(ins->output()); - JS_ASSERT(first == output); +#endif Label done, nan, minMaxInst; diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h index fea20621da6f..b11840b9ca60 100644 --- a/js/src/jscntxtinlines.h +++ b/js/src/jscntxtinlines.h @@ -433,6 +433,10 @@ class AutoLockForExclusiveAccess AutoLockForExclusiveAccess(JSRuntime *rt MOZ_GUARD_OBJECT_NOTIFIER_PARAM) { MOZ_GUARD_OBJECT_NOTIFIER_INIT; } + ~AutoLockForExclusiveAccess() { + // An empty destructor is needed to avoid warnings from clang about + // unused local variables of this type. + } #endif // JS_THREADSAFE MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 2fa56bcd0293..fadc6a622a05 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -507,7 +507,7 @@ Class JSFunction::class_ = { fun_trace }; -JS_FRIEND_DATA(Class* const) js::FunctionClassPtr = &JSFunction::class_; +Class* const js::FunctionClassPtr = &JSFunction::class_; /* Find the body of a function (not including braces). */ static bool diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 527514296f84..792231fb6e36 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -80,7 +80,7 @@ Class JSObject::class_ = { JS_ConvertStub }; -JS_FRIEND_DATA(Class* const) js::ObjectClassPtr = &JSObject::class_; +Class* const js::ObjectClassPtr = &JSObject::class_; JS_FRIEND_API(JSObject *) JS_ObjectToInnerObject(JSContext *cx, JSObject *objArg) diff --git a/js/src/jsproxy.cpp b/js/src/jsproxy.cpp index 4101908bbe19..ee1df97431bc 100644 --- a/js/src/jsproxy.cpp +++ b/js/src/jsproxy.cpp @@ -3133,7 +3133,7 @@ Class js::ObjectProxyObject::class_ = { } }; -JS_FRIEND_DATA(Class* const) js::ObjectProxyClassPtr = &ObjectProxyObject::class_; +Class* const js::ObjectProxyClassPtr = &ObjectProxyObject::class_; Class js::OuterWindowProxyObject::class_ = { "Proxy", @@ -3192,7 +3192,7 @@ Class js::OuterWindowProxyObject::class_ = { } }; -JS_FRIEND_DATA(Class* const) js::OuterWindowProxyClassPtr = &OuterWindowProxyObject::class_; +Class* const js::OuterWindowProxyClassPtr = &OuterWindowProxyObject::class_; static bool proxy_Call(JSContext *cx, unsigned argc, Value *vp) @@ -3263,7 +3263,7 @@ Class js::FunctionProxyObject::class_ = { } }; -JS_FRIEND_DATA(Class* const) js::FunctionProxyClassPtr = &FunctionProxyObject::class_; +Class* const js::FunctionProxyClassPtr = &FunctionProxyObject::class_; /* static */ ProxyObject * ProxyObject::New(JSContext *cx, BaseProxyHandler *handler, HandleValue priv, TaggedProto proto_, diff --git a/js/src/jsworkers.h b/js/src/jsworkers.h index fcab0a960898..36dae078f0ee 100644 --- a/js/src/jsworkers.h +++ b/js/src/jsworkers.h @@ -289,8 +289,10 @@ class AutoUnlockWorkerThreadState /* Pause any threads that are running jobs off thread during GC activity. */ class AutoPauseWorkersForGC { +#ifdef JS_WORKER_THREADS JSRuntime *runtime; bool needsUnpause; +#endif MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER public: diff --git a/js/src/vm/ThreadPool.cpp b/js/src/vm/ThreadPool.cpp index faeb25e8be45..6f4c522252a9 100644 --- a/js/src/vm/ThreadPool.cpp +++ b/js/src/vm/ThreadPool.cpp @@ -186,7 +186,10 @@ ThreadPoolWorker::terminate() // them down when requested. ThreadPool::ThreadPool(JSRuntime *rt) - : runtime_(rt), + : +#if defined(JS_THREADSAFE) || defined(DEBUG) + runtime_(rt), +#endif numWorkers_(0), // updated during init() nextId_(0) { diff --git a/js/src/vm/ThreadPool.h b/js/src/vm/ThreadPool.h index eb5ff8467185..8e7f34838ace 100644 --- a/js/src/vm/ThreadPool.h +++ b/js/src/vm/ThreadPool.h @@ -70,7 +70,9 @@ class ThreadPool friend class ThreadPoolWorker; // Initialized at startup only: +#if defined(JS_THREADSAFE) || defined(DEBUG) JSRuntime *const runtime_; +#endif js::Vector workers_; // Number of workers we will start, when we actually start them From e1bc35f024d2ba2fd18a5e1f55974e79a67cf38a Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 8 Aug 2013 08:20:20 -0700 Subject: [PATCH 12/19] Bug 901658 - Introduce an uninlined version of JSScript::global() to use in Debugger.h assertions. r=njn --- js/src/jsscript.cpp | 6 ++++++ js/src/jsscript.h | 1 + js/src/vm/Debugger.h | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 86b1beea2e6e..ba8da66cd416 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -2009,6 +2009,12 @@ JSScript::isShortRunning() !analysis()->hasFunctionCalls(); } +js::GlobalObject& +JSScript::uninlinedGlobal() const +{ + return global(); +} + bool JSScript::enclosingScriptsCompiledSuccessfully() const { diff --git a/js/src/jsscript.h b/js/src/jsscript.h index 176f4326efa3..535cb6a804af 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -809,6 +809,7 @@ class JSScript : public js::gc::Cell inline void clearPropertyReadTypes(); inline js::GlobalObject &global() const; + js::GlobalObject &uninlinedGlobal() const; /* See StaticScopeIter comment. */ JSObject *enclosingStaticScope() const { diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index f14148e2c109..cba0194694dc 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -677,7 +677,7 @@ void Debugger::onNewScript(JSContext *cx, HandleScript script, GlobalObject *compileAndGoGlobal) { JS_ASSERT_IF(script->compileAndGo, compileAndGoGlobal); - JS_ASSERT_IF(script->compileAndGo, compileAndGoGlobal == &script->global()); + JS_ASSERT_IF(script->compileAndGo, compileAndGoGlobal == &script->uninlinedGlobal()); // We early return in slowPathOnNewScript for self-hosted scripts, so we can // ignore those in our assertion here. JS_ASSERT_IF(!script->compartment()->options().invisibleToDebugger && From 60f2a9dfd278ddb0c0bc03d31894de58bc47b6c8 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Thu, 8 Aug 2013 17:28:38 +0200 Subject: [PATCH 13/19] Bug 866888 part 5 - Ion-compile try-catch statements (preffed off for now). r=djvj --- js/src/ion/Bailouts.cpp | 65 ++++++++-- js/src/ion/Bailouts.h | 14 +++ js/src/ion/BaselineBailouts.cpp | 50 ++++++-- js/src/ion/BaselineCompiler.cpp | 9 +- js/src/ion/BaselineJIT.h | 3 +- js/src/ion/BytecodeAnalysis.cpp | 60 +++++++++- js/src/ion/BytecodeAnalysis.h | 5 +- js/src/ion/Ion.cpp | 15 ++- js/src/ion/Ion.h | 11 ++ js/src/ion/IonAnalysis.cpp | 6 + js/src/ion/IonBuilder.cpp | 145 +++++++++++++++++++++++ js/src/ion/IonBuilder.h | 9 +- js/src/ion/IonCode.h | 10 ++ js/src/ion/IonFrameIterator.h | 9 ++ js/src/ion/IonFrames.cpp | 74 ++++++++++-- js/src/ion/IonFrames.h | 5 + js/src/ion/MIRGraph.h | 11 +- js/src/ion/arm/MacroAssembler-arm.cpp | 11 ++ js/src/ion/x64/MacroAssembler-x64.cpp | 10 ++ js/src/ion/x86/MacroAssembler-x86.cpp | 10 ++ js/src/jit-test/tests/ion/try-catch-1.js | 12 ++ js/src/jit-test/tests/ion/try-catch-2.js | 14 +++ js/src/jit-test/tests/ion/try-catch-3.js | 27 +++++ js/src/jit-test/tests/ion/try-catch-4.js | 15 +++ js/src/shell/js.cpp | 4 + 25 files changed, 562 insertions(+), 42 deletions(-) create mode 100644 js/src/jit-test/tests/ion/try-catch-1.js create mode 100644 js/src/jit-test/tests/ion/try-catch-2.js create mode 100644 js/src/jit-test/tests/ion/try-catch-3.js create mode 100644 js/src/jit-test/tests/ion/try-catch-4.js diff --git a/js/src/ion/Bailouts.cpp b/js/src/ion/Bailouts.cpp index 7365964cec93..9c9ae3309778 100644 --- a/js/src/ion/Bailouts.cpp +++ b/js/src/ion/Bailouts.cpp @@ -140,6 +140,44 @@ ion::InvalidationBailout(InvalidationBailoutStack *sp, size_t *frameSizeOut, return retval; } +IonBailoutIterator::IonBailoutIterator(const JitActivationIterator &activations, + const IonFrameIterator &frame) + : IonFrameIterator(activations), + machine_(frame.machineState()) +{ + returnAddressToFp_ = frame.returnAddressToFp(); + topIonScript_ = frame.ionScript(); + const OsiIndex *osiIndex = frame.osiIndex(); + + current_ = (uint8_t *) frame.fp(); + type_ = IonFrame_OptimizedJS; + topFrameSize_ = frame.frameSize(); + snapshotOffset_ = osiIndex->snapshotOffset(); +} + +uint32_t +ion::ExceptionHandlerBailout(JSContext *cx, const InlineFrameIterator &frame, + const ExceptionBailoutInfo &excInfo, + BaselineBailoutInfo **bailoutInfo) +{ + JS_ASSERT(cx->isExceptionPending()); + + cx->mainThread().ionTop = NULL; + JitActivationIterator jitActivations(cx->runtime()); + IonBailoutIterator iter(jitActivations, frame.frame()); + JitActivation *activation = jitActivations.activation()->asJit(); + + *bailoutInfo = NULL; + uint32_t retval = BailoutIonToBaseline(cx, activation, iter, true, bailoutInfo, &excInfo); + JS_ASSERT(retval == BAILOUT_RETURN_OK || + retval == BAILOUT_RETURN_FATAL_ERROR || + retval == BAILOUT_RETURN_OVERRECURSED); + + JS_ASSERT((retval == BAILOUT_RETURN_OK) == (*bailoutInfo != NULL)); + + return retval; +} + // Initialize the decl env Object, call object, and any arguments obj of the current frame. bool ion::EnsureHasScopeObjects(JSContext *cx, AbstractFramePtr fp) @@ -156,19 +194,26 @@ ion::EnsureHasScopeObjects(JSContext *cx, AbstractFramePtr fp) bool ion::CheckFrequentBailouts(JSContext *cx, JSScript *script) { - // Invalidate if this script keeps bailing out without invalidation. Next time - // we compile this script LICM will be disabled. + if (script->hasIonScript()) { + // Invalidate if this script keeps bailing out without invalidation. Next time + // we compile this script LICM will be disabled. + IonScript *ionScript = script->ionScript(); - if (script->hasIonScript() && - script->ionScript()->numBailouts() >= js_IonOptions.frequentBailoutThreshold && - !script->hadFrequentBailouts) - { - script->hadFrequentBailouts = true; + if (ionScript->numBailouts() >= js_IonOptions.frequentBailoutThreshold && + !script->hadFrequentBailouts) + { + script->hadFrequentBailouts = true; - IonSpew(IonSpew_Invalidate, "Invalidating due to too many bailouts"); + IonSpew(IonSpew_Invalidate, "Invalidating due to too many bailouts"); - if (!Invalidate(cx, script)) - return false; + if (!Invalidate(cx, script)) + return false; + } else { + // If we keep bailing out to handle exceptions, invalidate and + // forbid compilation. + if (ionScript->numExceptionBailouts() >= js_IonOptions.exceptionBailoutThreshold) + ForbidCompilation(cx, script); + } } return true; diff --git a/js/src/ion/Bailouts.h b/js/src/ion/Bailouts.h index ea884173d594..af7781760db1 100644 --- a/js/src/ion/Bailouts.h +++ b/js/src/ion/Bailouts.h @@ -125,6 +125,7 @@ class IonBailoutIterator : public IonFrameIterator public: IonBailoutIterator(const JitActivationIterator &activations, BailoutStack *sp); IonBailoutIterator(const JitActivationIterator &activations, InvalidationBailoutStack *sp); + IonBailoutIterator(const JitActivationIterator &activations, const IonFrameIterator &frame); SnapshotOffset snapshotOffset() const { JS_ASSERT(topIonScript_); @@ -157,6 +158,19 @@ uint32_t Bailout(BailoutStack *sp, BaselineBailoutInfo **info); uint32_t InvalidationBailout(InvalidationBailoutStack *sp, size_t *frameSizeOut, BaselineBailoutInfo **info); +struct ExceptionBailoutInfo +{ + size_t frameNo; + jsbytecode *resumePC; + size_t numExprSlots; +}; + +// Called from the exception handler to enter a catch or finally block. +// Returns a BAILOUT_* error code. +uint32_t ExceptionHandlerBailout(JSContext *cx, const InlineFrameIterator &frame, + const ExceptionBailoutInfo &excInfo, + BaselineBailoutInfo **bailoutInfo); + uint32_t FinishBailoutToBaseline(BaselineBailoutInfo *bailoutInfo); bool CheckFrequentBailouts(JSContext *cx, JSScript *script); diff --git a/js/src/ion/BaselineBailouts.cpp b/js/src/ion/BaselineBailouts.cpp index 9de58f694895..d654520d2b75 100644 --- a/js/src/ion/BaselineBailouts.cpp +++ b/js/src/ion/BaselineBailouts.cpp @@ -464,9 +464,16 @@ InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC, HandleFunction fun, HandleScript script, IonScript *ionScript, SnapshotIterator &iter, bool invalidate, BaselineStackBuilder &builder, AutoValueVector &startFrameFormals, MutableHandleFunction nextCallee, - jsbytecode **callPC) + jsbytecode **callPC, const ExceptionBailoutInfo *excInfo) { - uint32_t exprStackSlots = iter.slots() - (script->nfixed + CountArgSlots(script, fun)); + // If excInfo is non-NULL, we are bailing out to a catch or finally block + // and this is the frame where we will resume. Usually the expression stack + // should be empty in this case but there can be iterators on the stack. + uint32_t exprStackSlots; + if (excInfo) + exprStackSlots = excInfo->numExprSlots; + else + exprStackSlots = iter.slots() - (script->nfixed + CountArgSlots(script, fun)); builder.resetFramePushed(); @@ -641,10 +648,13 @@ InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC, return false; } - // Get the PC - jsbytecode *pc = script->code + iter.pcOffset(); + // Get the pc. If we are handling an exception, resume at the pc of the + // catch or finally block. + jsbytecode *pc = excInfo ? excInfo->resumePC : script->code + iter.pcOffset(); + bool resumeAfter = excInfo ? false : iter.resumeAfter(); + JSOp op = JSOp(*pc); - bool resumeAfter = iter.resumeAfter(); + JS_ASSERT_IF(excInfo, op == JSOP_ENTERBLOCK); // Fixup inlined JSOP_FUNCALL, JSOP_FUNAPPLY, and accessors on the caller side. // On the caller side this must represent like the function wasn't inlined. @@ -808,8 +818,9 @@ InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC, BailoutKindString(bailoutKind)); #endif - // If this was the last inline frame, then unpacking is almost done. - if (!iter.moreFrames()) { + // If this was the last inline frame, or we are bailing out to a catch or + // finally block in this frame, then unpacking is almost done. + if (!iter.moreFrames() || excInfo) { // Last frame, so PC for call to next frame is set to NULL. *callPC = NULL; @@ -1167,7 +1178,8 @@ InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC, uint32_t ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIterator &iter, - bool invalidate, BaselineBailoutInfo **bailoutInfo) + bool invalidate, BaselineBailoutInfo **bailoutInfo, + const ExceptionBailoutInfo *excInfo) { JS_ASSERT(bailoutInfo != NULL); JS_ASSERT(*bailoutInfo == NULL); @@ -1214,10 +1226,17 @@ ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIt IonSpew(IonSpew_BaselineBailouts, "Bailing to baseline %s:%u (IonScript=%p) (FrameType=%d)", iter.script()->filename(), iter.script()->lineno, (void *) iter.ionScript(), (int) prevFrameType); + + if (excInfo) + IonSpew(IonSpew_BaselineBailouts, "Resuming in catch or finally block"); + IonSpew(IonSpew_BaselineBailouts, " Reading from snapshot offset %u size %u", iter.snapshotOffset(), iter.ionScript()->snapshotsSize()); - iter.ionScript()->incNumBailouts(); + if (excInfo) + iter.ionScript()->incNumExceptionBailouts(); + else + iter.ionScript()->incNumBailouts(); iter.script()->updateBaselineOrIonRaw(); // Allocate buffer to hold stack replacement data. @@ -1242,7 +1261,7 @@ ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIt IonSpew(IonSpew_BaselineBailouts, " Not constructing!"); IonSpew(IonSpew_BaselineBailouts, " Restoring frames:"); - int frameNo = 0; + size_t frameNo = 0; // Reconstruct baseline frames using the builder. RootedScript caller(cx); @@ -1250,6 +1269,7 @@ ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIt RootedFunction fun(cx, callee); RootedScript scr(cx, iter.script()); AutoValueVector startFrameFormals(cx); + while (true) { #if JS_TRACE_LOGGING if (frameNo > 0) { @@ -1258,11 +1278,16 @@ ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIt } #endif IonSpew(IonSpew_BaselineBailouts, " FrameNo %d", frameNo); + + // If we are bailing out to a catch or finally block in this frame, + // pass excInfo to InitFromBailout and don't unpack any other frames. + bool handleException = (excInfo && excInfo->frameNo == frameNo); + jsbytecode *callPC = NULL; RootedFunction nextCallee(cx, NULL); if (!InitFromBailout(cx, caller, callerPC, fun, scr, iter.ionScript(), snapIter, invalidate, builder, startFrameFormals, - &nextCallee, &callPC)) + &nextCallee, &callPC, handleException ? excInfo : NULL)) { return BAILOUT_RETURN_FATAL_ERROR; } @@ -1272,6 +1297,9 @@ ion::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIt break; } + if (handleException) + break; + JS_ASSERT(nextCallee); JS_ASSERT(callPC); caller = scr; diff --git a/js/src/ion/BaselineCompiler.cpp b/js/src/ion/BaselineCompiler.cpp index f75f9ff8e3a9..4121fa3c60f8 100644 --- a/js/src/ion/BaselineCompiler.cpp +++ b/js/src/ion/BaselineCompiler.cpp @@ -27,7 +27,7 @@ BaselineCompiler::BaselineCompiler(JSContext *cx, HandleScript script) bool BaselineCompiler::init() { - if (!analysis_.init()) + if (!analysis_.init(cx)) return false; if (!labels_.init(script->length)) @@ -465,6 +465,13 @@ BaselineCompiler::emitUseCountIncrement() masm.add32(Imm32(1), countReg); masm.store32(countReg, useCountAddr); + // If this is a loop inside a catch or finally block, increment the use + // count but don't attempt OSR (Ion only compiles the try block). + if (analysis_.info(pc).loopEntryInCatchOrFinally) { + JS_ASSERT(JSOp(*pc) == JSOP_LOOPENTRY); + return true; + } + Label skipCall; uint32_t minUses = UsesBeforeIonRecompile(script, pc); diff --git a/js/src/ion/BaselineJIT.h b/js/src/ion/BaselineJIT.h index 23a93d31c7d2..5f94fd19e804 100644 --- a/js/src/ion/BaselineJIT.h +++ b/js/src/ion/BaselineJIT.h @@ -326,7 +326,8 @@ struct BaselineBailoutInfo uint32_t BailoutIonToBaseline(JSContext *cx, JitActivation *activation, IonBailoutIterator &iter, - bool invalidate, BaselineBailoutInfo **bailoutInfo); + bool invalidate, BaselineBailoutInfo **bailoutInfo, + const ExceptionBailoutInfo *exceptionInfo = NULL); // Mark baseline scripts on the stack as active, so that they are not discarded // during GC. diff --git a/js/src/ion/BytecodeAnalysis.cpp b/js/src/ion/BytecodeAnalysis.cpp index 472b2f128298..66dfe3425bec 100644 --- a/js/src/ion/BytecodeAnalysis.cpp +++ b/js/src/ion/BytecodeAnalysis.cpp @@ -19,8 +19,25 @@ BytecodeAnalysis::BytecodeAnalysis(JSScript *script) { } +// Bytecode range containing only catch or finally code. +struct CatchFinallyRange +{ + uint32_t start; // Inclusive. + uint32_t end; // Exclusive. + + CatchFinallyRange(uint32_t start, uint32_t end) + : start(start), end(end) + { + JS_ASSERT(end > start); + } + + bool contains(uint32_t offset) const { + return start <= offset && offset < end; + } +}; + bool -BytecodeAnalysis::init() +BytecodeAnalysis::init(JSContext *cx) { if (!infos_.growByUninitialized(script_->length)) return false; @@ -31,6 +48,8 @@ BytecodeAnalysis::init() mozilla::PodZero(infos_.begin(), infos_.length()); infos_[0].init(/*stackDepth=*/0); + Vector catchFinallyRanges; + for (jsbytecode *pc = script_->code; pc < end; pc += GetBytecodeLength(pc)) { JSOp op = JSOp(*pc); unsigned offset = pc - script_->code; @@ -59,7 +78,8 @@ BytecodeAnalysis::init() // If stack depth exceeds max allowed by analysis, fail fast. JS_ASSERT(stackDepth <= BytecodeInfo::MAX_STACK_DEPTH); - if (op == JSOP_TABLESWITCH) { + switch (op) { + case JSOP_TABLESWITCH: { unsigned defaultOffset = offset + GET_JUMP_OFFSET(pc); jsbytecode *pc2 = pc + JUMP_OFFSET_LEN; int32_t low = GET_JUMP_OFFSET(pc2); @@ -78,7 +98,10 @@ BytecodeAnalysis::init() } pc2 += JUMP_OFFSET_LEN; } - } else if (op == JSOP_TRY) { + break; + } + + case JSOP_TRY: { JSTryNote *tn = script_->trynotes()->vector; JSTryNote *tnlimit = tn + script_->trynotes()->length; for (; tn < tnlimit; tn++) { @@ -92,6 +115,37 @@ BytecodeAnalysis::init() } } } + + // Get the pc of the last instruction in the try block. It's a JSOP_GOTO to + // jump over the catch/finally blocks. + jssrcnote *sn = js_GetSrcNote(cx, script_, pc); + JS_ASSERT(SN_TYPE(sn) == SRC_TRY); + + jsbytecode *endOfTry = pc + js_GetSrcNoteOffset(sn, 0); + JS_ASSERT(JSOp(*endOfTry) == JSOP_GOTO); + + jsbytecode *afterTry = endOfTry + GET_JUMP_OFFSET(endOfTry); + JS_ASSERT(afterTry > endOfTry); + + // Pop CatchFinallyRanges that are no longer needed. + while (!catchFinallyRanges.empty() && catchFinallyRanges.back().end <= offset) + catchFinallyRanges.popBack(); + + CatchFinallyRange range(endOfTry - script_->code, afterTry - script_->code); + if (!catchFinallyRanges.append(range)) + return false; + break; + } + + case JSOP_LOOPENTRY: + for (size_t i = 0; i < catchFinallyRanges.length(); i++) { + if (catchFinallyRanges[i].contains(offset)) + infos_[offset].loopEntryInCatchOrFinally = true; + } + break; + + default: + break; } bool jump = IsJumpOpcode(op); diff --git a/js/src/ion/BytecodeAnalysis.h b/js/src/ion/BytecodeAnalysis.h index 14df7429118e..10d509899138 100644 --- a/js/src/ion/BytecodeAnalysis.h +++ b/js/src/ion/BytecodeAnalysis.h @@ -25,6 +25,9 @@ struct BytecodeInfo bool jumpFallthrough : 1; bool fallthrough : 1; + // If true, this is a JSOP_LOOPENTRY op inside a catch or finally block. + bool loopEntryInCatchOrFinally : 1; + void init(unsigned depth) { JS_ASSERT(depth <= MAX_STACK_DEPTH); JS_ASSERT_IF(initialized, stackDepth == depth); @@ -41,7 +44,7 @@ class BytecodeAnalysis public: explicit BytecodeAnalysis(JSScript *script); - bool init(); + bool init(JSContext *cx); BytecodeInfo &info(jsbytecode *pc) { JS_ASSERT(infos_[pc - script_->code].initialized); diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index 5c39a5590bd9..48640866684a 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -568,6 +568,7 @@ IonScript::IonScript() invalidateEpilogueOffset_(0), invalidateEpilogueDataOffset_(0), numBailouts_(0), + numExceptionBailouts_(0), hasUncompiledCallTarget_(false), hasSPSInstrumentation_(false), runtimeData_(0), @@ -995,8 +996,14 @@ OptimizeMIR(MIRGenerator *mir) if (mir->shouldCancel("Dominator Tree")) return false; - // This must occur before any code elimination. - if (!EliminatePhis(mir, graph, AggressiveObservability)) + // Aggressive phi elimination must occur before any code elimination. If the + // script contains a try-statement, we only compiled the try block and not + // the catch or finally blocks, so in this case it's also invalid to use + // aggressive phi elimination. + Observability observability = graph.hasTryBlock() + ? ConservativeObservability + : AggressiveObservability; + if (!EliminatePhis(mir, graph, observability)) return false; IonSpewPass("Eliminate phis"); AssertGraphCoherency(graph); @@ -1380,6 +1387,10 @@ IonCompile(JSContext *cx, JSScript *script, if (!script->ensureRanAnalysis(cx)) return AbortReason_Alloc; + // Try-finally is not yet supported. + if (script->analysis()->hasTryFinally()) + return AbortReason_Disable; + LifoAlloc *alloc = cx->new_(BUILDER_LIFO_ALLOC_PRIMARY_CHUNK_SIZE); if (!alloc) return AbortReason_Alloc; diff --git a/js/src/ion/Ion.h b/js/src/ion/Ion.h index 818972be3b15..2afa0ee08860 100644 --- a/js/src/ion/Ion.h +++ b/js/src/ion/Ion.h @@ -124,6 +124,15 @@ struct IonOptions // Default: 10 uint32_t frequentBailoutThreshold; + // Number of exception bailouts (resuming into catch/finally block) before + // we invalidate and forbid Ion compilation. + // + // Default: 10 + uint32_t exceptionBailoutThreshold; + + // Whether Ion should compile try-catch statements. + bool compileTryCatch; + // How many actual arguments are accepted on the C stack. // // Default: 4,096 @@ -205,6 +214,8 @@ struct IonOptions usesBeforeInliningFactor(.125), osrPcMismatchesBeforeRecompile(6000), frequentBailoutThreshold(10), + exceptionBailoutThreshold(10), + compileTryCatch(false), maxStackArgs(4096), maxInlineDepth(3), smallFunctionMaxInlineDepth(10), diff --git a/js/src/ion/IonAnalysis.cpp b/js/src/ion/IonAnalysis.cpp index 6a7cb36de81a..07a9a902943d 100644 --- a/js/src/ion/IonAnalysis.cpp +++ b/js/src/ion/IonAnalysis.cpp @@ -56,6 +56,12 @@ ion::SplitCriticalEdges(MIRGraph &graph) bool ion::EliminateDeadResumePointOperands(MIRGenerator *mir, MIRGraph &graph) { + // If we are compiling try blocks, locals and arguments may be observable + // from catch or finally blocks (which Ion does not compile). For now just + // disable the pass in this case. + if (graph.hasTryBlock()) + return true; + for (PostorderIterator block = graph.poBegin(); block != graph.poEnd(); block++) { if (mir->shouldCancel("Eliminate Dead Resume Point Operands (main loop)")) return false; diff --git a/js/src/ion/IonBuilder.cpp b/js/src/ion/IonBuilder.cpp index 8d127bbebb5f..cff0faac918c 100644 --- a/js/src/ion/IonBuilder.cpp +++ b/js/src/ion/IonBuilder.cpp @@ -1162,6 +1162,9 @@ IonBuilder::inspectOpcode(JSOp op) case JSOP_IFEQ: return jsop_ifeq(JSOP_IFEQ); + case JSOP_TRY: + return jsop_try(); + case JSOP_CONDSWITCH: return jsop_condswitch(); @@ -1633,6 +1636,9 @@ IonBuilder::processCfgEntry(CFGState &state) case CFGState::LABEL: return processLabelEnd(state); + case CFGState::TRY: + return processTryEnd(state); + default: MOZ_ASSUME_UNREACHABLE("unknown cfgstate"); } @@ -2175,6 +2181,30 @@ IonBuilder::processLabelEnd(CFGState &state) return ControlStatus_Joined; } +IonBuilder::ControlStatus +IonBuilder::processTryEnd(CFGState &state) +{ + JS_ASSERT(state.state == CFGState::TRY); + + if (!state.try_.successor) { + JS_ASSERT(!current); + return ControlStatus_Ended; + } + + if (current) { + current->end(MGoto::New(state.try_.successor)); + + if (!state.try_.successor->addPredecessor(current)) + return ControlStatus_Error; + } + + // Start parsing the code after this try-catch statement. + setCurrentAndSpecializePhis(state.try_.successor); + graph().moveBlockToEnd(current); + pc = current->pc(); + return ControlStatus_Joined; +} + IonBuilder::ControlStatus IonBuilder::processBreak(JSOp op, jssrcnote *sn) { @@ -2861,6 +2891,16 @@ IonBuilder::CFGState::Label(jsbytecode *exitpc) return state; } +IonBuilder::CFGState +IonBuilder::CFGState::Try(jsbytecode *exitpc, MBasicBlock *successor) +{ + CFGState state; + state.state = TRY; + state.stopAt = exitpc; + state.try_.successor = successor; + return state; +} + IonBuilder::ControlStatus IonBuilder::processCondSwitchCase(CFGState &state) { @@ -3180,6 +3220,82 @@ IonBuilder::jsop_ifeq(JSOp op) return true; } +bool +IonBuilder::jsop_try() +{ + JS_ASSERT(JSOp(*pc) == JSOP_TRY); + + if (!js_IonOptions.compileTryCatch) + return abort("Try-catch support disabled"); + + // Try-finally is not yet supported. + JS_ASSERT(script()->analysis()->hasTryFinally()); + + graph().setHasTryBlock(); + + jssrcnote *sn = info().getNote(cx, pc); + JS_ASSERT(SN_TYPE(sn) == SRC_TRY); + + // Get the pc of the last instruction in the try block. It's a JSOP_GOTO to + // jump over the catch block. + jsbytecode *endpc = pc + js_GetSrcNoteOffset(sn, 0); + JS_ASSERT(JSOp(*endpc) == JSOP_GOTO); + JS_ASSERT(GetJumpOffset(endpc) > 0); + + jsbytecode *afterTry = endpc + GetJumpOffset(endpc); + + // If controlflow in the try body is terminated (by a return or throw + // statement), the code after the try-statement may still be reachable + // via the catch block (which we don't compile) and OSR can enter it. + // For example: + // + // try { + // throw 3; + // } catch(e) { } + // + // for (var i=0; i<1000; i++) {} + // + // To handle this, we create two blocks: one for the try block and one + // for the code following the try-catch statement. Both blocks are + // connected to the graph with an MTest instruction that always jumps to + // the try block. This ensures the successor block always has a predecessor + // and later passes will optimize this MTest to a no-op. + // + // If the code after the try block is unreachable (control flow in both the + // try and catch blocks is terminated), only create the try block, to avoid + // parsing unreachable code. + + MBasicBlock *tryBlock = newBlock(current, GetNextPc(pc)); + if (!tryBlock) + return false; + + MBasicBlock *successor; + if (script()->analysis()->maybeCode(afterTry)) { + successor = newBlock(current, afterTry); + if (!successor) + return false; + + // Add MTest(true, tryBlock, successorBlock). + MConstant *true_ = MConstant::New(BooleanValue(true)); + current->add(true_); + current->end(MTest::New(true_, tryBlock, successor)); + } else { + successor = NULL; + current->end(MGoto::New(tryBlock)); + } + + if (!cfgStack_.append(CFGState::Try(endpc, successor))) + return false; + + // The baseline compiler should not attempt to enter the catch block + // via OSR. + JS_ASSERT(info().osrPc() < endpc || info().osrPc() >= afterTry); + + // Start parsing the try block. + setCurrentAndSpecializePhis(tryBlock); + return true; +} + IonBuilder::ControlStatus IonBuilder::processReturn(JSOp op) { @@ -3220,6 +3336,35 @@ IonBuilder::processThrow() { MDefinition *def = current->pop(); + if (graph().hasTryBlock()) { + // MThrow is not marked as effectful. This means when it throws and we + // are inside a try block, we could use an earlier resume point and this + // resume point may not be up-to-date, for example: + // + // (function() { + // try { + // var x = 1; + // foo(); // resume point + // x = 2; + // throw foo; + // } catch(e) { + // print(x); + // } + // ])(); + // + // If we use the resume point after the call, this will print 1 instead + // of 2. To fix this, we create a resume point right before the MThrow. + // + // Note that this is not a problem for instructions other than MThrow + // because they are either marked as effectful (have their own resume + // point) or cannot throw a catchable exception. + MNop *ins = MNop::New(); + current->add(ins); + + if (!resumeAfter(ins)) + return ControlStatus_Error; + } + MThrow *ins = MThrow::New(def); current->end(ins); diff --git a/js/src/ion/IonBuilder.h b/js/src/ion/IonBuilder.h index c8f7ad093386..97dc897875d9 100644 --- a/js/src/ion/IonBuilder.h +++ b/js/src/ion/IonBuilder.h @@ -87,7 +87,8 @@ class IonBuilder : public MIRGenerator COND_SWITCH_CASE, // switch() { case X: ... } COND_SWITCH_BODY, // switch() { case ...: X } AND_OR, // && x, || x - LABEL // label: x + LABEL, // label: x + TRY // try { x } catch(e) { } }; State state; // Current state of this control structure. @@ -169,6 +170,9 @@ class IonBuilder : public MIRGenerator struct { DeferredEdge *breaks; } label; + struct { + MBasicBlock *successor; + } try_; }; inline bool isLoop() const { @@ -192,6 +196,7 @@ class IonBuilder : public MIRGenerator static CFGState TableSwitch(jsbytecode *exitpc, MTableSwitch *ins); static CFGState CondSwitch(jsbytecode *exitpc, jsbytecode *defaultTarget); static CFGState Label(jsbytecode *exitpc); + static CFGState Try(jsbytecode *exitpc, MBasicBlock *successor); }; static int CmpSuccessors(const void *a, const void *b); @@ -249,6 +254,7 @@ class IonBuilder : public MIRGenerator ControlStatus processSwitchEnd(DeferredEdge *breaks, jsbytecode *exitpc); ControlStatus processAndOrEnd(CFGState &state); ControlStatus processLabelEnd(CFGState &state); + ControlStatus processTryEnd(CFGState &state); ControlStatus processReturn(JSOp op); ControlStatus processThrow(); ControlStatus processContinue(JSOp op); @@ -381,6 +387,7 @@ class IonBuilder : public MIRGenerator bool jsop_call(uint32_t argc, bool constructing); bool jsop_eval(uint32_t argc); bool jsop_ifeq(JSOp op); + bool jsop_try(); bool jsop_label(); bool jsop_condswitch(); bool jsop_andor(JSOp op); diff --git a/js/src/ion/IonCode.h b/js/src/ion/IonCode.h index 0e8db528b41a..db1009f2e125 100644 --- a/js/src/ion/IonCode.h +++ b/js/src/ion/IonCode.h @@ -192,6 +192,10 @@ struct IonScript // Number of times this script bailed out without invalidation. uint32_t numBailouts_; + // Number of times this scripted bailed out to enter a catch or + // finally block. + uint32_t numExceptionBailouts_; + // Flag set when it is likely that one of our (transitive) call // targets is not compiled. Used in ForkJoin.cpp to decide when // we should add call targets to the worklist. @@ -410,6 +414,12 @@ struct IonScript bool bailoutExpected() const { return numBailouts_ > 0; } + void incNumExceptionBailouts() { + numExceptionBailouts_++; + } + uint32_t numExceptionBailouts() const { + return numExceptionBailouts_; + } void setHasUncompiledCallTarget() { hasUncompiledCallTarget_ = true; } diff --git a/js/src/ion/IonFrameIterator.h b/js/src/ion/IonFrameIterator.h index 29cb9f7a5b15..81ac348cba47 100644 --- a/js/src/ion/IonFrameIterator.h +++ b/js/src/ion/IonFrameIterator.h @@ -480,6 +480,15 @@ class InlineFrameIteratorMaybeGC void resetOn(const IonFrameIterator *iter); + const IonFrameIterator &frame() const { + return *frame_; + } + + // Inline frame number, 0 for the outermost (non-inlined) frame. + size_t frameNo() const { + return start_.frameCount() - framesRead_; + } + private: InlineFrameIteratorMaybeGC() MOZ_DELETE; InlineFrameIteratorMaybeGC(const InlineFrameIteratorMaybeGC &iter) MOZ_DELETE; diff --git a/js/src/ion/IonFrames.cpp b/js/src/ion/IonFrames.cpp index c9ef1737aeb0..e39389e9fd9f 100644 --- a/js/src/ion/IonFrames.cpp +++ b/js/src/ion/IonFrames.cpp @@ -350,7 +350,8 @@ CloseLiveIterator(JSContext *cx, const InlineFrameIterator &frame, uint32_t loca } static void -CloseLiveIterators(JSContext *cx, const InlineFrameIterator &frame) +HandleExceptionIon(JSContext *cx, const InlineFrameIterator &frame, ResumeFromException *rfe, + bool *overrecursed) { RootedScript script(cx, frame.script()); jsbytecode *pc = frame.pc(); @@ -368,20 +369,63 @@ CloseLiveIterators(JSContext *cx, const InlineFrameIterator &frame) if (pcOffset >= tn->start + tn->length) continue; - if (tn->kind != JSTRY_ITER) - continue; + switch (tn->kind) { + case JSTRY_ITER: { + JS_ASSERT(JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); + JS_ASSERT(tn->stackDepth > 0); - JS_ASSERT(JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER); - JS_ASSERT(tn->stackDepth > 0); + uint32_t localSlot = tn->stackDepth; + CloseLiveIterator(cx, frame, localSlot); + break; + } - uint32_t localSlot = tn->stackDepth; - CloseLiveIterator(cx, frame, localSlot); + case JSTRY_LOOP: + break; + + case JSTRY_CATCH: + if (cx->isExceptionPending()) { + // Bailout at the start of the catch block. + jsbytecode *catchPC = script->main() + tn->start + tn->length; + + ExceptionBailoutInfo excInfo; + excInfo.frameNo = frame.frameNo(); + excInfo.resumePC = catchPC; + excInfo.numExprSlots = tn->stackDepth; + + BaselineBailoutInfo *info = NULL; + uint32_t retval = ExceptionHandlerBailout(cx, frame, excInfo, &info); + + if (retval == BAILOUT_RETURN_OK) { + JS_ASSERT(info); + rfe->kind = ResumeFromException::RESUME_BAILOUT; + rfe->target = cx->runtime()->ionRuntime()->getBailoutTail()->raw(); + rfe->bailoutInfo = info; + return; + } + + // Bailout failed. If there was a fatal error, clear the + // exception to turn this into an uncatchable error. If the + // overrecursion check failed, continue popping all inline + // frames and have the caller report an overrecursion error. + JS_ASSERT(!info); + cx->clearPendingException(); + + if (retval == BAILOUT_RETURN_OVERRECURSED) + *overrecursed = true; + else + JS_ASSERT(retval == BAILOUT_RETURN_FATAL_ERROR); + } + break; + + default: + MOZ_ASSUME_UNREACHABLE("Unexpected try note"); + } } } static void -HandleException(JSContext *cx, const IonFrameIterator &frame, ResumeFromException *rfe, - bool *calledDebugEpilogue) +HandleExceptionBaseline(JSContext *cx, const IonFrameIterator &frame, ResumeFromException *rfe, + bool *calledDebugEpilogue) { JS_ASSERT(frame.isBaselineJS()); JS_ASSERT(!*calledDebugEpilogue); @@ -511,12 +555,15 @@ HandleException(ResumeFromException *rfe) IonFrameIterator iter(cx->mainThread().ionTop); while (!iter.isEntry()) { + bool overrecursed = false; if (iter.isOptimizedJS()) { // Search each inlined frame for live iterator objects, and close // them. InlineFrameIterator frames(cx, &iter); for (;;) { - CloseLiveIterators(cx, frames); + HandleExceptionIon(cx, frames, rfe, &overrecursed); + if (rfe->kind != ResumeFromException::RESUME_ENTRY_FRAME) + return; // When profiling, each frame popped needs a notification that // the function has exited, so invoke the probe that a function @@ -536,7 +583,7 @@ HandleException(ResumeFromException *rfe) // It's invalid to call DebugEpilogue twice for the same frame. bool calledDebugEpilogue = false; - HandleException(cx, iter, rfe, &calledDebugEpilogue); + HandleExceptionBaseline(cx, iter, rfe, &calledDebugEpilogue); if (rfe->kind != ResumeFromException::RESUME_ENTRY_FRAME) return; @@ -575,6 +622,11 @@ HandleException(ResumeFromException *rfe) EnsureExitFrame(current); cx->mainThread().ionTop = (uint8_t *)current; } + + if (overrecursed) { + // We hit an overrecursion error during bailout. Report it now. + js_ReportOverRecursed(cx); + } } rfe->stackPointer = iter.fp(); diff --git a/js/src/ion/IonFrames.h b/js/src/ion/IonFrames.h index d835d24e9f2b..4eef76ccc4fc 100644 --- a/js/src/ion/IonFrames.h +++ b/js/src/ion/IonFrames.h @@ -255,6 +255,8 @@ class FrameSizeClass } }; +struct BaselineBailoutInfo; + // Data needed to recover from an exception. struct ResumeFromException { @@ -262,6 +264,7 @@ struct ResumeFromException static const uint32_t RESUME_CATCH = 1; static const uint32_t RESUME_FINALLY = 2; static const uint32_t RESUME_FORCED_RETURN = 3; + static const uint32_t RESUME_BAILOUT = 4; uint8_t *framePointer; uint8_t *stackPointer; @@ -270,6 +273,8 @@ struct ResumeFromException // Value to push when resuming into a |finally| block. Value exception; + + BaselineBailoutInfo *bailoutInfo; }; void HandleException(ResumeFromException *rfe); diff --git a/js/src/ion/MIRGraph.h b/js/src/ion/MIRGraph.h index a5f58e56df81..02c6518992a6 100644 --- a/js/src/ion/MIRGraph.h +++ b/js/src/ion/MIRGraph.h @@ -545,6 +545,7 @@ class MIRGraph Vector scripts_; size_t numBlocks_; + bool hasTryBlock_; public: MIRGraph(TempAllocator *alloc) @@ -554,7 +555,8 @@ class MIRGraph idGen_(0), osrBlock_(NULL), osrStart_(NULL), - numBlocks_(0) + numBlocks_(0), + hasTryBlock_(false) { } template @@ -677,6 +679,13 @@ class MIRGraph return scripts_.begin(); } + bool hasTryBlock() const { + return hasTryBlock_; + } + void setHasTryBlock() { + hasTryBlock_ = true; + } + // The per-thread context. So as not to modify the calling convention for // parallel code, we obtain the current slice from thread-local storage. // This helper method will lazilly insert an MForkJoinSlice instruction in diff --git a/js/src/ion/arm/MacroAssembler-arm.cpp b/js/src/ion/arm/MacroAssembler-arm.cpp index f9e7657de6c5..d3a9d7822525 100644 --- a/js/src/ion/arm/MacroAssembler-arm.cpp +++ b/js/src/ion/arm/MacroAssembler-arm.cpp @@ -9,6 +9,7 @@ #include "mozilla/DebugOnly.h" #include "mozilla/MathAlgorithms.h" +#include "ion/Bailouts.h" #include "ion/BaselineFrame.h" #include "ion/MoveEmitter.h" @@ -3330,12 +3331,14 @@ MacroAssemblerARMCompat::handleFailureWithHandlerTail() Label catch_; Label finally; Label return_; + Label bailout; ma_ldr(Operand(sp, offsetof(ResumeFromException, kind)), r0); branch32(Assembler::Equal, r0, Imm32(ResumeFromException::RESUME_ENTRY_FRAME), &entryFrame); branch32(Assembler::Equal, r0, Imm32(ResumeFromException::RESUME_CATCH), &catch_); branch32(Assembler::Equal, r0, Imm32(ResumeFromException::RESUME_FINALLY), &finally); branch32(Assembler::Equal, r0, Imm32(ResumeFromException::RESUME_FORCED_RETURN), &return_); + branch32(Assembler::Equal, r0, Imm32(ResumeFromException::RESUME_BAILOUT), &bailout); breakpoint(); // Invalid kind. @@ -3380,6 +3383,14 @@ MacroAssemblerARMCompat::handleFailureWithHandlerTail() ma_mov(r11, sp); pop(r11); ret(); + + // If we are bailing out to baseline to handle an exception, jump to + // the bailout tail stub. + bind(&bailout); + ma_ldr(Operand(sp, offsetof(ResumeFromException, bailoutInfo)), r2); + ma_mov(Imm32(BAILOUT_RETURN_OK), r0); + ma_ldr(Operand(sp, offsetof(ResumeFromException, target)), r1); + jump(r1); } Assembler::Condition diff --git a/js/src/ion/x64/MacroAssembler-x64.cpp b/js/src/ion/x64/MacroAssembler-x64.cpp index 8286e274925d..2ac5a49c0cd4 100644 --- a/js/src/ion/x64/MacroAssembler-x64.cpp +++ b/js/src/ion/x64/MacroAssembler-x64.cpp @@ -8,6 +8,7 @@ #include "mozilla/Casting.h" +#include "ion/Bailouts.h" #include "ion/BaselineFrame.h" #include "ion/IonFrames.h" #include "ion/MoveEmitter.h" @@ -253,12 +254,14 @@ MacroAssemblerX64::handleFailureWithHandlerTail() Label catch_; Label finally; Label return_; + Label bailout; loadPtr(Address(rsp, offsetof(ResumeFromException, kind)), rax); branch32(Assembler::Equal, rax, Imm32(ResumeFromException::RESUME_ENTRY_FRAME), &entryFrame); branch32(Assembler::Equal, rax, Imm32(ResumeFromException::RESUME_CATCH), &catch_); branch32(Assembler::Equal, rax, Imm32(ResumeFromException::RESUME_FINALLY), &finally); branch32(Assembler::Equal, rax, Imm32(ResumeFromException::RESUME_FORCED_RETURN), &return_); + branch32(Assembler::Equal, rax, Imm32(ResumeFromException::RESUME_BAILOUT), &bailout); breakpoint(); // Invalid kind. @@ -300,6 +303,13 @@ MacroAssemblerX64::handleFailureWithHandlerTail() movq(rbp, rsp); pop(rbp); ret(); + + // If we are bailing out to baseline to handle an exception, jump to + // the bailout tail stub. + bind(&bailout); + movq(Operand(esp, offsetof(ResumeFromException, bailoutInfo)), r9); + movl(Imm32(BAILOUT_RETURN_OK), rax); + jmp(Operand(rsp, offsetof(ResumeFromException, target))); } Assembler::Condition diff --git a/js/src/ion/x86/MacroAssembler-x86.cpp b/js/src/ion/x86/MacroAssembler-x86.cpp index aba1d776b8be..5ee581caf5d4 100644 --- a/js/src/ion/x86/MacroAssembler-x86.cpp +++ b/js/src/ion/x86/MacroAssembler-x86.cpp @@ -8,6 +8,7 @@ #include "mozilla/Casting.h" +#include "ion/Bailouts.h" #include "ion/BaselineFrame.h" #include "ion/IonFrames.h" #include "ion/MoveEmitter.h" @@ -225,12 +226,14 @@ MacroAssemblerX86::handleFailureWithHandlerTail() Label catch_; Label finally; Label return_; + Label bailout; loadPtr(Address(esp, offsetof(ResumeFromException, kind)), eax); branch32(Assembler::Equal, eax, Imm32(ResumeFromException::RESUME_ENTRY_FRAME), &entryFrame); branch32(Assembler::Equal, eax, Imm32(ResumeFromException::RESUME_CATCH), &catch_); branch32(Assembler::Equal, eax, Imm32(ResumeFromException::RESUME_FINALLY), &finally); branch32(Assembler::Equal, eax, Imm32(ResumeFromException::RESUME_FORCED_RETURN), &return_); + branch32(Assembler::Equal, eax, Imm32(ResumeFromException::RESUME_BAILOUT), &bailout); breakpoint(); // Invalid kind. @@ -272,6 +275,13 @@ MacroAssemblerX86::handleFailureWithHandlerTail() movl(ebp, esp); pop(ebp); ret(); + + // If we are bailing out to baseline to handle an exception, jump to + // the bailout tail stub. + bind(&bailout); + movl(Operand(esp, offsetof(ResumeFromException, bailoutInfo)), ecx); + movl(Imm32(BAILOUT_RETURN_OK), eax); + jmp(Operand(esp, offsetof(ResumeFromException, target))); } void diff --git a/js/src/jit-test/tests/ion/try-catch-1.js b/js/src/jit-test/tests/ion/try-catch-1.js new file mode 100644 index 000000000000..33041451aa88 --- /dev/null +++ b/js/src/jit-test/tests/ion/try-catch-1.js @@ -0,0 +1,12 @@ +function F() { + try { + var T = {}; + throw 12; + } catch (e) { + // Don't throw. + T.x = 5; + } +} +F(); +F(); +F(); diff --git a/js/src/jit-test/tests/ion/try-catch-2.js b/js/src/jit-test/tests/ion/try-catch-2.js new file mode 100644 index 000000000000..80ed6c9e9fb4 --- /dev/null +++ b/js/src/jit-test/tests/ion/try-catch-2.js @@ -0,0 +1,14 @@ +// Control flow does not reach end of try block, code after try statement is +// reachable by catch block. +function f() { + try { + throw 3; + } catch(e) { + } + + var res = 0; + for (var i=0; i<40; i++) + res += 2; + return res; +} +assertEq(f(), 80); diff --git a/js/src/jit-test/tests/ion/try-catch-3.js b/js/src/jit-test/tests/ion/try-catch-3.js new file mode 100644 index 000000000000..53b6babe181d --- /dev/null +++ b/js/src/jit-test/tests/ion/try-catch-3.js @@ -0,0 +1,27 @@ +// Don't fail if code after try statement is unreachable. +function f() { + try { + throw 1; + } catch(e) { + throw 5; + } + + // Unreachable. + assertEq(0, 2); + var res = 0; + for (var i=0; i<10; i++) + res += 2; + return res; +} + +var c = 0; + +for (var i=0; i<5; i++) { + try { + f(); + assertEq(0, 1); + } catch(e) { + c += e; + } +} +assertEq(c, 25); diff --git a/js/src/jit-test/tests/ion/try-catch-4.js b/js/src/jit-test/tests/ion/try-catch-4.js new file mode 100644 index 000000000000..f87a5a44e513 --- /dev/null +++ b/js/src/jit-test/tests/ion/try-catch-4.js @@ -0,0 +1,15 @@ +// Entering catch blocks via OSR is not possible (because the catch block +// is not compiled by Ion). Don't crash. +function f() { + var res = 0; + try { + throw 1; + } catch(e) { + for (var i=0; i<10; i++) { + res += 3; + } + } + + assertEq(res, 30); +} +f(); diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 15d16069036f..9fe894fd400f 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -5141,6 +5141,9 @@ ProcessArgs(JSContext *cx, JSObject *obj_, OptionParser *op) if (op->getBoolOption("ion-eager")) ion::js_IonOptions.setEagerCompilation(); + if (op->getBoolOption("ion-compile-try-catch")) + ion::js_IonOptions.compileTryCatch = true; + #ifdef JS_THREADSAFE if (const char *str = op->getStringOption("ion-parallel-compile")) { if (strcmp(str, "on") == 0) { @@ -5386,6 +5389,7 @@ main(int argc, char **argv, char **envp) " backtracking: Priority based backtracking register allocation\n" " stupid: Simple block local register allocation") || !op.addBoolOption('\0', "ion-eager", "Always ion-compile methods (implies --baseline-eager)") + || !op.addBoolOption('\0', "ion-compile-try-catch", "Ion-compile try-catch statements") #ifdef JS_THREADSAFE || !op.addStringOption('\0', "ion-parallel-compile", "on/off", "Compile scripts off thread (default: off)") From 87fff5741bb14681c9b57ff4e8d66ae414143ce9 Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Thu, 8 Aug 2013 17:41:48 +0200 Subject: [PATCH 14/19] Bug 902383: Remove the unused 'nonNativeGetElement' hint from the TI analysis, r=jandem --- js/src/ion/IonBuilder.cpp | 4 +--- js/src/jsanalyze.h | 2 -- js/src/vm/Interpreter-inl.h | 28 ---------------------------- js/src/vm/Interpreter.cpp | 15 ++++----------- 4 files changed, 5 insertions(+), 44 deletions(-) diff --git a/js/src/ion/IonBuilder.cpp b/js/src/ion/IonBuilder.cpp index cff0faac918c..c2286a8e32c9 100644 --- a/js/src/ion/IonBuilder.cpp +++ b/js/src/ion/IonBuilder.cpp @@ -6517,9 +6517,7 @@ IonBuilder::jsop_getelem() bool cacheable = obj->mightBeType(MIRType_Object) && !obj->mightBeType(MIRType_String) && (index->mightBeType(MIRType_Int32) || index->mightBeType(MIRType_String)); - bool nonNativeGetElement = - script()->analysis()->getCode(pc).nonNativeGetElement || - inspector->hasSeenNonNativeGetElement(pc); + bool nonNativeGetElement = inspector->hasSeenNonNativeGetElement(pc); // Turn off cacheing if the element is int32 and we've seen non-native objects as the target // of this getelem. diff --git a/js/src/jsanalyze.h b/js/src/jsanalyze.h index 0d20d230d3f2..90b7216c756a 100644 --- a/js/src/jsanalyze.h +++ b/js/src/jsanalyze.h @@ -104,8 +104,6 @@ class Bytecode * hints about the script for use during compilation. */ bool arrayWriteHole: 1; /* SETELEM which has written to an array hole. */ - bool getStringElement:1; /* GETELEM which has accessed string properties. */ - bool nonNativeGetElement:1; /* GETELEM on a non-native, non-array object. */ bool accessGetter: 1; /* Property read on a shape with a getter hook. */ /* Stack depth before this opcode. */ diff --git a/js/src/vm/Interpreter-inl.h b/js/src/vm/Interpreter-inl.h index 7e42c8676c27..5172fecf179a 100644 --- a/js/src/vm/Interpreter-inl.h +++ b/js/src/vm/Interpreter-inl.h @@ -357,20 +357,8 @@ GetObjectElementOperation(JSContext *cx, JSOp op, JSObject *objArg, bool wasObje HandleValue rref, MutableHandleValue res) { do { - // Don't call GetPcScript (needed for analysis) from inside Ion since it's expensive. - bool analyze = cx->currentlyRunningInInterpreter(); - uint32_t index; if (IsDefinitelyIndex(rref, &index)) { - if (analyze && !objArg->isNative() && !objArg->is()) { - JSScript *script = NULL; - jsbytecode *pc = NULL; - types::TypeScript::GetPcScript(cx, &script, &pc); - - if (script->hasAnalysis()) - script->analysis()->getCode(pc).nonNativeGetElement = true; - } - if (JSObject::getElementNoGC(cx, objArg, objArg, index, res.address())) break; @@ -381,22 +369,6 @@ GetObjectElementOperation(JSContext *cx, JSOp op, JSObject *objArg, bool wasObje break; } - if (analyze) { - JSScript *script = NULL; - jsbytecode *pc = NULL; - types::TypeScript::GetPcScript(cx, &script, &pc); - - if (script->hasAnalysis()) { - script->analysis()->getCode(pc).getStringElement = true; - - if (!objArg->is() && !objArg->isNative() && - !objArg->is()) - { - script->analysis()->getCode(pc).nonNativeGetElement = true; - } - } - } - if (ValueMightBeSpecial(rref)) { RootedObject obj(cx, objArg); Rooted special(cx); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 457c9926980d..e29057484239 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1263,17 +1263,10 @@ SetObjectElementOperation(JSContext *cx, Handle obj, HandleId id, con uint32_t length = obj->getDenseInitializedLength(); int32_t i = JSID_TO_INT(id); if ((uint32_t)i >= length) { - // In an Ion activation, GetPcScript won't work. For non-baseline activations, - // that's ok, because optimized ion doesn't generate analysis info. However, - // baseline must generate this information, so it passes the script and pc in - // as arguments. - if (script || cx->currentlyRunningInInterpreter()) { - JS_ASSERT(!!script == !!pc); - if (!script) - types::TypeScript::GetPcScript(cx, script.address(), &pc); - - if (script->hasAnalysis()) - script->analysis()->getCode(pc).arrayWriteHole = true; + // Annotate script if provided with information (e.g. baseline) + if (script && script->hasAnalysis()) { + JS_ASSERT(pc); + script->analysis()->getCode(pc).arrayWriteHole = true; } } } From 5785631db5fc6e04f677d9e0056953817b412a26 Mon Sep 17 00:00:00 2001 From: Robert Bindar Date: Thu, 8 Aug 2013 08:49:14 -0700 Subject: [PATCH 15/19] bug 835357 - Add telemetry to measure whether DNT is used or not. r=jduell --- netwerk/protocol/http/nsHttpHandler.cpp | 8 ++++++++ toolkit/components/telemetry/Histograms.json | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 99f6043609b2..0f0c2b44e792 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -98,6 +98,7 @@ static NS_DEFINE_CID(kSocketProviderServiceCID, NS_SOCKETPROVIDERSERVICE_CID); #define BROWSER_PREF_PREFIX "browser.cache." #define DONOTTRACK_HEADER_ENABLED "privacy.donottrackheader.enabled" #define DONOTTRACK_HEADER_VALUE "privacy.donottrackheader.value" +#define DONOTTRACK_VALUE_UNSET 2 #ifdef MOZ_TELEMETRY_ON_BY_DEFAULT #define TELEMETRY_ENABLED "toolkit.telemetry.enabledPreRelease" #else @@ -235,6 +236,13 @@ nsHttpHandler::~nsHttpHandler() mPipelineTestTimer = nullptr; } + if (!mDoNotTrackEnabled) { + Telemetry::Accumulate(Telemetry::DNT_USAGE, DONOTTRACK_VALUE_UNSET); + } + else { + Telemetry::Accumulate(Telemetry::DNT_USAGE, mDoNotTrackValue); + } + gHttpHandler = nullptr; } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index ed6dc260c6c6..91df9db1a61b 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -1573,6 +1573,11 @@ "n_buckets": 50, "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSASYNCDOOMEVENT_RUN" }, + "DNT_USAGE": { + "kind": "enumerated", + "n_values": 3, + "description": "I want to be tracked, I do NOT want to be tracked, DNT unset" + }, "DNS_LOOKUP_METHOD2": { "kind": "enumerated", "n_values": 16, From 9ed085af3a9c3f9721a66ed5d22e2fabc2d57fbf Mon Sep 17 00:00:00 2001 From: Scott Johnson Date: Thu, 8 Aug 2013 12:22:26 -0500 Subject: [PATCH 16/19] Bug 808173: Add a switch for reflow-on-zoom to not activate when non-text elements are the target of a double-tap event. [r=kats] --- mobile/android/chrome/content/browser.js | 32 +++++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 17794d311915..725b1b55a433 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -4320,10 +4320,14 @@ var BrowserEventHandler = { onDoubleTap: function(aData) { let data = JSON.parse(aData); + let element = ElementTouchHelper.anyElementFromPoint(data.x, data.y); - // We only want to do this if reflow-on-zoom is enabled. + // We only want to do this if reflow-on-zoom is enabled, we don't already + // have a reflow-on-zoom event pending, and the element upon which the user + // double-tapped isn't of a type we want to avoid reflow-on-zoom. if (BrowserEventHandler.mReflozPref && - !BrowserApp.selectedTab._mReflozPoint) { + !BrowserApp.selectedTab._mReflozPoint && + !this._shouldSuppressReflowOnZoom(element)) { let data = JSON.parse(aData); let zoomPointX = data.x; let zoomPointY = data.y; @@ -4333,8 +4337,6 @@ var BrowserEventHandler = { BrowserApp.selectedTab.probablyNeedRefloz = true; } - let zoom = BrowserApp.selectedTab._zoom; - let element = ElementTouchHelper.anyElementFromPoint(data.x, data.y); if (!element) { this._zoomOut(); return; @@ -4350,6 +4352,28 @@ var BrowserEventHandler = { } }, + /** + * Determine if reflow-on-zoom functionality should be suppressed, given a + * particular element. Double-tapping on the following elements suppresses + * reflow-on-zoom: + * + *