Bug 41642 - Web Inspector: Show Error Messages for Application Cache Errors
Summary: Web Inspector: Show Error Messages for Application Cache Errors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-05 15:03 PDT by Joseph Pecoraro
Modified: 2011-05-16 11:36 PDT (History)
10 users (show)

See Also:


Attachments
proposed fix (5.44 KB, patch)
2011-05-12 17:07 PDT, Alexey Proskuryakov
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
proposed fix (5.63 KB, patch)
2011-05-12 17:32 PDT, Alexey Proskuryakov
pfeldman: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from cr-jail-7 (230.01 KB, application/zip)
2011-05-12 21:23 PDT, WebKit Commit Bot
no flags Details
patch for landing (11.22 KB, patch)
2011-05-12 21:48 PDT, Alexey Proskuryakov
ap: commit-queue-
Details | Formatted Diff | Diff
patch for landing (11.23 KB, patch)
2011-05-12 23:31 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2010-07-05 15:03:49 PDT
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!
Comment 1 Alexey Proskuryakov 2011-05-12 17:07:12 PDT
Created attachment 93370 [details]
proposed fix
Comment 2 Early Warning System Bot 2011-05-12 17:17:38 PDT
Comment on attachment 93370 [details]
proposed fix

Attachment 93370 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8688889
Comment 3 WebKit Review Bot 2011-05-12 17:29:15 PDT
Comment on attachment 93370 [details]
proposed fix

Attachment 93370 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8686921
Comment 4 Alexey Proskuryakov 2011-05-12 17:32:14 PDT
Created attachment 93373 [details]
proposed fix

With Qt build fix.
Comment 5 Joseph Pecoraro 2011-05-12 17:59:40 PDT
Looks good. No tests?
Comment 6 Pavel Feldman 2011-05-12 19:06:16 PDT
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.
Comment 7 Joseph Pecoraro 2011-05-12 19:12:45 PDT
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.
Comment 8 WebKit Commit Bot 2011-05-12 21:23:21 PDT
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
Comment 9 WebKit Commit Bot 2011-05-12 21:23:25 PDT
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
Comment 10 Alexey Proskuryakov 2011-05-12 21:48:59 PDT
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.
Comment 11 WebKit Commit Bot 2011-05-12 21:58:11 PDT
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
Comment 12 Joseph Pecoraro 2011-05-12 22:08:57 PDT
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.
Comment 13 Alexey Proskuryakov 2011-05-12 22:19:10 PDT
Comment on attachment 93401 [details]
patch for landing

Sure. I've decided to actually build the patch, and run layout tests this time.
Comment 14 Alexey Proskuryakov 2011-05-12 23:31:18 PDT
Created attachment 93405 [details]
patch for landing
Comment 15 WebKit Commit Bot 2011-05-13 00:20:14 PDT
Comment on attachment 93405 [details]
patch for landing

Clearing flags on attachment: 93405

Committed r86417: <http://trac.webkit.org/changeset/86417>
Comment 16 WebKit Commit Bot 2011-05-13 00:20:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 anton muhin 2011-05-13 03:37:40 PDT
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.
Comment 18 Alexey Proskuryakov 2011-05-13 08:51:33 PDT
I believe Chromium logs similar messages in some different way, so they don't show up in DRT.
Comment 19 Michael Nordman 2011-05-14 14:53:50 PDT
(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?
Comment 20 Alexey Proskuryakov 2011-05-14 21:53:45 PDT
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.
Comment 21 Michael Nordman 2011-05-16 11:36:13 PDT
(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.