Open Bug 1623482 Opened 5 years ago Updated 2 years ago

Without calling a parent Runtime domain method no execution contexts are tracked internally

Categories

(Remote Protocol :: CDP, defect, P3)

defect

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [puppeteer-beta2-mvp])

With the chrome-remote-interface.js client I run the following code right after Chrome has been started and I get a valid response:

var result = Runtime.evaluate({ expression: "42n"});
Promise {
  {
    result: { type: 'bigint', unserializableValue: '42n', description: '42n' }
  },
  inspect: [AsyncFunction]
}

With Firefox instead it fails:

>>> var result = Runtime.evaluate({ expression: "42n"});
>>> (node:71447) UnhandledPromiseRejectionWarning: Error: can't access property "evaluate", context is null (evaluate@chrome://remote/content/domains/content/Runtime.jsm:124:5
execute@chrome://remote/content/domains/DomainCache.jsm:99:25
receiveMessage@chrome://remote/content/sessions/ContentProcessSession.jsm:69:45
MessageListener.receiveMessage*ContentProcessSession@chrome://remote/content/sessions/ContentProcessSession.jsm:27:25
@chrome://remote/content/sessions/frame-script.js:12:1
)
    at /Users/henrik/code/chrome-remote-interface/lib/chrome.js:93:35
    at Chrome._handleMessage (/Users/henrik/code/chrome-remote-interface/lib/chrome.js:256:17)
    at WebSocket.<anonymous> (/Users/henrik/code/chrome-remote-interface/lib/chrome.js:234:22)
    at WebSocket.emit (events.js:196:13)
    at WebSocket.EventEmitter.emit (domain.js:471:20)
    at Receiver.receiverOnMessage (/Users/henrik/code/chrome-remote-interface/node_modules/ws/lib/websocket.js:800:20)
    at Receiver.emit (events.js:196:13)
    at Receiver.EventEmitter.emit (domain.js:471:20)
    at Receiver.dataMessage (/Users/henrik/code/chrome-remote-interface/node_modules/ws/lib/receiver.js:423:14)
    at Receiver.getData (/Users/henrik/code/chrome-remote-interface/node_modules/ws/lib/receiver.js:353:17)
(node:71447) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:71447) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Only when I enable the Runtime and run the same code again it works:

>>> Runtime.enable()
Promise { <pending>, inspect: [AsyncFunction] }
>>> var result = Runtime.evaluate({ expression: "42n"});
>>> result
Promise {
  {
    result: {
      objectId: '6766a3d2-1709-9342-a793-575475dd6800',
      type: 'bigint'
    }
  },
  inspect: [AsyncFunction]
}

Please don't obey the returned data, which is clearly wrong, and needs to be fixed in a different bug.

What we have to fix is that the call has to be return without an error when the Runtime is not enabled.

Whiteboard: [puppeteer-beta-reserve]

Please note that a couple of browser chrome tests have to use Runtime.enable() to workaround this bug, and need to be updated with the fix.

By having the FrameTree (see bug 1605359) the opened execution contexts (browsing contexts) will be tracked there. So it might fix this problem.

Depends on: 1605359

With bug 1593226 I'm going to refactor the context observer. So it will be best to wait for it first.

Depends on: 1593226

Ok, I found the problem. The underlying reason here is that the Runtime domain instance is created when the first access happens. Which is for Runtime.enabled(). Until that happens the context observer is not initialized, nor any execution context tracked internally.

Maja, I think we should at least initialize the Runtime domain immediately inside the constructor of the ContentProcessSession given that it tracks crucial internal states. Not sure about other domains. What do you think?

Flags: needinfo?(mjzffr)

That's fine in the short-term.

Another idea is to factor that internal state out of the Runtime domain into the Session (or elsewhere). Intuitively, if there's internal state that's shared across domains, it doesn't belong in any one domain. That approach might also help with keeping Page+Runtime decoupled beyond Bug 1603223 as we implement more functionality around script evaluation and isolated worlds.

Flags: needinfo?(mjzffr)

Sounds like a good idea. Maybe we can take my solution first to get the immediate problem fixed, and do the refactoring, which might take a bit, on a follow-up bug?

Blocks: 1636384

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

Sounds like a good idea. Maybe we can take my solution first to get the immediate problem fixed, and do the refactoring, which might take a bit, on a follow-up bug?

Sure, a follow-up sounds good to me.

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Severity: normal → S3
Priority: P3 → P2
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: P2 → P1

This is actually more complicated as I first thought. Basically even by adding the this.domains.get("Runtime") call into the constructor of ContentProcessSession, it's too late. This is probably because the TabTarget installs the framescript, but doesn't wait until it has been initialized. Instead it already fires Target.targetCreated events even without the default execution context(s) being created.

Also the context observer is registered too late and as such would have to iterate through all frames and isolated worlds, to built-up the context list internally, and to send out the events if wanted.

I would say we should wait here until we have finished the JSWindowActor implementation.

Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Depends on: 1565162
Priority: P1 → P3

This is not tied to Runtime.enable but to the very first call of a domain. I noticed the same for the Page domain too, which I will file a separate bug about.

Summary: Without a call to Runtime.enable() no execution contexts are tracked internally → Without a call to Runtime no execution contexts are tracked internally

Similar as bug 1645065 for the page domain:

Here is something more as what I discovered yesterday. Even when you call a content page domain method the parent Page domain is not getting initialized. You explicitly have to call one of its own methods.

The same would apply to Runtime.

Summary: Without a call to Runtime no execution contexts are tracked internally → Without calling a parent Runtime domain method no execution contexts are tracked internally
Whiteboard: [puppeteer-beta-reserve] → [puppeteer-beta2-mvp]
Component: CDP: Runtime → CDP
You need to log in before you can comment on or make changes to this bug.