Right now in ApplicationCacheStorage.cpp when openDatabase(true) is called it always assume it will succeed, but it could fail, for example when appcache storage is set to a storage card and the storage card is now unplugged. Plan to add m_database.isOpen() check after every openDatabase(true) call.
Created attachment 57674 [details] patch for 40074 with coding style fix.
Created attachment 57708 [details] patch for 40074 with updated comments.
Normally we require regression tests for fixes. Is there some way to test this? How did you test it?
Hi, Darin, I actually got the crash first before I know this is an issue. I just set the storage location to somewhere nonexistent and then will get the crash without the fix. (In reply to comment #3)
(In reply to comment #4) > Hi, Darin, I actually got the crash first before I know this is an issue. I just set the storage location to somewhere nonexistent and then will get the crash without the fix. I'm not sure I understand fully. Is there way to turn that into a test other people can run? Maybe something in DumpRenderTree?
(In reply to comment #5) I don't think we can test it with DumpRenderTree. And as there's no way to change the AppCache storage location of Chrome, Safari by user, I am not sure how to test this with these 2 browsers. But for our test phone, I can use a SD card as AppCache location, and if that card is unplugged, then it will crash.
Comment on attachment 57708 [details] patch for 40074 with updated comments. Rejecting patch 57708 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 2 Parsed 2 diffs from patch file(s). patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/loader/appcache/ApplicationCacheStorage.cpp patch: **** malformed patch at line 65: @@ -742,6 +750,10 @@ bool ApplicationCacheStorage::storeNewestCache(ApplicationCacheGroup* group) Full output: http://webkit-commit-queue.appspot.com/results/3101012
Created attachment 57876 [details] patch for 40074. After reading the failed commit log, not sure exactly why it failed at line #65. But re-submit a new patch updated to newest master anyway.
Comment on attachment 57876 [details] patch for 40074. Rejecting patch 57876 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 57876, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1 Last 500 characters of output: cgi?id=57876&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=40074&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 57876 from bug 40074. NOBODY (OOPS!) found in /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Users/eseidel/Projects/CommitQueue/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Created attachment 57892 [details] patch for 40074 with reviewer info updated Update patch for 40074 with correct reviewer info.
Comment on attachment 57892 [details] patch for 40074 with reviewer info updated Clearing flags on attachment: 57892 Committed r60729: <http://trac.webkit.org/changeset/60729>
All reviewed patches have been landed. Closing bug.
IMHO, I think this can be and should be tested with a layout test. This can be similar to how we test for the app cache hitting the maximum size: http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/appcache/max-size.html One could add a method on the LayoutTestController to configure the storage location to some invalid path. I think that could trigger the bug.