Use the build id instead of the XDR_BYTECODE_VERSION constant

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jandem, Assigned: arai)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(5 attachments, 4 obsolete attachments)

Reporter

Description

4 years ago
The XDR version is stored as a constant that has to be incremented whenever the bytecode (or error messages) change. This is a bit tedious, easy to forget and it can be pretty annoying when you get merge conflicts (especially when uplifting patches..)

The asm.js cache uses the browser's build id (via a callback), I think it'd be pretty simple and nice to reuse this mechanism for XDR.

The only caveat is that you might run into problems when you do a local build and rebuild only xul (mach build js/src) and not the Firefox executable. Personally I don't think it's worth keeping the current mechanism just for that use case, also because, in my experience, most patches that bump this constant are only tested in the shell.
What would be the impact on nightly users?
Reporter

Comment 2

4 years ago
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> What would be the impact on nightly users?

Their XDR caches will be invalidated each time they update, but that's fine I think.
Ted: Do you know if using build-id, extracted like so:
  http://mxr.mozilla.org/mozilla-central/source/dom/asmjscache/AsmJSCache.cpp#1698
as the way to invalidate our XDR bytecode cache (which isn't CPU/arch dependent) is sufficient as a replacement for the current XDR number we bump manually:
  http://mxr.mozilla.org/mozilla-central/source/js/src/vm/Xdr.h#18
?  (Sorry for shotgun ni; feel free to forward if you can think of a better person.)
Flags: needinfo?(ted)
Assignee

Comment 4

4 years ago
Here's draft patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a7719e29155

changes:
  Part 1
    * Added JS::SetBuildIdOp public JSAPI
    * Moved buildId member from JS::AsmJSCacheOps to JSRuntime
    * Added ExclusiveContext::buildId method to access runtime's buildId

    * Moved GetBuildId to xpcom/base/CycleCollectedJSRuntime.cpp
      * Use GRE_BUILDID constant directly instead of nsIXULAppInfo
        * nsIXULAppInfo is not available on xpcshell, that is used while
          packaging
        * Added xpcom/base/Makefile.in to define GRE_BUILDID

    * Call JS::SetBuildIdOp in CycleCollectedJSRuntime::CycleCollectedJSRuntime
    * Call JS::SetBuildIdOp in main in js shell

  Part 2
    * Changed XDR VersionCheck to use cx->buildId()
      * encode/decode length(uint32) + buildId(bytes)
        current XDR_BYTECODE_VERSION(uint32) will be decoded as length, and
        it's rejected because of different length, that is checked before
        decoding buildId bytes

    * Added GetBuildId function to js/src/jsapi-tests/testXDR.cpp
    * Call JS::SetBuildIdOp in FreezeThaw in js/src/jsapi-tests/testXDR.cpp

    * Removed XDR_BYTECODE_VERSION, XDR_BYTECODE_VERSION_SUBTRAHEND,
      and JSErr_Limit check \o/
    * Removed XDR_BYTECODE_VERSION-related code from make_opcode_doc.py
    * Removed XDR_BYTECODE_VERSION-related comment from SourceNotes.h
    * Removed XDR_BYTECODE_VERSION-related comment from jsfriendapi.h


then, I have some questions:

1. As noted above, nsIXULAppInfo is not available in xpcshell, is it okay to embed GRE_BUILDID directly in CycleCollectedJSRuntime.cpp ?

https://dxr.mozilla.org/mozilla-central/rev/c5cb194cc9cb56d742fb3a7a826f0080b0404edc/toolkit/xre/nsAppRunner.cpp#803
>   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIXULAppInfo, gAppData ||
>                                      XRE_IsContentProcess())

there, gAppData is null, and XRE_IsContentProcess() is false on xpcshell


2. Now buildId is used in XDR encode/decode, and all runtime that may use XDR needs JS::SetBuildIdOp call.  Where/when is the best place/timing to call it?

Looks like CycleCollectedJSRuntime is used for most of runtimes in firefox/xpcshell, so I added it there for now.  Is there any better place?
Assignee

Comment 5

4 years ago
Can I get some feedback?
Attachment #8703245 - Flags: feedback?(jdemooij)
Reporter

Comment 7

4 years ago
Comment on attachment 8703245 [details] [diff] [review]
Part 1: Separate buildIdOp from AsmJSCacheOps.

