943512 - Stubify Promise.jsm so it can be used in the devtools debugger server

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, defect, P2)

Description

[Brandon Benvie [:benvie]]

As described in bug 881050 comment 20, in order for the devtools debugger server to use Promise.jsm it has to be loaded in the same compartment as the debugger server. This seems to only be doable by using loadSubScript. In order to satisfy the needs of the debugger server, Promise.jsm needs to be loadable via loadSubScript.

Comment 1

[Brandon Benvie [:benvie]]

Attachment: promise-stubify.patch

This patch does the aforementioned stubification of Promise.jsm by introducing promise.js which can be loaded via loadSubScript.

Comment 2

[Brandon Benvie [:benvie]]

https://tbpl.mozilla.org/?tree=Try&rev=e8bdff2ede8e

Comment 3

[Brandon Benvie [:benvie]]

Hmmm, why would this cause a crash?

Comment 4

[Brandon Benvie [:benvie]]

It appears to crash during the first rejection test, which involves running Cu.forceGC/forceCC/forceShrinkingGC. Is there any reason these would interact badly with loading Promise via loadSubScript?

Comment 5

[Brandon Benvie [:benvie]]

Nope that's not it. The test still times out every time at the same spot.

Comment 6

[Brandon Benvie [:benvie]]

Comment on attachment 8338700 [details] [diff] [review]
promise-stubify.patch

Cancelling review request until I fix this.

Comment 7

[Brandon Benvie [:benvie]]

Replacing with needinfo. Do you have any idea why changing to loading this via loadSubScript would cause this test to fail?

Comment 8

[David Teller [:Yoric] (please use "needinfo")]

I have no clear idea, Just a few observations.
- looking at the stack, it seems that the cycle collector causes some JS code to be executed - I'm not sure that's safe;
- looking at the output, it seems that we're crashing during or shortly after the test that interacts with nsIFinalizationWitnessService - it makes sense that, if anything should crash, it's that.

Comment 9

[Brandon Benvie [:benvie]]

Panos, I think you said at the work week that wanted to take a look at what was happening with this crash?

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Unfortunately the logs are no longer available on tbpl and I can't get Firefox to crash locally with this patch. I may have botched rebasing though, as it has bitrotten a bit.

Comment 11

[Brandon Benvie [:benvie]]

Hmm it looks like the problem has been fixed.

https://tbpl.mozilla.org/?tree=Try&rev=7ba15efb6296

Comment 12

[Brandon Benvie [:benvie]]

Attachment: promise-stubify.patch

Rebased.

Comment 13

[:Paolo Amadini]

Attachment: Updated patch

I updated your patch to the latest version of the Promise module, and clarified the use of the JS file by renaming it to "Promise-backend.js" and keeping the documentation in the main Promise.jsm file.

If this sounds good to you, I think the change is ready to land.

Comment 14

[:Paolo Amadini]

I'll be updating the whiteboard of various Promise bugs with "p=" entries, indicating my take on the effort estimate and difficulty level of the bug. We're doing this in general for work in which the Firefox Desktop team is involved.

Comment 15

[Brandon Benvie [:benvie]]

Comment on attachment 8389737 [details] [diff] [review]
Updated patch

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

LGTM. I'll put r=paolo on this even though I'm giving the r+ here (since I'm just r+ing the diff between my patch and yours.

Comment 16

[:Paolo Amadini]

Thanks! Landed on fx-team with r=paolo.

https://hg.mozilla.org/integration/fx-team/rev/1c3076666191

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Backed out for numerous test failures. Please run this through Try before re-pushing.
https://hg.mozilla.org/integration/fx-team/rev/f9f13eefdc2d

Comment 18

[:Paolo Amadini]

Attachment: Fix for B2G

There were indeed failures on B2G, that looked like infrastructure failures on TBPL, but were caused by the fact that my patch didn't take into account the difference in the global scope of JSM modules between B2G and other platforms.

This is a tentative fix, let's see how it goes:

https://tbpl.mozilla.org/?tree=Try&rev=d9dc99a49c0b

Comment 19

[:Paolo Amadini]

The tryserver build looks reasonably green, with some known intermittent failures. Retrying:

https://hg.mozilla.org/integration/fx-team/rev/53e8cf0a814b

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/53e8cf0a814b