Bug 192171 – Web Inspector: Audit: tests should support async operations

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

If a test returns a Promise (either by manually `return new Promise(...)` or using an `async function() { ... }`), we should wait on that promise before completing the test.

[Devin Rousso]

Created attachment 356034 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[EWS Watchlist]

Attachment 356034 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:97:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:97:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:97:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:138:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:138:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:138:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356034 [details]
Patch

Attachment 356034 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10201662

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356045 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356034 [details]
Patch

Attachment 356034 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10201765

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356051 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Devin Rousso]

Created attachment 356056 [details]
Patch

Add locking

[EWS Watchlist]

Attachment 356056 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:139:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:139:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:139:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356056 [details]
Patch

Attachment 356056 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10203425

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356073 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356056 [details]
Patch

Attachment 356056 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10204152

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356077 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356056 [details]
Patch

Attachment 356056 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10204801

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356096 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Devin Rousso]

Created attachment 356160 [details]
Patch

[EWS Watchlist]

Attachment 356160 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356160 [details]
Patch

Attachment 356160 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10212121

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356164 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356160 [details]
Patch

Attachment 356160 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10212233

New failing tests:
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356166 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 356160 [details]
Patch

Attachment 356160 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10214898

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall.https.html

[EWS Watchlist]

Created attachment 356183 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Devin Rousso]

Created attachment 356200 [details]
Patch

[EWS Watchlist]

Attachment 356200 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:98:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:134:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 356160 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356160&action=review

> Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:131
> +    ASSERT(!hadException);
> +    // FIXME: since `callback` is moved above, we can't call it if there's an execption while trying
> +    //        to execute the `JSNativeStdFunction` inside InjectedScriptSource.js.

I think it is fine to just ASSERT here. But you can invoke the JSNativeStdFunction yourself (or reach into it and call it). Just save a reference to it yourself above.

> Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:39
> +    static supportsAwaitPromise()

Can we just inline this, with a

    // COMPATIBILITY (iOS 12): ...

> LayoutTests/inspector/runtime/awaitPromise.html:32
> +    function addValueTest(name, value) {
> +        addTest(name, `Promise.resolve(${JSON.stringify(value)})`, {returnByValue: true, saveResult: true}, (remoteObject, wasThrown) => {

There should be tests for a rejected promise. There are only Tests here for resolved promises.

[Devin Rousso]

Created attachment 356218 [details]
[Patch] Mapped Callbacks

Store callbacks within a map on InjectedScriptBase so that they can later be retrieved

[EWS Watchlist]

Attachment 356218 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:141:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:141:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:141:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 356220 [details]
Patch

[EWS Watchlist]

Attachment 356220 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356220 [details]
Patch

Attachment 356220 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10218484

New failing tests:
inspector/runtime/awaitPromise.html
inspector/audit/basic.html

[EWS Watchlist]

Created attachment 356228 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Devin Rousso]

Created attachment 356229 [details]
Patch

[Devin Rousso]

Created attachment 356286 [details]
Patch

This is what happens when you manually edit a diff... :|

[EWS Watchlist]

Attachment 356286 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Radar WebKit Bug Importer]

<rdar://problem/46423562>

[Joseph Pecoraro]

Comment on attachment 356286 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356286&action=review

r=me

> Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:138
> +        // Since `callback` is moved above, we can't call it if there's an execption while trying to

Typo: "execption"

> Source/JavaScriptCore/inspector/protocol/Runtime.json:230
> +            "description": "Evaluates promise on global object.",

Might use a better description. This calls the callback when the given objectId (which must be a Promise) resolves / rejects.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:239
> +            let response = null;
> +
> +            metadata.startTimestamp = new Date;
> +            response = await RuntimeAgent.evaluate.invoke(evaluateArguments);

Merge the `let response = ...` here?

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:244
> +                    metadata.asyncTimestamp = metadata.endTimestamp;

Note to Devin: Add `asyncTimestamp` to `fromPayload` (so export/import uses it).

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:249
> +                    addError(WI.UIString("Async audits are not supported on this device."));

Probably better to just say "are not support". The "on this device" thing is a little weird.

> LayoutTests/inspector/runtime/awaitPromise.html:49
> +        let expression = `new Promise((resolve, reject) => setTimeout(reject, 100, ${JSON.stringify(value)}))`;

If this is triggering onunhandledrejection console messages we should provide a way in tests to mute those so that this doesn't end up being flakey.

> LayoutTests/inspector/runtime/awaitPromise.html:77
> +    addRejectTest("Runtime.awaitPromise.Reject.Object", {a: 1, b: 2});

I'd also want to see a Promise chain.

    • Promise (1) that resolves to
        -> Promise (2) that resolves to
            -> Promise (3) that resolves to
                -> a value

Should end up with the final value and not object (2).

[Devin Rousso]

Created attachment 356453 [details]
Patch

[EWS Watchlist]

Attachment 356453 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 356457 [details]
Patch

[EWS Watchlist]

Attachment 356457 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 356457 [details]
Patch

Attachment 356457 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10258952

New failing tests:
inspector/runtime/awaitPromise.html

[EWS Watchlist]

Created attachment 356470 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Devin Rousso]

Created attachment 356471 [details]
Patch

[EWS Watchlist]

Attachment 356471 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:99:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_resultObject is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_wasThrown is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/InjectedScriptBase.cpp:144:  out_savedResultIndex is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 356471 [details]
Patch

Clearing flags on attachment: 356471

Committed r238850: <https://trac.webkit.org/changeset/238850>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.