Bug 157626 - Modern IDB: Website data store management
Summary: Modern IDB: Website data store management
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on: 157757 157823 157843
Blocks: 149117
  Show dependency treegraph
 
Reported: 2016-05-12 09:31 PDT by Brady Eidson
Modified: 2016-05-19 15:57 PDT (History)
4 users (show)

See Also:


Attachments
WIP patch (23.66 KB, patch)
2016-05-13 16:35 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (18.93 KB, patch)
2016-05-18 17:08 PDT, Brady Eidson
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 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.
Comment 1 Brady Eidson 2016-05-12 09:32:13 PDT
<rdar://problem/25446044>
Comment 2 Brady Eidson 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.
Comment 3 Brady Eidson 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.
Comment 4 Brady Eidson 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
Comment 5 Brady Eidson 2016-05-17 16:09:58 PDT
This got bigger, because a lot more bookkeeping and message passing became necessary.
*sigh*

Patch incoming
Comment 6 Brady Eidson 2016-05-17 16:24:18 PDT
Never mind - I'm going to have to start splitting into multiple patches to keep it sane.
Comment 7 Brady Eidson 2016-05-18 17:08:55 PDT
Created attachment 279323 [details]
Patch
Comment 8 Alex Christensen 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.
Comment 9 Brady Eidson 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.
Comment 10 Brady Eidson 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)
Comment 11 Brady Eidson 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.
Comment 12 Brady Eidson 2016-05-19 10:03:56 PDT
Actually, it's a mysterious hashing issue, not an automatic easy fix. Ugh.
Comment 13 Brady Eidson 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
Comment 14 Brady Eidson 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!
Comment 15 Brady Eidson 2016-05-19 15:57:36 PDT
http://trac.webkit.org/changeset/201195