Integrate mojo localstorage implementation with DOMStorageContext API.

Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194
Committed: https://crrev.com/0abdba0cfbf16e9d05fe372a9b7674286b801008
Cr-Commit-Position: refs/heads/master@{#441493}

[Marijn Kruisselbrink, 2017-01-03 18:43:46]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-03 18:43:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-03 19:43:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-03 19:43:20]

Dry run: This issue passed the CQ dry run.

[Marijn Kruisselbrink, 2017-01-04 00:46:30]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 00:46:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2017-01-04 00:52:48]

Description was changed from

==========
WIP: integrate mojo localstorage implementation with DOMStorageContextWrapper.

BUG=586194
==========

to

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

BUG=586194
==========

[Marijn Kruisselbrink, 2017-01-04 00:53:53]

Description was changed from

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

BUG=586194
==========

to

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194
==========

[Marijn Kruisselbrink, 2017-01-04 00:54:12]

mek@chromium.org changed reviewers:
+ michaeln@chromium.org

[commit-bot: I haz the power, 2017-01-04 01:46:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 01:46:04]

Dry run: This issue passed the CQ dry run.

[michaeln, 2017-01-04 21:14:44]

https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.cc (right):

https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.cc:307:
context_->task_runner()->PostShutdownBlockingTask(
Not sure you want to handle this in the current CL, but "what if" data has been
migrated to the new backend but has not yet been deleted from the old backend? I
think we'd get two 'info' entries for the same origin in that case.

[Marijn Kruisselbrink, 2017-01-04 21:17:37]

https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.cc (right):

https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.cc:307:
context_->task_runner()->PostShutdownBlockingTask(
On 2017/01/04 at 21:14:44, michaeln wrote:
> Not sure you want to handle this in the current CL, but "what if" data has
been migrated to the new backend but has not yet been deleted from the old
backend? I think we'd get two 'info' entries for the same origin in that case.

Yeah, that's true. And that results in two "localstorage" bubbles in the
corresponding UI. While maybe slightly confusing, I don't think that's a real
issue though, as it does accurately reflect what is going on.

And yes, figuring out what to do about data migration (and when to delete the
sqlite data) is an issue I'd rather address separately (and I didn't want to
complicate the code here by trying to merge the two vectors somehow).

[michaeln, 2017-01-04 21:34:52]

dealing with that later lgtm

On 2017/01/04 21:17:37, Marijn Kruisselbrink wrote:
>
https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
> File content/browser/dom_storage/dom_storage_context_wrapper.cc (right):
> 
>
https://codereview.chromium.org/2611743002/diff/20001/content/browser/dom_sto...
> content/browser/dom_storage/dom_storage_context_wrapper.cc:307:
> context_->task_runner()->PostShutdownBlockingTask(
> On 2017/01/04 at 21:14:44, michaeln wrote:
> > Not sure you want to handle this in the current CL, but "what if" data has
> been migrated to the new backend but has not yet been deleted from the old
> backend? I think we'd get two 'info' entries for the same origin in that case.
> 
> Yeah, that's true. And that results in two "localstorage" bubbles in the
> corresponding UI. While maybe slightly confusing, I don't think that's a real
> issue though, as it does accurately reflect what is going on.
>
> And yes, figuring out what to do about data migration (and when to delete the
> sqlite data) is an issue I'd rather address separately (and I didn't want to
> complicate the code here by trying to merge the two vectors somehow).

maybe we take getalltime as an opportunity to delete the state sqldb instead of
dbl reporting it?

[Marijn Kruisselbrink, 2017-01-04 22:26:37]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2017-01-04 22:27:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 22:33:38]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483568797053740,
"parent_rev": "13d8e83711fad8897d1ed5318a89e33410f75f7a", "commit_rev":
"d1fa04dcd28c8eea70627c623b5133fcb082a022"}

[commit-bot: I haz the power, 2017-01-04 22:34:11]

Description was changed from

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194
==========

to

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194

Review-Url: https://codereview.chromium.org/2611743002
==========

[commit-bot: I haz the power, 2017-01-04 22:34:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2017-01-04 22:35:49]

Description was changed from

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194

Review-Url: https://codereview.chromium.org/2611743002
==========

to

==========
Integrate mojo localstorage implementation with DOMStorageContext API.

Calls out to the mojo localstorage implementation for querying and deleting
local storage.

Also avoids initializing the mojo implementation unless it is actually
enabled.

BUG=586194

Committed: https://crrev.com/0abdba0cfbf16e9d05fe372a9b7674286b801008
Cr-Commit-Position: refs/heads/master@{#441493}
==========

[commit-bot: I haz the power, 2017-01-04 22:35:50]

Patchset 2 (id:??) landed as
https://crrev.com/0abdba0cfbf16e9d05fe372a9b7674286b801008
Cr-Commit-Position: refs/heads/master@{#441493}