Bug 190318 – Web Inspector: refactor constructor of WI.Resource

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

[Devin Rousso]

It's a pretty huge mess right now, especially if we need to add more parameters.

[Devin Rousso]

Created attachment 351685 [details]
Patch

[EWS Watchlist]

Comment on attachment 351685 [details]
Patch

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

New failing tests:
http/tests/websocket/tests/hybi/inspector/resolveWebSocket.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 351685 [details]
Patch

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

New failing tests:
http/tests/websocket/tests/hybi/inspector/resolveWebSocket.html

[EWS Watchlist]

Created attachment 351692 [details]
Archive of layout-test-results from ews100 for mac-sierra

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

[Devin Rousso]

Created attachment 351694 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 351694 [details]
Patch

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

r=me if all tests are green.

> Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:139
> +            let frameResource = this._addNewResourceToFrameOrTarget(framePayload.url, framePayload.id, {
> +                loaderIdentifier: framePayload.loaderId,
> +            }, {
> +                name: framePayload.name,
> +                securityOrigin: framePayload.securityOrigin,
> +            });

Can we style this like:

    let resourceOptions = { ... };
    let frameOptions = { ... };
    let frameResource = this._addNewResourceToFrameOrTarget(...);

> Source/WebInspectorUI/UserInterface/Models/WebSocketResource.js:-28
> -    constructor(url, loaderIdentifier, targetId, requestIdentifier, requestHeaders, requestData, timestamp, walltime, requestSentTimestamp, initiatorSourceCodeLocation)

It seems concerning that this takes a subset of Resource initialization options and so might need to be manually added back if used later.

[Devin Rousso]

Comment on attachment 351694 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/WebSocketResource.js:-28
>> -    constructor(url, loaderIdentifier, targetId, requestIdentifier, requestHeaders, requestData, timestamp, walltime, requestSentTimestamp, initiatorSourceCodeLocation)
> 
> It seems concerning that this takes a subset of Resource initialization options and so might need to be manually added back if used later.

One of the nice things about using an object-bag is that it makes it MUCH easier/cleaner to re-add these if needed.  Updating both the constructor and any call-sites is much simpler to keep track of.  :)

[Devin Rousso]

Created attachment 351700 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 351700 [details]
Patch

Clearing flags on attachment: 351700

Committed r236891: <https://trac.webkit.org/changeset/236891>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45060404>