Bug 84566 – Web Inspector: [chromium] show more useful information for throttled requests.

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

Currently, when a request is intentionally throttled by URLRequestThrottler, it shows up as a failure in the DevTools console and the network tab, without any specific information. It may cause confusion for users because it looks the same as other failures, such as the request is sent but no response is received.

The corresponding Chromium bug is http://code.google.com/p/chromium/issues/detail?id=109857.

I will upload a patch soon.

Created attachment 138444 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

Hi, Inspector experts.

This patch shows the description for failed requests in the Status column of the Network tab.

Currently the Status column shows things like:
successful request: "304 \n Not Modified"
failed request: "(failed)"

This patch will show the following for failed requests:
"(failed) \n <localizedFailDescription>"

To me, this adds more useful information for failures (e.g., for requests rejected by the Chromium-specific feature of HTTP throttling). And it matches the successful cases, which have already had a human-friendly description.

Please let me know if this looks okay to you. I would be happy to make changes if you have better ideas. Thanks in advance!

[WebKit Review Bot]

Comment on attachment 138444 [details]
Patch

Clearing flags on attachment: 138444

Committed r115086: <http://trac.webkit.org/changeset/115086>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

It made a test fail on Qt platform:

--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/inspector/network-status-non-http-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/inspector/network-status-non-http-actual.txt 
@@ -1,6 +1,6 @@
 {
     0 : "data:application/javascript: Success"
     1 : "network-test.js: Success"
-    2 : "non-existent-file.js: (failed)"
+    2 : "non-existent-file.js: (failed)Error opening /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/LayoutTests/inspector/non-existent-file.js: No such file or directory"
 }
 
Could you check it, please?

[Szilard Ledan]

This patch inspector/network-status-non-http.html fails on qt. This test has been skipped until it is fixed. See http://trac.webkit.org/changeset/115188
Please unskip it with the proper fix.

Sorry for the inconvenience.
I will fix it as soon as possible.

Created attachment 138916 [details]
Patch

Comment on attachment 138916 [details]
Patch

This patch fixes the test regression. Please take a look.

[Yury Semikhatsky]

Comment on attachment 138916 [details]
Patch

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

> LayoutTests/inspector/network-status-non-http.html:24
> +                if (outputStatus.indexOf("(failed)") == 0)

Please use .startsWith for prefix checks.

(In reply to comment #12)
> (From update of attachment 138916 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138916&action=review
> 
> > LayoutTests/inspector/network-status-non-http.html:24
> > +                if (outputStatus.indexOf("(failed)") == 0)
> 
> Please use .startsWith for prefix checks.

Javascript string doesn't have .startsWith method, does it?

[Pavel Feldman]

Comment on attachment 138916 [details]
Patch

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

>>> LayoutTests/inspector/network-status-non-http.html:24
>>> +                if (outputStatus.indexOf("(failed)") == 0)
>> 
>> Please use .startsWith for prefix checks.
> 
> Javascript string doesn't have .startsWith method, does it?

We added it to the String's prototype in utilities.js

[WebKit Review Bot]

Comment on attachment 138916 [details]
Patch

Rejecting attachment 138916 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ueue/

Parsed 3 diffs from patch file(s).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/inspector/network-status-non-http.html
patching file LayoutTests/platform/qt/Skipped
Hunk #1 FAILED at 2616.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/qt/Skipped.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Pavel Feld..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12597070

(In reply to comment #14)
> (From update of attachment 138916 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138916&action=review
> 
> >>> LayoutTests/inspector/network-status-non-http.html:24
> >>> +                if (outputStatus.indexOf("(failed)") == 0)
> >> 
> >> Please use .startsWith for prefix checks.
> > 
> > Javascript string doesn't have .startsWith method, does it?
> 
> We added it to the String's prototype in utilities.js

(Sorry I am not familiar with this part of codebase.)
Is it allowed to directly refer to utilities.js from the LayoutTests folder?
Or we have other ways to access startsWith?

[Andreas Kling]

Committed r115720: <http://trac.webkit.org/changeset/115720>