bing.com search results were causing a crash today.
Created attachment 245113 [details]
Comment on attachment 245113 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=245113&action=review
> + state after failing to load. We get rid of the cache once main resource finishes
Nit: "...once main..." => "...once the main...".
> +<html manifest="resources/404-resource.manifest">
As far as I can tell this page doesn't depend on quicks mode. So, I suggest we add the HTML5 doctype to the top of this file: <!DOCTYPE html>.
> + document.getElementById('result').innerHTML = 'SUCCESS';
Nit: This line used single quotes for string literals and the line below using double quotes. We should pick one style and stick with it for consistency.
> +// The test needs to attempt to load a subresource after appcache load fails. There is no way
> +// to observe appcache loading progress before onload fires, so we just let it run for a while.
> +window.setTimeout(finish, 1000);
:( I wish there was a better way to write this test and avoid this setTimeout(). I do not know of a better way at the moment.
> +<div id=container>
Nit: The quoting style used for HTML attributes on this line differs from the quoting style you used above (e.g. <div id="result">). We should pick one style and stick with it throughout the test for consistency. I suggest we surround the attribute value "container" in double quotes.
> + usleep(100000);
How did you come to the decision to wait 100000 microseconds? Trial and error?
Created attachment 245153 [details]
patch for landing
Applied most comments, except for the one below.
> As far as I can tell this page doesn't depend on quicks mode. So, I suggest
> we add the HTML5 doctype to the top of this file: <!DOCTYPE html>.
The test doesn't depend on standards mode either, so I don't see what the benefit of adding a noise line to it would be.
> How did you come to the decision to wait 100000 microseconds? Trial and
This was the first value that I tried, and it worked. I don't think that it matters much; the 1 second timeout to finish is a lot more questionable, as it makes the test always take a whole second. As you said there doesn't appear to be any better way.
Comment on attachment 245153 [details]
patch for landing
Rejecting attachment 245153 [details] from commit-queue.
Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 245153, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Dan Bates found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Full output: http://webkit-queues.appspot.com/results/4869605826756608
The test was failing on Mavericks Release WK1, there was no console message about appcache load failure. Tweaked the timeouts, hoping that this would help.