Review of attachment 8703245 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks!

::: js/src/asmjs/AsmJSModule.cpp
@@ +1990,5 @@
>      JS::BuildIdCharVector buildId_;
>  
>    public:
>      bool extractCurrentState(ExclusiveContext* cx) {
> +        if (!cx->buildId())

Nit: I'd call this buildIdOp() to make it more clear it returns a callback and not the buildId itself. That also nicely matches AsmJSCacheOps etc.

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +401,5 @@
> +mozilla::GetBuildId(JS::BuildIdCharVector* aBuildID)
> +{
> +#ifndef GRE_BUILDID
> +# error "GRE_BUILDID should be defined."
> +#endif

Nice.

::: xpcom/base/Makefile.in
@@ +1,5 @@
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +include $(topsrcdir)/config/makefiles/makeutils.mk

You'll need review from a build peer (like glandium or ted) for this change :)
Attachment #8703245 - Flags: feedback?(jdemooij) → feedback+
Reporter

Comment 8

4 years ago
Comment on attachment 8703246 [details] [diff] [review]
Part 2: Use the build id instead of the XDR_BYTECODE_VERSION constant.

Review of attachment 8703246 [details] [diff] [review]:
-----------------------------------------------------------------

\o/

::: js/src/vm/Xdr.cpp
@@ +16,5 @@
>  #include "vm/Debugger.h"
>  #include "vm/ScopeObject.h"
>  
>  using namespace js;
> +using mozilla::PodEqual;

Nit: either remove this line, or s/mozilla::PodEqual/PodEqual/ below :)

@@ +99,5 @@
> +    if (!xdr->cx()->buildId() || !xdr->cx()->buildId()(&buildId)) {
> +        JS_ReportErrorNumber(xdr->cx(), GetErrorMessage, nullptr, JSMSG_BUILD_ID_NOT_AVAILABLE);
> +        return false;
> +    }
> +

Maybe add a MOZ_ASSERT(!buildId.empty()); here, as a simple sanity check.

@@ +112,1 @@
>          JS_ReportErrorNumber(xdr->cx(), GetErrorMessage, nullptr, JSMSG_BAD_SCRIPT_MAGIC);

We can rename JSMSG_BAD_SCRIPT_MAGIC to JSMSG_BAD_BUILD_ID or something.

@@ +120,5 @@
> +        JS::BuildIdCharVector decodedBuildId;
> +
> +        /* buildIdLength is already checked against the length of current
> +         * buildId. */
> +        if (!decodedBuildId.resize(buildIdLength)) {

Nit: use // comments
Attachment #8703246 - Flags: feedback?(jdemooij) → feedback+

Comment 9

4 years ago
Dup of 601814 ?
Assignee

Comment 10

4 years ago
Thank you for your feedback :)
Addressed review comments.

ted, can you review the change in xpcom/base/Makefile.in?  in addition to questions in comment #3 and #4.
Assignee: jdemooij → arai.unmht
Attachment #8703245 - Attachment is obsolete: true
Attachment #8703910 - Flags: review?(ted)
The build id is always increasing, and it will get regenerated for any top-level build, so for those purposes it should be fine. We already use it for invaliding some XUL caches etc.
Flags: needinfo?(ted)
Comment on attachment 8703910 [details] [diff] [review]
Part 1: Separate buildIdOp from AsmJSCacheOps.

Review of attachment 8703910 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/CycleCollectedJSRuntime.cpp
@@ +408,5 @@
> +mozilla::GetBuildId(JS::BuildIdCharVector* aBuildID)
> +{
> +#ifndef GRE_BUILDID
> +# error "GRE_BUILDID should be defined."
> +#endif

Is there any reason you can't just grab this off of nsIXULAppInfo?
https://dxr.mozilla.org/mozilla-central/rev/aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/xpcom/system/nsIXULAppInfo.idl#56

I'm not a fan of adding more Makefile.ins at this point.
Attachment #8703910 - Flags: review?(ted)
Assignee

Comment 13

3 years ago
Thank you for reviewing :D

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12)
> Is there any reason you can't just grab this off of nsIXULAppInfo?
> https://dxr.mozilla.org/mozilla-central/rev/
> aa90f482e16db77cdb7dea84564ea1cbd8f7f6b3/xpcom/system/nsIXULAppInfo.idl#56

nsIXULAppInfo is not available in xpcshell.  XDR with xpcshell is used while packaging jsm, so I needed other way to get build id.
Maybe I should make it possible to use nsIXULAppInfo in xpcshell?

https://dxr.mozilla.org/mozilla-central/rev/c5cb194cc9cb56d742fb3a7a826f0080b0404edc/toolkit/xre/nsAppRunner.cpp#803
>   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIXULAppInfo, gAppData ||
>                                      XRE_IsContentProcess())

