Bug 41642 – Web Inspector: Show Error Messages for Application Cache Errors

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

[Joseph Pecoraro]

This would be extremely useful for developers working with Application Caches.
Error events trigger right now and give no indication of what the error was.
Some errors include:

  - manifest file not found / redirect
  - manifest file wrong mime type
  - could not parse manifest file  (and why?)
  - potentially reached storage limit / quota (see bug 40627)
  - other reasons!

[Alexey Proskuryakov]

Created attachment 93370 [details]
proposed fix

[Early Warning System Bot]

Comment on attachment 93370 [details]
proposed fix

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

[WebKit Review Bot]

Comment on attachment 93370 [details]
proposed fix

Attachment 93370 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8686921

[Alexey Proskuryakov]

Created attachment 93373 [details]
proposed fix

With Qt build fix.

[Joseph Pecoraro]

Looks good. No tests?

[Pavel Feldman]

Comment on attachment 93373 [details]
proposed fix

I don't think we can regress it badly, so I am fine with no tests on this one.

[Joseph Pecoraro]

I just wanted different cases to test each of the different error messages,
and show how they could be produced. I am also fine with no tests, it
was just a preference.

[WebKit Commit Bot]

Comment on attachment 93373 [details]
proposed fix

Rejecting attachment 93373 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build-..." exit_code: 2

Last 500 characters of output:
ests/xmlhttprequest ................................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers ...........
http/tests/xmlviewer .
http/tests/xmlviewer/dumpAsText ...........
767.22s total testing time

23510 test cases (99%) succeeded
8 test cases (<1%) had incorrect layout
15 test cases (<1%) had stderr output

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

[WebKit Commit Bot]

Created attachment 93397 [details]
Archive of layout-test-results from cr-jail-7

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: cr-jail-7  Port: Mac  Platform: Mac OS X 10.6.7

[Alexey Proskuryakov]

Created attachment 93401 [details]
patch for landing

I don't know why I thought that this couldn't affect tests... Took results from the bot.

[WebKit Commit Bot]

Comment on attachment 93401 [details]
patch for landing

Rejecting attachment 93401 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build'..." exit_code: 2

Last 500 characters of output:
v YACC /Developer/usr/bin/yacc
    /bin/sh -c /mnt/git/webkit-commit-queue/WebKitBuild/WebCore.build/Debug/WebCore.build/Script-5DF50887116F3077005202AB.sh

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /mnt/git/webkit-commit-queue/WebKitBuild/WebCore.build/Debug/WebCore.build/Objects-normal/x86_64/ApplicationCacheGroup.o /mnt/git/webkit-commit-queue/Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2
(1 failure)


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

[Joseph Pecoraro]

Comment on attachment 93401 [details]
patch for landing

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

> Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:530
> +                ((response.httpStatusCode() / 100 != 2) ? " could not be fetched.", " was redirected."), 0, String());

I think in:

    ((response.httpStatusCode() / 100 != 2) ? " could not be fetched.", " was redirected.")

The comma should be a colon, for the ternary operator.

[Alexey Proskuryakov]

Comment on attachment 93401 [details]
patch for landing

Sure. I've decided to actually build the patch, and run layout tests this time.

[Alexey Proskuryakov]

Created attachment 93405 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 93405 [details]
patch for landing

Clearing flags on attachment: 93405

Committed r86417: <http://trac.webkit.org/changeset/86417>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[anton muhin]

This change demonstrates that WebKit and Chromium now behave differently.  I've rebaselined Chromium expectations (http://trac.webkit.org/changeset/86423, sorry old svn-commit.tmp go in the way, so description is false).  Please, have a look if it's fine to have those differences between WebKit and Chromium.

[Alexey Proskuryakov]

I believe Chromium logs similar messages in some different way, so they don't show up in DRT.

[Michael Nordman]

(In reply to comment #18)
> I believe Chromium logs similar messages in some different way, so they don't show up in DRT.

It's really unfortunate to have different test expectations because of differences in the CONSOLE output :(

Is there a way to tell DRT to not include CONSOLE output in the results?

[Alexey Proskuryakov]

We actually want to test the console logging here... 

For some kinds of console output (namely, network loader traces), we make the tests cross-platform by filtering out the differences as a post-processing step. In this case however, a better approach would be to make sure that Chromium messages match ours, and are logged in a way that reaches DRT console.

[Michael Nordman]

(In reply to comment #20)
> We actually want to test the console logging here... 
> 
> For some kinds of console output (namely, network loader traces), we make the tests cross-platform by filtering out the differences as a post-processing step. In this case however, a better approach would be to make sure that Chromium messages match ours, and are logged in a way that reaches DRT console.

I did like that the tests actually focused mostly on spec compliance and not other artifacts, but now that other non-spec compliant things are required to pass the tests... idk... maybe it's a blessing in disguise that the results are forked too.