Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo) DOMStorageCachedArea, since there could be several LocalStorageArea objects in a renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea objects in the renderer.

BUG=586194
Committed: https://crrev.com/5af3c81e40aebf43387314e2bc7f18aca80eb178
Cr-Commit-Position: refs/heads/master@{#378818}

[jam, 2016-02-26 21:20:01]

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

[michaeln, 2016-02-26 22:21:44]

https://codereview.chromium.org/1745603002/diff/1/content/renderer/dom_storag...
File content/renderer/dom_storage/local_storage_area.h (right):

https://codereview.chromium.org/1745603002/diff/1/content/renderer/dom_storag...
content/renderer/dom_storage/local_storage_area.h:21: class LocalStorageArea :
public blink::WebStorageArea,
The [Sycn] attribute looks good, i'm not sure of the code to use it just yet. 

In the current system there are two classes
- WebStorageAreaImpl
- DOMStorageCachedArea
There may be many instances of WebStorageAreaImpl for a origin but only one
instance of a DOMStorageCachedArea. The cached area is a refcounted object
shared by the areaimpls for the origin. That object maintains the cache.

In the new system, I think we want to retain that kind of split where multiple
WebStorageAreas utilize the same cache. As coded this class is starting to look
like a merger of the two.

[michaeln, 2016-02-26 22:31:30]

also, about DOMStorageCachedArea::Prime() and ignore_all_mutations_ = true, we
do need that, the comment explains why.

  // The LoadArea method is actually synchronous, but we have to
  // wait for an asyncly delivered message to know when incoming
  // mutation events should be applied. Our valuemap is plucked
  // from ipc stream out of order, mutations in front if it need
  // to be ignored.

  // Ignore all mutations until OnLoadComplete time.

The method is sync and the function returns out-of-order. It returns in advance
of all msgs that were in flight prior to the loadarea response being sent. We
don't want to apply those changes because they're already reflected in the set
of values we loaded.

[jam, 2016-02-29 23:47:47]

On 2016/02/26 22:31:30, michaeln wrote:
> also, about DOMStorageCachedArea::Prime() and ignore_all_mutations_ = true, we
> do need that, the comment explains why.
> 
>   // The LoadArea method is actually synchronous, but we have to
>   // wait for an asyncly delivered message to know when incoming
>   // mutation events should be applied. Our valuemap is plucked
>   // from ipc stream out of order, mutations in front if it need
>   // to be ignored.
> 
>   // Ignore all mutations until OnLoadComplete time.
> 
> The method is sync and the function returns out-of-order. It returns in
advance
> of all msgs that were in flight prior to the loadarea response being sent. We
> don't want to apply those changes because they're already reflected in the set
> of values we loaded.

Thanks, I had missed that. Per discussion, the patch now passes in the observer
to GetAll/DeleteAll instead, to avoid having each consumer of LevelDBWrapper
have to keep this logic.

https://codereview.chromium.org/1745603002/diff/1/content/renderer/dom_storag...
File content/renderer/dom_storage/local_storage_area.h (right):

https://codereview.chromium.org/1745603002/diff/1/content/renderer/dom_storag...
content/renderer/dom_storage/local_storage_area.h:21: class LocalStorageArea :
public blink::WebStorageArea,
On 2016/02/26 22:21:44, michaeln wrote:
> The [Sycn] attribute looks good, i'm not sure of the code to use it just yet. 
> 
> In the current system there are two classes
> - WebStorageAreaImpl
> - DOMStorageCachedArea
> There may be many instances of WebStorageAreaImpl for a origin but only one
> instance of a DOMStorageCachedArea. The cached area is a refcounted object
> shared by the areaimpls for the origin. That object maintains the cache.
> 
> In the new system, I think we want to retain that kind of split where multiple
> WebStorageAreas utilize the same cache. As coded this class is starting to
look
> like a merger of the two.

ah, I missed that there are multiple blink::WebStorageArea implementations in
content/renderer for the same origin. For some reason I thought this was handled
in Blink (I also missed CachedAreaHolder). Thanks for bringing this up, redone.

[jam, 2016-02-29 23:49:11]

Description was changed from

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

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

to

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo)
DOMStorageCachedArea, since there could be several LocalStorageArea objects in a
renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea
objects in the renderer.

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

[michaeln, 2016-03-01 01:38:48]

