Bug 264374 - Web Inspector: support awaitPromise parameter in Runtime.callFunctionOn
Summary: Web Inspector: support awaitPromise parameter in Runtime.callFunctionOn
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords: InRadar
Depends on:
Blocks: 264920
  Show dependency treegraph
 
Reported: 2023-11-07 16:35 PST by Yury Semikhatsky
Modified: 2023-11-15 20:44 PST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2023-11-07 16:35:04 PST
Today, the only way to await a promise in the protocol is to call `Runtime.awaitPromise`. In the scenario when the front-end need to get the result by value, it incurs one or two additional round trips to the backend:  1)  call the function with `returnByValue: false`; 2) if the result is a promise send Runtime.awaitPromise command to await it; 3) Call function to get serialized value of the result from step 1 or 2. This has a fundamental flaw: by the time Runtime.awaitPromise is sent to the backend, the page may have reloaded and the promise we want to await or the object we want to returnByValue may be gone.

Having `awaitPromise` as a parameter on `Runtime.callFunctionOn` allows to avoid extra round-trips to the backend. If the command is called with awaitPromise=true and returnByValue=true, it is guaranteed to return resulting object value even in case of navigation.
Comment 1 Devin Rousso 2023-11-07 16:46:33 PST
if you also add `awaitPromise` to `Audit.run` then Web Inspector could/would use it in `WI.AuditTestBase.prototype.runSetup` and `WI.AuditTestCase.prototype.run` too =D

for consistency/"completeness" id also suggest adding it to `Runtime.evaluate` and `Debugger.evaluateOnCallFrame` as well
Comment 2 Yury Semikhatsky 2023-11-07 16:51:35 PST
Pull request: https://github.com/WebKit/WebKit/pull/20137
Comment 3 Yury Semikhatsky 2023-11-07 17:27:43 PST
(In reply to Devin Rousso from comment #1)
> if you also add `awaitPromise` to `Audit.run` then Web Inspector could/would
> use it in `WI.AuditTestBase.prototype.runSetup` and
> `WI.AuditTestCase.prototype.run` too =D
> 

Given that we'd need to check for pretense of the new parameter in the protocol and the code already prefers AuditAgent.run if it's present, I believe it'd be sufficient to add the parameter to Audit.run. In practice, it would always be called it with awaitPromise=true. Do you think it's worth updating?

> for consistency/"completeness" id also suggest adding it to
> `Runtime.evaluate` and `Debugger.evaluateOnCallFrame` as well

I'm a bit hesitant updating these commands if there is no demand. With callFunctionOn we've been using the parameter for a while and did not need it in other places. I can certainly implement it there as well.
Comment 4 Radar WebKit Bug Importer 2023-11-14 16:36:15 PST
<rdar://problem/118425148>
Comment 5 EWS 2023-11-15 12:24:23 PST
Committed 270784@main (700de83458a2): <https://commits.webkit.org/270784@main>

Reviewed commits have been landed. Closing PR #20137 and removing active labels.