[webkit-changes] [WebKit/WebKit] 3583b2: REGRESSION ( Sonoma?): [ Sonoma release ] TestWebK...

[Razvan Caliman - Apple]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 3583b2ac96f10694075cac1324cf5b14a41d77cb
      https://github.com/WebKit/WebKit/commit/3583b2ac96f10694075cac1324cf5b14a41d77cb
  Author: Razvan Caliman <rcaliman at apple.com>
  Date:   2023-12-12 (Tue, 12 Dec 2023)

  Changed paths:
    A LayoutTests/inspector/unit-tests/retryuntil-expected.txt
    A LayoutTests/inspector/unit-tests/retryuntil.html
    M Source/WebInspectorUI/UserInterface/Base/Utilities.js
    M Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js

  Log Message:
  -----------
  REGRESSION ( Sonoma?): [ Sonoma release ] TestWebKitAPI.WKInspectorExtension.EvaluateScriptOnPage is a consistent failure
https://bugs.webkit.org/show_bug.cgi?id=263447
rdar://117265270

Reviewed by Timothy Hatcher.

The API test loads a page from an URL, opens Web Inspector for it, registers a Web Inspector Web Extension
and immediately sends a request to evaluate some JavaScript in the main frame and subframes for that URL.

The test fails because of a race condition: the objects that represent the frame and execution context
within Web Inspector frontend are not necessarily available when the evaluation request comes in.

The missing objects result in early returns with error codes:
`WI.WebInspectorExtension.ErrorCode.InvalidRequest` or `WI.WebInspectorExtension.ErrorCode.ContextDestroyed`.

Fixing the race means waiting for the corresponding `WI.Frame` and `WI.ExecutionContext` instances
to be available before attempting to use them. A simple retry approach with a timeout works fine.

---

A `WI.Frame` is created in response to initializing an `InspectorPageAgent`
and receiving a response to the `getResourceTree()` command.

A `WI.ExecutionContext` is created asynchronously in response to the "executionContextCreated" event
dispatched by `PageRuntimeAgent`.

The test failed consistently on Release builds but not on Debug builds. The slower Debug builds likely gave
enough time for the Web Inspector frontend to be created and populated with the necessary objects.

---

Why is Web Inspector frontend involved in an Web Extensions API test? Good question!

The call to evaluate a script from a Web Inspector web extension in the page context goes down this path:

`WebInspectorUIExtensionController::evaluateScriptForExtension` (C++)
    -> `InspectorFrontendAPIDispatcher::dispatchCommandWithResultAsync()` (C++, Web Inspector backend)
        -> `WI.WebInspectorExtensionController.evaluateScriptForExtension()` (JS, Web Inspector frontend)
            -> `InspectorRuntimeAgent::evaluate()` (C++, Web Inspector backend)

The result of the evaluation gets ferried on the same route all the way back.

* LayoutTests/inspector/unit-tests/retryuntil.html: Added.
* LayoutTests/inspector/unit-tests/retryuntil-expected.txt: Added.
* Source/WebInspectorUI/UserInterface/Base/Utilities.js:
* Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js:
(WI.WebInspectorExtensionController.prototype.async evaluateScriptForExtension):
(WI.WebInspectorExtensionController.prototype.evaluateScriptForExtension): Deleted.

Canonical link: https://commits.webkit.org/271925@main