Pref service: add persistent pref store frontend and backend.

Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
Review-Url: https://codereview.chromium.org/2743563003
Cr-Commit-Position: refs/heads/master@{#456306}
Committed: https://chromium.googlesource.com/chromium/src/+/564fe924388fd462939f76cb1894dbface5eef71

[Sam McNally, 2017-03-09 07:14:54]

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

[commit-bot: I haz the power, 2017-03-09 07:15:07]

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

[Sam McNally, 2017-03-09 07:20:59]

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

[commit-bot: I haz the power, 2017-03-09 07:21:16]

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

[Sam McNally, 2017-03-09 07:30:22]

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

[commit-bot: I haz the power, 2017-03-09 07:30:25]

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-03-09 08:28:15]

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

[commit-bot: I haz the power, 2017-03-09 08:28:16]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2017-03-09 23:30:06]

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

[commit-bot: I haz the power, 2017-03-09 23:30:37]

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

[Sam McNally, 2017-03-10 00:23:50]

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

[Sam McNally, 2017-03-10 00:23:59]

Patchset #5 (id:80001) has been deleted

[Sam McNally, 2017-03-10 00:23:59]

Patchset #4 (id:60001) has been deleted

[Sam McNally, 2017-03-10 00:24:02]

Patchset #3 (id:40001) has been deleted

[Sam McNally, 2017-03-10 00:24:04]

Patchset #2 (id:20001) has been deleted

[Sam McNally, 2017-03-10 00:24:07]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2017-03-10 00:24:29]

Patchset #1 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-03-10 00:24:30]

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

[Sam McNally, 2017-03-10 00:25:19]

sammc@chromium.org changed reviewers:
+ tibell@chromium.org

[commit-bot: I haz the power, 2017-03-10 01:25:56]

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

[commit-bot: I haz the power, 2017-03-10 01:25:56]

Dry run: This issue passed the CQ dry run.

[tibell, 2017-03-10 02:18:27]

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
File services/preferences/public/cpp/persistent_pref_store_client.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
services/preferences/public/cpp/persistent_pref_store_client.cc:89:
pref_store_->CommitPendingWrite();
Perhaps DCHECK(pref_store_) here and elsewhere.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
File services/preferences/public/cpp/pref_store_client_mixin.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
services/preferences/public/cpp/pref_store_client_mixin.cc:42:
DCHECK(initialized_);
Also DCHECK cached_prefs_ here and elsewhere.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
File services/preferences/user_prefs.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.cc:63: bool call_in_progress_ = false;
Is this used to prohibit some kind of feedback loop between writing a value and
then immediately see an observer notification for that the value changed? Could
you please document this field (in particular why it exists)?

Perhaps