gAppData is null there.
Flags: needinfo?(ted)
Oh, right, sorry. I'm still not wild about this because this will wind up re-linking libxul every time someone does a top-level rebuild (the build id gets regenerated, which will cause CycleCollectedJSRuntime.cpp to get rebuilt, which will cause libxul to relink).

Making nsIXULAppInfo always available in xpcshell would break some things. The build ID gets into nsIXULAppInfo by way of being compiled into nsBrowserApp.cpp:
https://dxr.mozilla.org/mozilla-central/source/browser/app/nsBrowserApp.cpp#8

We could do a similar thing for the xpcshell binary, but then you'd have to do some plumbing to make it available down in the cycle collector code.
Flags: needinfo?(ted)
Assignee

Comment 15

3 years ago
thanks :)

looks like gToolkitVersion is always available.
so I'll prepare a dedicated way to pass it to CycleCollectedJSRuntime.cpp, maybe another interface on nsXULAppInfo, in case nsIXULAppInfo is not implemented.
Assignee

Comment 16

3 years ago
Added nsIPlatformInfo interface with platformVersion and platformBuildID properties, and made nsIXULAppInfo inherits from nsIPlatformInfo.

nsXULAppInfo always implements nsIPlatformInfo interface, regardless of gAppData, as platformVersion and platformBuildID are always available via gToolkitVersion and gToolkitBuildID global variables.
Attachment #8715633 - Flags: review?(ted)
Assignee

Comment 17

3 years ago
changed mozilla::GetBuildId to get platformBuildID by nsIPlatformInfo interface of app-info.
Attachment #8703910 - Attachment is obsolete: true
Attachment #8715634 - Flags: review?(ted)
Assignee

Updated

3 years ago
Attachment #8715633 - Flags: review?(ted) → review?(jst)
Assignee

Comment 18

3 years ago
Attachment #8715634 - Attachment is obsolete: true
Attachment #8715634 - Flags: review?(ted)
Attachment #8728553 - Flags: review?(jdemooij)
Reporter

Comment 20

3 years ago
Comment on attachment 8728553 [details] [diff] [review]
Part 1: Separate buildIdOp from AsmJSCacheOps.

Review of attachment 8728553 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I'm not a Gecko peer though so, bz, would you mind glancing at the non-JS parts? It's just moving GetBuildId to CycleCollectedJSRuntime.cpp and setting the build id hook there instead of in nsJSEnvironment.cpp.

::: xpcom/base/CycleCollectedJSRuntime.h
@@ +417,5 @@
>    return aKind == JS::TraceKind::Object || aKind == JS::TraceKind::Script;
>  }
>  
> +bool
> +GetBuildId(JS::BuildIdCharVector* aBuildID);

I think GetBuildId is only used in CycleCollectedJSRuntime.cpp. If that's true, we can remove this declaration and change CycleCollectedJSRuntime.cpp to have:

