Closed Bug 1540314 Opened 1 year ago Closed 1 year ago

Creating two Promises in the same test throws exception.

Categories

(DevTools :: Debugger, defect)

Unspecified
Android
defect
Not set

Tracking

(firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

RESOLVED FIXED
Firefox 68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: rbarker, Assigned: jimb)

References

(Blocks 1 open bug)

Details

(Whiteboard: [geckoview:fenix:m4])

Attachments

(1 file)

Trying to create a second promise in a GeckoView test causes the following error:

03-29 16:14:40.720 28430 28446 I Gecko   : console.error: "TypeError: this._navigationLifetimePool.cleanup is not a function: PromisesActor<._onWindowReady<@resource://devtools/server/actors/promises.js:194:34\nexpectState/<@resource://devtools/server/actors/common.js:213:23\nemit@resource://devtools/shared/event-emitter.js:186:24\nemit@resource://devtools/shared/event-emitter.js:267:18\n_windowReady@resource://devtools/server/actors/targets/browsing-context.js:1277:10\nDebuggerProgressListener.prototype.onWindowCreated<@resource://devtools/server/actors/targets/browsing-context.js:1593:23\nexports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:22\n"
03-29 16:14:40.720 28430 28446 E Web Content: [JavaScript Error: "TypeError: this._navigationLifetimePool.cleanup is not a function: PromisesActor<._onWindowReady<@resource://devtools/server/actors/promises.js:194:34
03-29 16:14:40.720 28430 28446 E Web Content: expectState/<@resource://devtools/server/actors/common.js:213:23
03-29 16:14:40.720 28430 28446 E Web Content: emit@resource://devtools/shared/event-emitter.js:186:24
03-29 16:14:40.720 28430 28446 E Web Content: emit@resource://devtools/shared/event-emitter.js:267:18
03-29 16:14:40.720 28430 28446 E Web Content: _windowReady@resource://devtools/server/actors/targets/browsing-context.js:1277:10
03-29 16:14:40.720 28430 28446 E Web Content: DebuggerProgressListener.prototype.onWindowCreated<@resource://devtools/server/actors/targets/browsing-context.js:1593:23
03-29 16:14:40.720 28430 28446 E Web Content: exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:22
03-29 16:14:40.720 28430 28446 E Web Content: "]
03-29 16:14:40.720 28430 28446 I GeckoDump: TypeError: this._navigationLifetimePool.cleanup is not a function: PromisesActor<._onWindowReady<@resource://devtools/server/actors/promises.js:194:34
03-29 16:14:40.720 28430 28446 I GeckoDump: expectState/<@resource://devtools/server/actors/common.js:213:23
03-29 16:14:40.720 28430 28446 I GeckoDump: emit@resource://devtools/shared/event-emitter.js:186:24
03-29 16:14:40.720 28430 28446 I GeckoDump: emit@resource://devtools/shared/event-emitter.js:267:18
03-29 16:14:40.720 28430 28446 I GeckoDump: _windowReady@resource://devtools/server/actors/targets/browsing-context.js:1277:10
03-29 16:14:40.720 28430 28446 I GeckoDump: DebuggerProgressListener.prototype.onWindowCreated<@resource://devtools/server/actors/targets/browsing-context.js:1593:23
03-29 16:14:40.720 28430 28446 I GeckoDump: exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:22

Only being able to create a single Promise per test makes creating repetitive test difficult.

I believe I know what was causing the issue:

    private fun waitForScroll(offset: Double, timeout: Double, param: String) {
        sessionRule.evaluateJS(mainSession, """
           const start = Date.now(); // This const variable seems to cause problems.
           new Promise((resolve, reject) => {
             function step() {
               console.log("REB  window.visualViewport.$param = " + window.visualViewport.pageTop);
               if (window.visualViewport.$param >= ($offset - $errorEpsilon)) {
                 resolve();
               } else if ($timeout < (Date.now() - start)) {
                 reject();
               } else {
                 window.requestAnimationFrame(step);
               }
             }
             window.requestAnimationFrame(step);
           });
        """.trimIndent()).asJSPromise().value
    }

Having the const start = Data.now() in the global scope seems to have caused the error.

Blocks: 1542525

I'm moving this bug to the Core::DOM component because Randall says appears to be a Promise bug, not a GeckoView bug. We're seeing this bug more frequently now that the GeckoView tests are using Promises more.

Component: General → DOM: Core & HTML
Product: GeckoView → Core
Priority: -- → P2

James says this is a detach() bug in the DevTools' Promise code, not the Promise Web API.

Assignee: nobody → snorp
Component: DOM: Core & HTML → General
OS: All → Android
Priority: P2 → P1
Product: Core → DevTools
Whiteboard: [geckoview:fenix:m4]
Target Milestone: --- → Firefox 68
Version: unspecified → Trunk
See Also: → 1547403

Harald, how do we get this bug in DevTools bug triage?

James can't reproduce this, but says it should be a straightforward fix for someone on the DevTools team.

Flags: needinfo?(hkirschner)

Jim, could you assess the issue to suggest next steps?

Assignee: snorp → nobody
Component: General → Debugger
Flags: needinfo?(hkirschner) → needinfo?(jimb)
Priority: P1 → --

James can't reproduce this, but says it should be a straightforward fix for someone on the DevTools team.

I think you overestimate our abilities. It is very hard to fix a bug without being able to reproduce it. If you can provide steps to reproduce, then we can start taking a look at it.

The information in this bug leaves many questions unanswered:

  • It is forbidden by ECMAScript to re-declare a const variable. So loading the triple-quoted code twice in the same global will certainly cause an error.

  • But the backtrace shows an error being thrown by devtools server code. I don't know how GeckoView tests work - why is the devtools server even active?

But, not having steps to reproduce is a blocker.

Flags: needinfo?(jimb) → needinfo?(cpeterson)

The prior code calls '.cleanup()', but ActorPools have no 'cleanup' method. I
think it means to call 'destroy'.

Well, I take that back, maybe we can fix this one just from the backtrace...

(In reply to Jim Blandy :jimb from comment #6)

  • It is forbidden by ECMAScript to re-declare a const variable. So loading the triple-quoted code twice in the same global will certainly cause an error.

Yes, trying to define const start = Data.now() in the global scope twice sounds like a problem. This evaluateJS() code should probably be wrapped in an IIFE scope, if evaluateJS() doesn't automatically do this.

(In reply to Jim Blandy :jimb from comment #7)

The prior code calls '.cleanup()', but ActorPools have no 'cleanup' method. I
think it means to call 'destroy'.

Oh! That fix makes sense given the "_navigationLifetimePool.cleanup is not a function" warning in comment 0's logs.

Assignee: nobody → jimb
Flags: needinfo?(cpeterson)
Pushed by jblandy@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40dad5764be7
Devtools promise actor: Use ActorPool 'destroy' method. r=gl
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

Randall, does Jim's promise fix also fix the GV test problems?

Flags: needinfo?(rbarker)

(In reply to Chris Peterson [:cpeterson] from comment #12)

Randall, does Jim's promise fix also fix the GV test problems?

It should. We'll know for certain if the intermittent failures stop.

Flags: needinfo?(rbarker)

How are the intermittents doing? Can we close this?

Flags: needinfo?(rbarker)
Status: RESOLVED → VERIFIED
Status: VERIFIED → RESOLVED
Closed: 1 year ago1 year ago
You need to log in before you can comment on or make changes to this bug.