Bug 180945 - Web Inspector: add RemoteObject.fetchProperties and some basic tests for RemoteObject API
Summary: Web Inspector: add RemoteObject.fetchProperties and some basic tests for Remo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Blaze Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-18 13:52 PST by Blaze Burg
Modified: 2022-03-01 02:31 PST (History)
4 users (show)

See Also:


Attachments
Patch (20.55 KB, patch)
2017-12-18 16:11 PST, Blaze Burg
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Blaze Burg 2017-12-18 13:52:53 PST
This makes tests nice too.
Comment 1 Blaze Burg 2017-12-18 16:11:39 PST
Created attachment 329701 [details]
Patch
Comment 2 Joseph Pecoraro 2017-12-19 16:30:59 PST
Comment on attachment 329701 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Protocol/RemoteObject.js:519
> +            let processResultForCallback = (error, result, wasThrown) => {

Nit: You could just inline this and it might read even better.

> LayoutTests/inspector/model/remote-object-api.html:15
> +    "resolved": Promise.resolve(666),
> +    "rejected": Promise.reject(new Error("I promised problems.")),

Nit: Perhaps a less contentious value.
Or remove these? They don't appear to be used, but do affect the page's output.

> LayoutTests/inspector/model/remote-object-api.html:25
> +            let object = await InspectorTest.evaluateInPage("window.testObject");

Nit: Use template strings for code. `window.testObject`, here and below.

> LayoutTests/inspector/model/remote-object-api.html:85
> +            let object = await InspectorTest.evaluateInPage("window.testObject");

Instead of doing this in each test, you can do this before runTestCasesAndFinish. That would reduce the protocol messages almost in half for this test.

> LayoutTests/inspector/model/remote-object-api.html:186
> +            InspectorTest.expectEqual(name, "Favorites", `Fetched property 'name' should equal 'Favorites'`);
> +            InspectorTest.expectEqual(size, 456, `Fetched property 'size' should equal '456'`);
> +            InspectorTest.expectThat(data instanceof WI.RemoteObject, `Fetched property 'data' should be a WI.RemoteObject`);

Nit: End these messages with a period.
Comment 3 Blaze Burg 2018-01-04 13:29:20 PST
Committed r226417: <https://trac.webkit.org/changeset/226417>
Comment 4 Radar WebKit Bug Importer 2018-01-04 13:30:39 PST
<rdar://problem/36304406>