The protocol should include some more info about resources in FrameResourceTree. Like failed, canceld, loading, resource size and request identifier.
Created attachment 126693 [details] Proposed Change
Comment on attachment 126693 [details] Proposed Change Attachment 126693 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11507627
Comment on attachment 126693 [details] Proposed Change Attachment 126693 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11507628
Comment on attachment 126693 [details] Proposed Change Attachment 126693 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11509553
Comment on attachment 126693 [details] Proposed Change Forgot the CachedResource.h change adding loader().
Created attachment 126698 [details] Proposed Change (with build fix)
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.
(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.
Created attachment 126951 [details] Proposed Change (round 2)
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.
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?
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
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.
Sigh. I'm going to disable the test. I want to keep the change.
Removed the test in r107720 and filed bug 78621 about making a better test.
Fixed in r107714.