SessionStorageDatabase fix: Delete namespace keys if there are no areas.

SessionStorageDatabase fix: Delete namespace keys if there are no areas.

Before the change, the database would contain leftover "namespace-namespaceid-"
keys with no areas. This change ensures that they are not left over, and that
the existing leftover keys are scavenged. (This will delete the leftover data
for users who have it.)

BUG=175232
R=michaeln


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182259

[marja, 2013-02-11 11:06:20]

ptal

[michaeln, 2013-02-12 01:42:46]

lgtm, thnx for looking at this one marja!

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
File webkit/dom_storage/session_storage_database.cc (right):

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:245:
(*namespaces_and_origins)[current_namespace_id];
maybe map->insert(id, std::vector<GURL>()) would be more clear despite being a
few more chars of code?

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:419: // too.
I doubt this explains the massive size of the file in the bug report. Is there a
compaction step that we have to do on leveldb instances that we aren't doing?

Let's ask the idb folks?

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:434: if
(key.find(namespace_start_key) != 0) {
nit: {} not needed on the one-liner

[marja, 2013-02-12 09:10:41]

Thanks for review!

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
File webkit/dom_storage/session_storage_database.cc (right):

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:245:
(*namespaces_and_origins)[current_namespace_id];
On 2013/02/12 01:42:46, michaeln wrote:
> maybe map->insert(id, std::vector<GURL>()) would be more clear despite being a
> few more chars of code?

Done.

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:419: // too.
On 2013/02/12 01:42:46, michaeln wrote:
> I doubt this explains the massive size of the file in the bug report. Is there
a
> compaction step that we have to do on leveldb instances that we aren't doing?
> 
> Let's ask the idb folks?

Yeah, this is not the reason behind the bug report.

https://codereview.chromium.org/12224092/diff/1/webkit/dom_storage/session_st...
webkit/dom_storage/session_storage_database.cc:434: if
(key.find(namespace_start_key) != 0) {
On 2013/02/12 01:42:46, michaeln wrote:
> nit: {} not needed on the one-liner

Done.

[commit-bot: I haz the power, 2013-02-12 09:11:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/12224092/5001

[commit-bot: I haz the power, 2013-02-12 11:23:12]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-02-12 11:28:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/12224092/5001

[commit-bot: I haz the power, 2013-02-12 14:00:24]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[marja, 2013-02-13 17:09:39]

Trying with the CQ once more, maybe these were the tests which were green on
tree but broken on trybots (I heard there were some tests like this).

[commit-bot: I haz the power, 2013-02-13 17:09:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/12224092/5001

[commit-bot: I haz the power, 2013-02-13 18:25:57]

Change committed as 182259