bing.com search results were causing a crash today. rdar://problem/7881290
Created attachment 245113 [details] proposed fixd
Comment on attachment 245113 [details] proposed fixd View in context: https://bugs.webkit.org/attachment.cgi?id=245113&action=review > Source/WebCore/ChangeLog:12 > + state after failing to load. We get rid of the cache once main resource finishes Nit: "...once main..." => "...once the main...". > LayoutTests/http/tests/appcache/404-resource-with-slow-main-resource.php:1 > +<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>. > LayoutTests/http/tests/appcache/404-resource-with-slow-main-resource.php:13 > + 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. > LayoutTests/http/tests/appcache/404-resource-with-slow-main-resource.php:21 > +// 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. > LayoutTests/http/tests/appcache/404-resource-with-slow-main-resource.php:24 > +<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. > LayoutTests/http/tests/appcache/404-resource-with-slow-main-resource.php:27 > + 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 > error? 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
Committed <http://trac.webkit.org/r178937>.
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.