Bug 78445 – Web Inspector: include failed and canceled in FrameResourceTree

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Timothy Hatcher]

The protocol should include some more info about resources in FrameResourceTree. Like failed, canceld, loading, resource size and request identifier.

[Timothy Hatcher]

Created attachment 126693 [details]
Proposed Change

[Philippe Normand]

Comment on attachment 126693 [details]
Proposed Change

Attachment 126693 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11507627

[Early Warning System Bot]

Comment on attachment 126693 [details]
Proposed Change

Attachment 126693 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11507628

[Gyuyoung Kim]

Comment on attachment 126693 [details]
Proposed Change

Attachment 126693 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11509553

[Timothy Hatcher]

Comment on attachment 126693 [details]
Proposed Change

Forgot the CachedResource.h change adding loader().

[Timothy Hatcher]

Created attachment 126698 [details]
Proposed Change (with build fix)

[Pavel Feldman]

Comment on attachment 126698 [details]
Proposed Change (with build fix)

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

At this moment, we only add methods to the protocol that are used by the front-end. This way we can guarantee it is useful and we can test it. Are you planning on using this data in the front-end or is there alternative front-end it is for? I am fine with changing the rules to the "we can expose functionality not used by the inspector front-end", but we need to make sure these methods are tested.

> Source/WebCore/inspector/Inspector.json:141
> +                                { "name": "decodedSize", "type": "integer", "optional": true, "description": "Decoded resource size." },

These are going to be wrong for the ports that have network stack responsible for encoding.

> Source/WebCore/inspector/Inspector.json:143
> +                                { "name": "loading", "type": "boolean", "optional": true, "description": "True if the resource is still loading." },

I am not sure how relevant this is given the asynchronous nature of the front-end - backend interaction. There is no guarantee that it is still "true" at the moment message is received.

[Timothy Hatcher]

(In reply to comment #7)
> (From update of attachment 126698 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126698&action=review
> 
> At this moment, we only add methods to the protocol that are used by the front-end. This way we can guarantee it is useful and we can test it. Are you planning on using this data in the front-end or is there alternative front-end it is for? I am fine with changing the rules to the "we can expose functionality not used by the inspector front-end", but we need to make sure these methods are tested.

Do we have other tests that just test the protocol and not the front-end?

> 
> > Source/WebCore/inspector/Inspector.json:141
> > +                                { "name": "decodedSize", "type": "integer", "optional": true, "description": "Decoded resource size." },
> 
> These are going to be wrong for the ports that have network stack responsible for encoding.

I'm fine just having one property, "size",  and using encodedSize.

> > Source/WebCore/inspector/Inspector.json:143
> > +                                { "name": "loading", "type": "boolean", "optional": true, "description": "True if the resource is still loading." },
> 
> I am not sure how relevant this is given the asynchronous nature of the front-end - backend interaction. There is no guarantee that it is still "true" at the moment message is received.

You would also observe the Network.loadingFailed and Network.loadingFinished events. You would have those events queued up to make it false.

[Timothy Hatcher]

Created attachment 126951 [details]
Proposed Change (round 2)

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets
Using index info to reconstruct a base tree...
<stdin>:1578: trailing whitespace.
        
<stdin>:1647: trailing whitespace.
    
<stdin>:1657: trailing whitespace.
    
<stdin>:1672: trailing whitespace.
        return 0;        
<stdin>:1674: trailing whitespace.
    
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 168766 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


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

[Pavel Feldman]

Comment on attachment 126951 [details]
Proposed Change (round 2)

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

> LayoutTests/inspector/protocol/page-agent-expected.txt:57
> +                {

I wonder if this could result in flakiness. Do you know if the order cached resources are created and persisted is fixed?

[WebKit Review Bot]

Comment on attachment 126951 [details]
Proposed Change (round 2)

Attachment 126951 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11514268

New failing tests:
inspector/protocol/page-agent.html

[Tony Chang]

Comment on attachment 126951 [details]
Proposed Change (round 2)

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

>> LayoutTests/inspector/protocol/page-agent-expected.txt:57
>> +                {
> 
> I wonder if this could result in flakiness. Do you know if the order cached resources are created and persisted is fixed?

This is indeed causing flakiness.  We're getting a different order on the 3 bots that have run this test so far.  This is also why the ews bot failed.

[Timothy Hatcher]

Sigh. I'm going to disable the test. I want to keep the change.

[Timothy Hatcher]

Removed the test in r107720 and filed bug 78621 about making a better test.

[Timothy Hatcher]

Fixed in r107714.