nice, i have some questions/comments about modularization and a couple naming
nits

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.h:82:
std::map<std::string, scoped_ptr<LevelDBWrapperImpl>> level_db_wrappers_;
using url::Origin as the key type might be nice to avoid problems with different
strings being used to represent the same origin

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
File content/browser/leveldb_wrapper_impl.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
content/browser/leveldb_wrapper_impl.h:23: class LevelDBWrapperImpl : public
LevelDBWrapper {
Would it make sense for this class to be in the components/leveldb directory?
The mojom class and observer interfaces are defined there. I think it'd be good
to modularize as much as possible.

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_area.cc:16: :
cached_area_(cached_area) {
would std::move(cached_area) make a difference here or is that already what the
compiler generates given these sources?

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_area.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_area.h:36:
scoped_refptr<LocalStorageCachedArea> cached_area_;
hurray for smartptrs!

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_cached_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.cc:57: binding_.Close();
Nice, this is a much nicer api for ignoring mutation events. The caller still
has to know to do this but the api is a lot more instructive.

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_cached_area.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:24: // other processes
are applied to the cache via LevelDBObserver callbacks.
With modularity in mind, would it make sense for this class to be defined in
components/leveldb? And to be renamed to something more
leveldbwrapperclient'ish.

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:29:
StoragePartitionService* storage_partition_service,
Questions about unittests? Is it easy to mock out a LevelDBWrapperPtr leveldb_?
Also, with that in mind, would it make sense to pass a LevelDBWrapperPtr into
this ctor instead of the StoragePartitionService*. Have the caller be
responsible for opening the leveldbwrapper for the origin so a unittest could
pass in a mock?

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:43: url::Origin
origin() { return origin_; }
maybe make this a const ref return value

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_cached_areas.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_areas.h:28:
scoped_refptr<LocalStorageCachedArea> GetLocalStorageCachedArea(
naming nit: method names might be shorter are more clear

GetOrCreateCachedArea()
CachedAreaDestructed()

[jam, 2016-03-01 06:50:54]

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.h:82:
std::map<std::string, scoped_ptr<LevelDBWrapperImpl>> level_db_wrappers_;
On 2016/03/01 01:38:47, michaeln wrote:
> using url::Origin as the key type might be nice to avoid problems with
different
> strings being used to represent the same origin

mojo doesn't have a url type, so we're getting a mojo::String on the receiver
side.

i'm not familiar with the problem you're mentioning: if we have url::Origin in
the renderer and we call .Serialize() to get a string, how do we get different
strings for the same origin?

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
File content/browser/leveldb_wrapper_impl.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
content/browser/leveldb_wrapper_impl.h:23: class LevelDBWrapperImpl : public
LevelDBWrapper {
On 2016/03/01 01:38:47, michaeln wrote:
> Would it make sense for this class to be in the components/leveldb directory?
> The mojom class and observer interfaces are defined there. I think it'd be
good
> to modularize as much as possible.

Based on discussions with Elliot, we had agreed that it's better to keep
components/leveldb just strictly a mojom wrapper around leveldb.

i think a lot will depend on who else uses this. if it's just web platform
features, this might just stay along with the profile mojoms

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_area.cc:16: :
cached_area_(cached_area) {
On 2016/03/01 01:38:48, michaeln wrote:
> would std::move(cached_area) make a difference here or is that already what
the
> compiler generates given these sources?

I haven't seen that as the standard, i think it's up to each developer per the
recent thread on chromium-dev

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_cached_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.cc:57: binding_.Close();
On 2016/03/01 01:38:48, michaeln wrote:
> Nice, this is a much nicer api for ignoring mutation events. The caller still
> has to know to do this but the api is a lot more instructive.

fyi if the caller forgets to do this, they will get a dcheck because it's
already bound.

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_cached_area.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:24: // other processes
are applied to the cache via LevelDBObserver callbacks.
On 2016/03/01 01:38:48, michaeln wrote:
> With modularity in mind, would it make sense for this class to be defined in
> components/leveldb? And to be renamed to something more
> leveldbwrapperclient'ish.

it's a bit hard to see since it's still a stub, but this will be local storage
specific. i.e. on mutation events it will call
blink::WebStorageEventDispatcher::dispatchLocalStorageEvent.

indexeddb will do things differently, at least it seems that way so far, but
we'll see when we have other consumers of leveldb whether there's much logic
here to share and we can reconsider then

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:29:
StoragePartitionService* storage_partition_service,
On 2016/03/01 01:38:48, michaeln wrote:
> Questions about unittests? Is it easy to mock out a LevelDBWrapperPtr
leveldb_?
> Also, with that in mind, would it make sense to pass a LevelDBWrapperPtr into
> this ctor instead of the StoragePartitionService*. Have the caller be
> responsible for opening the leveldbwrapper for the origin so a unittest could
> pass in a mock?

Re mocking LevelDBWrapperPtr, yep that's easy with mojo (i.e. just have a new
implementation of LevelDBWrapper interface).

Re unittests, yes your suggestion is definitely possible. At this point, it's
not clear if this class will have much logic that makes it worth testing, or
it'll be mostly just pass through calls to LevelDBWrapperPtr. If that's the
case, then the layout tests should be enough IMO.

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_cached_area.h:43: url::Origin
origin() { return origin_; }
On 2016/03/01 01:38:48, michaeln wrote:
> maybe make this a const ref return value

Done.

[michaeln, 2016-03-01 21:38:44]

lgtm

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.h:82:
std::map<std::string, scoped_ptr<LevelDBWrapperImpl>> level_db_wrappers_;
On 2016/03/01 06:50:53, jam wrote:
> On 2016/03/01 01:38:47, michaeln wrote:
> > using url::Origin as the key type might be nice to avoid problems with
> different
> > strings being used to represent the same origin
> 
> mojo doesn't have a url type, so we're getting a mojo::String on the receiver
> side.
> 
> i'm not familiar with the problem you're mentioning: if we have url::Origin in
> the renderer and we call .Serialize() to get a string, how do we get different
> strings for the same origin?

There are other consumers that need to ask about how much 'originA' is using or
to tell us to delete everytihng for 'originB'. Those consumers are not in blink
or the renderer. Does http://foo:80/ == http://foo. I'd like to eliminate any
ambiguity about the details of the key. If not in this CL, i'll eventually want
to do that.

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
File content/browser/leveldb_wrapper_impl.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/leveldb...
content/browser/leveldb_wrapper_impl.h:23: class LevelDBWrapperImpl : public
LevelDBWrapper {
On 2016/03/01 06:50:53, jam wrote:
> On 2016/03/01 01:38:47, michaeln wrote:
> > Would it make sense for this class to be in the components/leveldb
directory?
> > The mojom class and observer interfaces are defined there. I think it'd be
> good
> > to modularize as much as possible.
> 
> Based on discussions with Elliot, we had agreed that it's better to keep
> components/leveldb just strictly a mojom wrapper around leveldb.
> 
> i think a lot will depend on who else uses this. if it's just web platform
> features, this might just stay along with the profile mojoms

sg

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_area.cc:16: :
cached_area_(cached_area) {
On 2016/03/01 06:50:53, jam wrote:
> On 2016/03/01 01:38:48, michaeln wrote:
> > would std::move(cached_area) make a difference here or is that already what
> the
> > compiler generates given these sources?
> 
> I haven't seen that as the standard, i think it's up to each developer per the
> recent thread on chromium-dev

It's a practical question, do you know if as coded it will churn the refcount or
move the refcount? I'm guessing it'll do an extra add/dec ref but am not sure
about that. Everything else being being equal, it is a little better to not
churn.

[michaeln, 2016-03-01 22:26:14]

https://codereview.chromium.org/1745603002/diff/60001/content/browser/leveldb...
File content/browser/leveldb_wrapper_impl.cc (right):

https://codereview.chromium.org/1745603002/diff/60001/content/browser/leveldb...
content/browser/leveldb_wrapper_impl.cc:23: no_bindings_callback_.Run();
also, this call looks fishy, looks like we can callback more than once?

[jam, 2016-03-02 06:21:20]

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_context_wrapper.h (right):

https://codereview.chromium.org/1745603002/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_context_wrapper.h:82:
std::map<std::string, scoped_ptr<LevelDBWrapperImpl>> level_db_wrappers_;
On 2016/03/01 21:38:44, michaeln wrote:
> On 2016/03/01 06:50:53, jam wrote:
> > On 2016/03/01 01:38:47, michaeln wrote:
> > > using url::Origin as the key type might be nice to avoid problems with
> > different
> > > strings being used to represent the same origin
> > 
> > mojo doesn't have a url type, so we're getting a mojo::String on the
receiver
> > side.
> > 
> > i'm not familiar with the problem you're mentioning: if we have url::Origin
in
> > the renderer and we call .Serialize() to get a string, how do we get
different
> > strings for the same origin?
> 
> There are other consumers that need to ask about how much 'originA' is using
or
> to tell us to delete everytihng for 'originB'. Those consumers are not in
blink
> or the renderer. Does http://foo:80/ == http://foo. I'd like to eliminate any
> ambiguity about the details of the key. If not in this CL, i'll eventually
want
> to do that.

got it, now I understand this better thanks.

i'll look into sending the url::Origin in mojo ipc in a followup

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_area.cc (right):

https://codereview.chromium.org/1745603002/diff/40001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_area.cc:16: :
cached_area_(cached_area) {
On 2016/03/01 21:38:44, michaeln wrote:
> On 2016/03/01 06:50:53, jam wrote:
> > On 2016/03/01 01:38:48, michaeln wrote:
> > > would std::move(cached_area) make a difference here or is that already
what
> > the
> > > compiler generates given these sources?
> > 
> > I haven't seen that as the standard, i think it's up to each developer per
the
> > recent thread on chromium-dev
> 
> It's a practical question, do you know if as coded it will churn the refcount
or
> move the refcount? I'm guessing it'll do an extra add/dec ref but am not sure
> about that. Everything else being being equal, it is a little better to not
> churn.

yep it'll do an extra refcount. thinking more about it, i agree. added std::move

https://codereview.chromium.org/1745603002/diff/60001/content/browser/leveldb...
File content/browser/leveldb_wrapper_impl.cc (right):

https://codereview.chromium.org/1745603002/diff/60001/content/browser/leveldb...
content/browser/leveldb_wrapper_impl.cc:23: no_bindings_callback_.Run();
On 2016/03/01 22:26:14, michaeln wrote:
> also, this call looks fishy, looks like we can callback more than once?

ah yes, fixed thanks.

[jam, 2016-03-02 16:50:49]

The CQ bit was checked by jam@chromium.org

[jam, 2016-03-02 16:50:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/1745603002/#ps80001
(title: "review comments")

[commit-bot: I haz the power, 2016-03-02 16:51:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1745603002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1745603002/80001

[commit-bot: I haz the power, 2016-03-02 17:01:53]

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

[commit-bot: I haz the power, 2016-03-02 17:01:55]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jam, 2016-03-02 17:05:31]

jam@chromium.org changed reviewers:
+ dcheng@chromium.org

[jam, 2016-03-02 17:05:32]

+dcheng for mojom changes

[dcheng, 2016-03-02 18:40:23]

https://codereview.chromium.org/1745603002/diff/80001/content/common/leveldb_...
File content/common/leveldb_wrapper.mojom (right):

https://codereview.chromium.org/1745603002/diff/80001/content/common/leveldb_...
content/common/leveldb_wrapper.mojom:10: // The reason this is a parameter to
DeleteAll and GetAll above, instead of
Below?

https://codereview.chromium.org/1745603002/diff/80001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_namespace.h (right):

https://codereview.chromium.org/1745603002/diff/80001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_namespace.h:27:
LocalStorageCachedAreas* local_storage_cached_areas_;
Nit: maybe LocalStorageCachedAreas* const if it doesn't change for the lifetime
of this object?

[jam, 2016-03-02 19:08:20]

https://codereview.chromium.org/1745603002/diff/80001/content/common/leveldb_...
File content/common/leveldb_wrapper.mojom (right):

https://codereview.chromium.org/1745603002/diff/80001/content/common/leveldb_...
content/common/leveldb_wrapper.mojom:10: // The reason this is a parameter to
DeleteAll and GetAll above, instead of
On 2016/03/02 18:40:23, dcheng wrote:
> Below?

Done.

https://codereview.chromium.org/1745603002/diff/80001/content/renderer/dom_st...
File content/renderer/dom_storage/local_storage_namespace.h (right):

https://codereview.chromium.org/1745603002/diff/80001/content/renderer/dom_st...
content/renderer/dom_storage/local_storage_namespace.h:27:
LocalStorageCachedAreas* local_storage_cached_areas_;
On 2016/03/02 18:40:23, dcheng wrote:
> Nit: maybe LocalStorageCachedAreas* const if it doesn't change for the
lifetime
> of this object?

Done.

[dcheng, 2016-03-02 19:39:20]

lgtm

[jam, 2016-03-02 19:59:25]

The CQ bit was checked by jam@chromium.org

[jam, 2016-03-02 19:59:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/1745603002/#ps100001
(title: "review comment")

[commit-bot: I haz the power, 2016-03-02 20:00:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1745603002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1745603002/100001

[commit-bot: I haz the power, 2016-03-02 20:35:07]

Description was changed from

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo)
DOMStorageCachedArea, since there could be several LocalStorageArea objects in a
renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea
objects in the renderer.

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

to

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo)
DOMStorageCachedArea, since there could be several LocalStorageArea objects in a
renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea
objects in the renderer.

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

[commit-bot: I haz the power, 2016-03-02 20:35:08]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-02 20:36:07]

Description was changed from

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo)
DOMStorageCachedArea, since there could be several LocalStorageArea objects in a
renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea
objects in the renderer.

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

to

==========
Switch LevelDBWrapper::GetAll to use the new Mojo sync IPC mechanism.

Also create LocalStorageCachedArea to mirror the (non-mojo)
DOMStorageCachedArea, since there could be several LocalStorageArea objects in a
renderer for the same origin and we want just one cache.

LocalStorageCachedAreas is a container for all the live LocalStorageCachedArea
objects in the renderer.

BUG=586194

Committed: https://crrev.com/5af3c81e40aebf43387314e2bc7f18aca80eb178
Cr-Commit-Position: refs/heads/master@{#378818}
==========

[commit-bot: I haz the power, 2016-03-02 20:36:09]

Patchset 6 (id:??) landed as
https://crrev.com/5af3c81e40aebf43387314e2bc7f18aca80eb178
Cr-Commit-Position: refs/heads/master@{#378818}

[jam, 2016-03-03 21:36:16]

Patchset #7 (id:120001) has been deleted