// If true then a write is in progress and any update notifications should be
ignored, as those updates would originate from ourselves.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
File services/preferences/user_prefs.h (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.h:22: class UserPrefs : public
mojom::PersistentPrefStoreConnector,
Sorry to be a pain, but I think it would be nice to have a consistent naming
scheme for the "client" and "server" part of our pref stores. Right now we have
*Client and *Impl, respectively. Could this be called PersistentPrefStoreImpl
for consistency?

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.h:58: std::unordered_map<Connection*,
std::unique_ptr<Connection>> connections_;
Could this just be set<std::unique_ptr<Connection>>?

[Sam McNally, 2017-03-10 02:30:10]

Description was changed from

==========
Pref service: add user prefs frontend and backend.

Key parts:
- UserPrefs: A mojo adapter that provides access from multiple clients
  to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the UserPrefs backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
==========

to

==========
Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
==========

[Sam McNally, 2017-03-10 02:30:34]

Description was changed from

==========
Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
==========

to

==========
Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
==========

[Sam McNally, 2017-03-10 02:43:25]

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
File services/preferences/public/cpp/persistent_pref_store_client.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
services/preferences/public/cpp/persistent_pref_store_client.cc:89:
pref_store_->CommitPendingWrite();
On 2017/03/10 02:18:27, tibell wrote:
> Perhaps DCHECK(pref_store_) here and elsewhere.

Done.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
File services/preferences/public/cpp/pref_store_client_mixin.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/p...
services/preferences/public/cpp/pref_store_client_mixin.cc:42:
DCHECK(initialized_);
On 2017/03/10 02:18:27, tibell wrote:
> Also DCHECK cached_prefs_ here and elsewhere.

Done.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
File services/preferences/user_prefs.cc (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.cc:63: bool call_in_progress_ = false;
On 2017/03/10 02:18:27, tibell wrote:
> Is this used to prohibit some kind of feedback loop between writing a value
and
> then immediately see an observer notification for that the value changed?
Could
> you please document this field (in particular why it exists)?
> 
> Perhaps
> 
> // If true then a write is in progress and any update notifications should be
> ignored, as those updates would originate from ourselves.

Done.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
File services/preferences/user_prefs.h (right):

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.h:22: class UserPrefs : public
mojom::PersistentPrefStoreConnector,
On 2017/03/10 02:18:27, tibell wrote:
> Sorry to be a pain, but I think it would be nice to have a consistent naming
> scheme for the "client" and "server" part of our pref stores. Right now we
have
> *Client and *Impl, respectively. Could this be called PersistentPrefStoreImpl
> for consistency?

Done.

https://codereview.chromium.org/2743563003/diff/100001/services/preferences/u...
services/preferences/user_prefs.h:58: std::unordered_map<Connection*,
std::unique_ptr<Connection>> connections_;
On 2017/03/10 02:18:27, tibell wrote:
> Could this just be set<std::unique_ptr<Connection>>?

Lookup by unique_ptr is inefficient.

[Sam McNally, 2017-03-10 02:44:26]

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

[commit-bot: I haz the power, 2017-03-10 02:45:11]

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

[Sam McNally, 2017-03-10 02:57:47]

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

[commit-bot: I haz the power, 2017-03-10 02:58:09]

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

[Sam McNally, 2017-03-10 03:30:21]

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

[commit-bot: I haz the power, 2017-03-10 03:30:41]

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

[tibell, 2017-03-10 04:14:01]

lgtm

[Sam McNally, 2017-03-10 04:25:21]

sammc@chromium.org changed reviewers:
+ bauerb@chromium.org, mbarbella@chromium.org

[Sam McNally, 2017-03-10 04:25:22]

+bauerb for the DEPS change

+mbarbella for mojom, typemap and traits

[commit-bot: I haz the power, 2017-03-10 05:28:08]

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

[commit-bot: I haz the power, 2017-03-10 05:28:09]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-03-10 13:03:34]

DEPS LGTM.

[Martin Barbella, 2017-03-10 22:14:41]

mojom/traits lgtm

[Sam McNally, 2017-03-12 22:07:40]

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

[commit-bot: I haz the power, 2017-03-12 22:08:04]

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-03-12 23:09:27]

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

[commit-bot: I haz the power, 2017-03-12 23:09:28]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2017-03-12 23:24:25]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2017-03-12 23:24:46]

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-03-12 23:28:43]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1489361065148570,
"parent_rev": "06df7d33871184bc92190c5d0071e0b6a9eee58e", "commit_rev":
"564fe924388fd462939f76cb1894dbface5eef71"}

[commit-bot: I haz the power, 2017-03-12 23:29:25]

Description was changed from

==========
Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988
==========

to

==========
Pref service: add persistent pref store frontend and backend.

Key parts:
- PersistentPrefStoreImpl: A mojo adapter that provides access from
  multiple clients to a single PersistentPrefStore.

- PersistentPrefStoreClient: This bridges the PersistentPrefStore
  interface and a mojo connection to the PersistentPrefStoreImpl
  backend.

- PrefStoreClientMixin: This is a version of PrefStoreClient modified
  to be a mixin so it can be reused by PersistentPrefStoreClient to
  implement the PrefStore part of the PersistentPrefStore interface.

- PrefStoreClient: The functionality that was previously here mostly
  moved into PrefStoreClientMixin, which this now uses.

BUG=654988

Review-Url: https://codereview.chromium.org/2743563003
Cr-Commit-Position: refs/heads/master@{#456306}
Committed:
https://chromium.googlesource.com/chromium/src/+/564fe924388fd462939f76cb1894...
==========

[commit-bot: I haz the power, 2017-03-12 23:29:27]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/564fe924388fd462939f76cb1894...