static bool
GetBuildId(...)
{
Attachment #8728553 - Flags: review?(jdemooij)
Attachment #8728553 - Flags: review?(bzbarsky)
Attachment #8728553 - Flags: review+
Reporter

Comment 21

3 years ago
Comment on attachment 8728554 [details] [diff] [review]
Part 2: Use the build id instead of the XDR_BYTECODE_VERSION constant.

Review of attachment 8728554 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/Xdr.h
@@ -30,5 @@
> - *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
> - *
> - * (If you're wondering, 0xb973c0de is used because it looks like "bytecode".)
> - */
> -static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 350;

The end of an era :'(
Attachment #8728554 - Flags: review?(jdemooij) → review+
Attachment #8715633 - Flags: review?(jst) → review+
Assignee

Comment 22

3 years ago
Thank you for reviewing :)

(In reply to Jan de Mooij [:jandem] from comment #20)
> ::: xpcom/base/CycleCollectedJSRuntime.h
> @@ +417,5 @@
> >    return aKind == JS::TraceKind::Object || aKind == JS::TraceKind::Script;
> >  }
> >  
> > +bool
> > +GetBuildId(JS::BuildIdCharVector* aBuildID);
> 
> I think GetBuildId is only used in CycleCollectedJSRuntime.cpp. If that's
> true, we can remove this declaration and change CycleCollectedJSRuntime.cpp
> to have:
> 
> static bool
> GetBuildId(...)
> {

It's used also in AsmJSCache.cpp

https://dxr.mozilla.org/mozilla-central/rev/af7c0cb0798f5425d5d344cbaf0ac0ecb1a72a86/dom/asmjscache/AsmJSCache.cpp#84
https://dxr.mozilla.org/mozilla-central/rev/af7c0cb0798f5425d5d344cbaf0ac0ecb1a72a86/dom/asmjscache/AsmJSCache.cpp#116
Comment on attachment 8728553 [details] [diff] [review]
Part 1: Separate buildIdOp from AsmJSCacheOps.

Yeah, just make this thing a file static, if it's only used in this file.

r=me on the xpcom and dom changes with that fixed.
Attachment #8728553 - Flags: review?(bzbarsky) → review+
Assignee

Comment 24

3 years ago
The build ID string is now shared between XDR bytecode and asm.js cache, and GetBuildId is used in 2 files CycleCollectedJSRuntime.cpp (to call JS::SetBuildIdOp with it) and AsmJSCache.cpp (directly call it from WriteMetadataFile and ReadMetadataFile).  so it cannot be made static.
am I missing something?
Flags: needinfo?(bzbarsky)
Reporter

Comment 25

3 years ago
If it's also used in AsmJSCache.cpp, I think it's fine to land it as is :)
Assignee

Comment 26

3 years ago
I see, thanks!
Flags: needinfo?(bzbarsky)
Assignee

Comment 27

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/905e3a9f755778aacfbf3308ff00e9f2ee22ffb5
Bug 1153978 - Part 0: Separate nsIPlatformInfo from nsIXULAppInfo. r=jst

https://hg.mozilla.org/integration/mozilla-inbound/rev/08ee8e76c7039e07c91f34477121787c845c2cca
Bug 1153978 - Part 1: Separate buildIdOp from AsmJSCacheOps. r=jandem,bz

https://hg.mozilla.org/integration/mozilla-inbound/rev/8db1ac84d30e663f9487a9a54844dd044ebd934b
Bug 1153978 - Part 2: Use the build id instead of the XDR_BYTECODE_VERSION constant. r=jandem
Assignee

Comment 29

3 years ago
Still investigating the reason of the xpcshell crash tho,
the mock app-info [1] implemented in JS seems to be the reason, that is registered as "@mozilla.org/xre/app-info;1" but doesn't implement Ci.nsIPlatformInfo.
So, I need to add Ci.nsIPlatformInfo to its QueryInterface.

The mock app-info code is copied around the tree, with several variants, and even in comm-central too.
We should merge all of them into single implementation. will file a dedicated bug for merging.

[1] https://dxr.mozilla.org/mozilla-central/rev/d1d47ba19ce9d46222030d491f9fe28dbf80be12/js/xpconnect/tests/unit/test_xpcomutils.js#126
Assignee

Updated

3 years ago
See Also: → 1256088
Assignee

Comment 30

3 years ago
Then, another issue is that somewhere in the code assumes that XDR VersionCheck doesn't fail for certain input,
but now it can fail, as GetBuildId is fallible.

that seems to be the reason of the crash, with assertion failure for pending exception.
Assignee

Comment 31

3 years ago
ReadCachedScript calls JS_DecodeScript, and if it fails, ReadCachedScript returns NS_ERROR_OUT_OF_MEMORY.

https://dxr.mozilla.org/mozilla-central/rev/d1d47ba19ce9d46222030d491f9fe28dbf80be12/js/xpconnect/loader/mozJSLoaderUtils.cpp#33
>     scriptp.set(JS_DecodeScript(cx, buf.get(), len));
>     if (!scriptp)
>         return NS_ERROR_OUT_OF_MEMORY;
>     return NS_OK;


mozJSComponentLoader::ObjectForLocation calls ReadCachedScript, and just *ignores* the failure.  So, in that case, the JS exception thrown by JS_DecodeScript is still pending, and the pending exception is caught by the assertion in BytecodeCompiler::compileScript.

https://dxr.mozilla.org/mozilla-central/rev/d1d47ba19ce9d46222030d491f9fe28dbf80be12/js/xpconnect/loader/mozJSComponentLoader.cpp#691
>         if (!mReuseLoaderGlobal) {
>             rv = ReadCachedScript(cache, cachePath, cx, mSystemPrincipal, &script);
>         } else {
>             rv = ReadCachedFunction(cache, cachePath, cx, mSystemPrincipal,
>                                     function.address());
>         }
> 
>         if (NS_SUCCEEDED(rv)) {
>             LOG(("Successfully loaded %s from startupcache\n", nativePath.get()));
>         } else {
>             // This is ok, it just means the script is not yet in the
>             // cache. Could mean that the cache was corrupted and got removed,
>             // but either way we're going to write this out.
>             writeToCache = true;
>         }

So, if it ignores the error, it should call JS_ClearPendingException.
> implemented in JS seems to be the reason, that is registered as "@mozilla.org/xre/app-info;1"

Uh....  That contract is used from non-main threads in general (e.g. ParentRunnable::OpenCacheFileForWrite will run on a non-main thread).  How is implementing it in JS in any way OK, unless we make sure to NEVER do anything with asmjs (or anything else that might use that contract from off the main thread) while using that mock?  :(

> mozJSComponentLoader::ObjectForLocation calls ReadCachedScript, and just *ignores* the failure.

Yeah, we should probably jsapi.ReportException() in ObjectForLocation in the "cache and read failed" case.
Assignee

Comment 33

3 years ago
What's the convention for handling a JS exception that is thrown by JSAPI, in XPCOM/XPConnect?
If a function returns NS_* failure, can it imply a pending JS exception? (is there any caller that handles the exception?)  or, the function should (report and) clear the exception before returning NS_* failure?


if latter, I guess ReadCachedScript should handle the exception (by jsapi.ReportException() or JS_ClearPendingException), as currently WriteCachedScript calls JS_ClearPendingException.

https://dxr.mozilla.org/mozilla-central/rev/d1d47ba19ce9d46222030d491f9fe28dbf80be12/js/xpconnect/loader/mozJSLoaderUtils.cpp#70
> nsresult
> WriteCachedScript(StartupCache* cache, nsACString& uri, JSContext* cx,
>                   nsIPrincipal* systemPrincipal, HandleScript script)
> {
>     MOZ_ASSERT(JS_GetScriptPrincipals(script) == nsJSPrincipals::get(systemPrincipal));
> 
>     uint32_t size;
>     void* data = JS_EncodeScript(cx, script, &size);
>     if (!data) {
>         // JS_EncodeScript may have set a pending exception.
>         JS_ClearPendingException(cx);
>         return NS_ERROR_FAILURE;
>     }
Flags: needinfo?(bzbarsky)
Assignee

Comment 34

3 years ago
one more problematic place related to ReadCachedScript here.

mozJSSubScriptLoader::DoLoadSubScriptWithOptions calls ReadCachedScript, and it also ignores the failure.

https://dxr.mozilla.org/mozilla-central/rev/d1d47ba19ce9d46222030d491f9fe28dbf80be12/js/xpconnect/loader/mozJSSubScriptLoader.cpp#660
> nsresult
> mozJSSubScriptLoader::DoLoadSubScriptWithOptions(const nsAString& url,
> ...
>     if (cache && !options.ignoreCache)
>         rv = ReadCachedScript(cache, cachePath, cx, mSystemPrincipal, &script);
> 
>     // If we are doing an async load, trigger it and bail out.
>     if (!script && options.async) {
>         return ReadScriptAsync(uri, targetObj, options.charset, serv,
>                                reusingGlobal, !!cache, retval);
>     }
> 
>     if (!script) {
>         rv = ReadScript(uri, cx, targetObj, options.charset,
>                         static_cast<const char*>(uriStr.get()), serv,
>                         principal, reusingGlobal, &script, &function);
>     } else {
>         cache = nullptr;
>     }
> 
>     if (NS_FAILED(rv) || (!script && !function))
>         return rv;
> 
>     return EvalScript(cx, targetObj, retval, uri, !!cache, script, function);
> }
> If a function returns NS_* failure, can it imply a pending JS exception? 

Yes, if it takes a JSContext argument.

> if latter, I guess ReadCachedScript should handle the exception (by jsapi.ReportException() or JS_ClearPendingException), as
> currently WriteCachedScript calls JS_ClearPendingException.

In practice this only matters in terms of what gets shown in the browser console.  If we think the error here will be actionable, we should report the exception.  If not, we should just suppress it via jsapi.ClearException() on the caller, or JS_ClearPendingException in the callee if we want to preserve the old callee behavior of never leaving exceptions on the JSContext.
Flags: needinfo?(bzbarsky)
Assignee

Comment 36

3 years ago
thanks :)
so, when a function ignores the failure of the function that takes JSContext, it should handle the exception, right?

then, in this bug's case, XDR related JSAPI function can fail with several reasons (version mismatch, OOM, cannot write to buffer, etc), and previously all of them could be just ignored,  but now VersionCheck can call JS function (in createInstance for app-info, and maybe platformBuildID getter?).

in term of error reporting to browser console, version mismatch and some other pre-existing errors are not supposed to be reported there.
now, it might be better reporting the error related to mock app-info, but it might make things complicated (especially, when to report, or how to distinguish them).

I guess, the mock app-info is the special case, used only in xpcshell-test, and we could add some restriction to it, so, for now, I'm going to just clear the exception without reporting, to see if it works or not.
> so, when a function ignores the failure of the function that takes JSContext,
> it should handle the exception, right?

Probably yes.  There is no hard and fast rule here...  In practice, we should document what these functions do.

> I guess, the mock app-info is the special case, used only in xpcshell-test, and we could add some
> restriction to it, so, for now, I'm going to just clear the exception without reporting, to see
> if it works or not.

That makes sense to me.

I still worry about this mock app-info causing problems if anyone does workers with wasm in xpcshell...
Assignee

Comment 38

3 years ago
now it passes the try, with bug 1256088's patch, and extra 2 patches for this bug.
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe8c505b6e9c
Assignee

Comment 39

3 years ago
Part 0 adds nsIPlatformInfo, that should be implemented in mock app-info,
and now all mock app-info is merged to AppInfo.jsm in bug 
1256088, so changed mock app-info in AppInfo.jsm to implement nsIPlatformInfo.

also, plarformBuildID property is used in XDR encode/decode, so it should be the same value as original one.
platformVersion can be modified to fit the testcase.

Will fold this to Part 0.
Attachment #8731472 - Flags: review?(gps)
Assignee

Comment 40

3 years ago
Added JS_ClearPendingException when ignoring ReadCachedScript and ReadCachedFunction failure
Attachment #8731474 - Flags: review?(bzbarsky)
Comment on attachment 8731474 [details] [diff] [review]
Part 2.1: Clear pending exception thrown from XDR related APIs when ignoring ReadCachedScript failure.

r=me
Attachment #8731474 - Flags: review?(bzbarsky) → review+
Attachment #8731472 - Flags: review?(gps) → review+
Assignee

Comment 42

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/89d44b4257f1896af77af5940f6cb4787f7b8252
Bug 1153978 - Part 0: Separate nsIPlatformInfo from nsIXULAppInfo. r=jst,gps

https://hg.mozilla.org/integration/mozilla-inbound/rev/f22eed4b7a39beeff05a30670e8339c3e29c8146
Bug 1153978 - Part 1: Separate buildIdOp from AsmJSCacheOps. r=jandem,bz

https://hg.mozilla.org/integration/mozilla-inbound/rev/e20421e07ab1b041efaa6ca7ae51df8115e1fcd2
Bug 1153978 - Part 2: Clear pending exception thrown from XDR related APIs when ignoring ReadCachedScript failure. r=bz

https://hg.mozilla.org/integration/mozilla-inbound/rev/288b2aef2a412317e7092e333f8fa3fbdca49e37
Bug 1153978 - Part 3: Use the build id instead of the XDR_BYTECODE_VERSION constant. r=jandem
You need to log in before you can comment on or make changes to this bug.