Bug 157626

Summary: Modern IDB: Website data store management
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, commit-queue, jsbell
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 157757, 157823, 157843    
Bug Blocks: 149117    
Attachments:
Description Flags
WIP patch
none
Patch achristensen: review+

Brady Eidson
Reported 2016-05-12 09:31:34 PDT
Modern IDB: Website data store management This is to support the browser feature of viewing which websites store data and removing all website data.
Attachments
WIP patch (23.66 KB, patch)
2016-05-13 16:35 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch (18.93 KB, patch)
2016-05-18 17:08 PDT, Brady Eidson 		achristensen: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brady Eidson
Comment 1 2016-05-12 09:32:13 PDT
<rdar://problem/25446044>
Brady Eidson
Comment 2 2016-05-13 15:03:49 PDT
The "Deleting files" part of this will basically be existing WK2 code ported over to WebCore. But we're doing one better over that - We'll also shut down currently open UniqueIDBDatabases. This task is relatively straightforward. Testing it is not.
Brady Eidson
Comment 3 2016-05-13 16:35:16 PDT
Created attachment 278892 [details] WIP patch This is a WIP patch, not ready for review - I just want access to it from a different machine later. It is functionally complete, if not without bugs. I'll find out more about those bugs as I work on the harder task - Testing.
Brady Eidson
Comment 4 2016-05-16 14:28:11 PDT
In testing, it became apparently that I'll need a refactor first to be sure to get things right. Filed https://bugs.webkit.org/show_bug.cgi?id=157757 for that
Brady Eidson
Comment 5 2016-05-17 16:09:58 PDT
This got bigger, because a lot more bookkeeping and message passing became necessary. *sigh* Patch incoming
Brady Eidson
Comment 6 2016-05-17 16:24:18 PDT
Never mind - I'm going to have to start splitting into multiple patches to keep it sane.
Brady Eidson
Comment 7 2016-05-18 17:08:55 PDT
Created attachment 279323 [details] Patch
Alex Christensen
Comment 8 2016-05-18 18:45:40 PDT
Yep, you moved the code. LGTM. Do we need more reviewers because this touches the filesystem? It looks like Mac-debug is asserting.
Brady Eidson
Comment 9 2016-05-18 19:11:13 PDT
(In reply to comment #8) > Yep, you moved the code. LGTM. > Do we need more reviewers because this touches the filesystem? I don't think so - it's literally a move of existing code > It looks like Mac-debug is asserting. I'll have to take a look in the AM.
Brady Eidson
Comment 10 2016-05-18 20:42:13 PDT
Yikes, those tests were in terrible shape - Not only 1 or 2 IDB tests asserting, but lots of other failures. Well, I'll try to dig in to the bot and get those crash logs (since they weren't packaged up in any report that I could find) and also try to repro locally (in the morning)
Brady Eidson
Comment 11 2016-05-19 09:45:31 PDT
(In reply to comment #10) > Yikes, those tests were in terrible shape - Not only 1 or 2 IDB tests > asserting, but lots of other failures. > > Well, I'll try to dig in to the bot and get those crash logs (since they > weren't packaged up in any report that I could find) and also try to repro > locally (in the morning) Was able to reproduce the DRT crash locally... once. But the crash log should be enough to help fix.
Brady Eidson
Comment 12 2016-05-19 10:03:56 PDT
Actually, it's a mysterious hashing issue, not an automatic easy fix. Ugh.
Brady Eidson
Comment 13 2016-05-19 12:08:33 PDT
Alexey has pointed out on IRC that this crash is already in the tree. https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK1%20(Tests)/r201160%20(6374)/storage/websql/database-lock-after-reload-crash-log.txt for example. So it's unrelated to this patch. So this patch can be reviewed as-is
Brady Eidson
Comment 14 2016-05-19 15:36:14 PDT
I added some nice asserts in addition to what was reviewed here, while trying to explore the other asserts that EWS was seeing. Yay asserts!
Brady Eidson
Comment 15 2016-05-19 15:57:36 PDT
http://trac.webkit.org/changeset/201195
Note You need to log in before you can comment on or make changes to this bug.