Convert TrackedPreferenceValidationDelegate into a mojo interface.

Convert TrackedPreferenceValidationDelegate into a mojo interface.

Currently, the C++ interface TrackedPreferenceValidationDelegate is used
to communicate between two components, both running on the same thread.
Once prefs is servicified, this direct path will no longer be available.
In preparation for this, this CL converts this interface to be a mojo
interface.

This also removes the unused |dict_value| parameter from
OnSplitPreferenceValidation().

BUG=654988
Review-Url: https://codereview.chromium.org/2719833002
Cr-Commit-Position: refs/heads/master@{#455342}
Committed: https://chromium.googlesource.com/chromium/src/+/2c885e024b61f6f0bf1cb7b5624b2b202917c0de

[Sam McNally, 2017-02-27 02:02:56]

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

[commit-bot: I haz the power, 2017-02-27 02:03: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-02-27 02:03:21]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2017-02-27 02:19:23]

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

[commit-bot: I haz the power, 2017-02-27 02:19:42]

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-02-27 02:32:07]

Patchset #1 (id:20001) has been deleted

[Sam McNally, 2017-02-27 02:32:48]

Description was changed from

==========
Convert TrackedPreferenceValidationDelegate into a mojo interface.

Currently, the C++ interface TrackedPreferenceValidationDelegate is used
to communicate between two components, both running on the same thread.
Once prefs is servicified, this direct path will no longer be available.
In preparation for this, this CL converts this interface to be a mojo
interface.

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

to

==========
Convert TrackedPreferenceValidationDelegate into a mojo interface.

Currently, the C++ interface TrackedPreferenceValidationDelegate is used
to communicate between two components, both running on the same thread.
Once prefs is servicified, this direct path will no longer be available.
In preparation for this, this CL converts this interface to be a mojo
interface.

This also removes the unused |dict_value| parameter from
OnSplitPreferenceValidation().

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

[Sam McNally, 2017-02-27 02:33:22]

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

[commit-bot: I haz the power, 2017-02-27 02:33:39]

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-02-27 03:03:46]

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

[commit-bot: I haz the power, 2017-02-27 03:03:47]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sam McNally, 2017-02-27 03:23:31]

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

[commit-bot: I haz the power, 2017-02-27 03:23:45]

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-02-27 03:44:15]

Patchset #3 (id:80001) has been deleted

[Sam McNally, 2017-02-27 03:44:17]

Patchset #2 (id:60001) has been deleted

[Sam McNally, 2017-02-27 03:44:19]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-02-27 04:18:17]

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

[commit-bot: I haz the power, 2017-02-27 04:18:18]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2017-02-27 04:26:56]

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

[Sam McNally, 2017-03-02 01:50:40]

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

[commit-bot: I haz the power, 2017-03-02 01:51:08]

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-02 02:06:40]

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

[commit-bot: I haz the power, 2017-03-02 02:06:59]

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-02 03:45:18]

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

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

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sam McNally, 2017-03-02 04:00:08]

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

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

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-02 04:17:21]

lgtm

[Sam McNally, 2017-03-02 04:35:13]

sammc@chromium.org changed reviewers:
+ bauerb@chromium.org, dcheng@chromium.org, mattm@chromium.org,
mlerman@chromium.org, rockot@chromium.org

[Sam McNally, 2017-03-02 04:35:13]

+mlerman for //chrome/browser/profiles

+mattm for //chrome/browser/safe_browsing

+rockot for //services and //mojo

+bauerb for //components/user_prefs/tracked and //chrome/browser/prefs

+dcheng for the mojom and struct traits

[commit-bot: I haz the power, 2017-03-02 07:54:47]

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

[commit-bot: I haz the power, 2017-03-02 07:54:48]

Dry run: This issue passed the CQ dry run.

[Mike Lerman, 2017-03-02 14:37:09]

profiles LGTM

[Ken Rockot(use gerrit already), 2017-03-02 21:54:14]

lgtm

[Bernhard Bauer, 2017-03-03 12:12:12]

prefs LGTM. +gab FYI

https://codereview.chromium.org/2719833002/diff/140001/chrome/browser/safe_br...
File
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
(right):

https://codereview.chromium.org/2719833002/diff/140001/chrome/browser/safe_br...
chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc:374:
return std::unique_ptr<prefs::mojom::TrackedPreferenceValidationDelegate>();
I think you can even use nullptr here, which has an implicit conversion to
unique_ptr<>.

[mattm, 2017-03-04 03:02:03]

lgtm stamp

[Sam McNally, 2017-03-06 02:04:54]

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

[commit-bot: I haz the power, 2017-03-06 02:05:09]

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-06 03:37:41]

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

[commit-bot: I haz the power, 2017-03-06 03:37:42]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2017-03-07 01:55:32]

dcheng: ping

[dcheng, 2017-03-07 08:14:16]

mojo lgtm

I assume all the direct calls will be passing around interface pointers at some
in the future.

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/prefs/p...
File chrome/browser/prefs/profile_pref_store_manager_unittest.cc (right):

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/prefs/p...
chrome/browser/prefs/profile_pref_store_manager_unittest.cc:174:
mock_validation_delegate = base::MakeUnique<MockValidationDelegate>(
Nit: using auto here is OK.

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/services_delegate_stub.cc (right):

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/safe_br...
chrome/browser/safe_browsing/services_delegate_stub.cc:51: return
std::unique_ptr<prefs::mojom::TrackedPreferenceValidationDelegate>();
nullptr?

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
File
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc
(right):

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc:66:
return false;
Is the enum value validated before it's passed to EnumTraits?

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
File
services/preferences/public/interfaces/tracked_preference_validation_delegate.mojom
(right):

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
services/preferences/public/interfaces/tracked_preference_validation_delegate.mojom:37:
// |value| (which may be NULL) at |pref_path|. |is_personal| indicates whether
Nit: NULL -> null

[Sam McNally, 2017-03-07 23:07:20]

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

[commit-bot: I haz the power, 2017-03-07 23:08:33]

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-08 00:06:53]

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/prefs/p...
File chrome/browser/prefs/profile_pref_store_manager_unittest.cc (right):

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/prefs/p...
chrome/browser/prefs/profile_pref_store_manager_unittest.cc:174:
mock_validation_delegate = base::MakeUnique<MockValidationDelegate>(
On 2017/03/07 08:14:15, dcheng wrote:
> Nit: using auto here is OK.

Done.

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/services_delegate_stub.cc (right):

https://codereview.chromium.org/2719833002/diff/160001/chrome/browser/safe_br...
chrome/browser/safe_browsing/services_delegate_stub.cc:51: return
std::unique_ptr<prefs::mojom::TrackedPreferenceValidationDelegate>();
On 2017/03/07 08:14:15, dcheng wrote:
> nullptr?

Done.

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
File
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc
(right):

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc:66:
return false;
On 2017/03/07 08:14:15, dcheng wrote:
> Is the enum value validated before it's passed to EnumTraits?

Yes.

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
File
services/preferences/public/interfaces/tracked_preference_validation_delegate.mojom
(right):

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
services/preferences/public/interfaces/tracked_preference_validation_delegate.mojom:37:
// |value| (which may be NULL) at |pref_path|. |is_personal| indicates whether
On 2017/03/07 08:14:15, dcheng wrote:
> Nit: NULL -> null

Done.

[Sam McNally, 2017-03-08 00:18:01]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2017-03-08 00:18:02]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rockot@chromium.org, mlerman@chromium.org, tibell@chromium.org,
mattm@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2719833002/#ps200001
(title: " ")

[commit-bot: I haz the power, 2017-03-08 00:18:40]

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

[dcheng, 2017-03-08 00:22:08]

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
File
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc
(right):

https://codereview.chromium.org/2719833002/diff/160001/services/preferences/p...
services/preferences/public/cpp/tracked_preference_validation_delegate_struct_traits.cc:66:
return false;
On 2017/03/08 00:06:53, Sam McNally wrote:
> On 2017/03/07 08:14:15, dcheng wrote:
> > Is the enum value validated before it's passed to EnumTraits?
> 
> Yes.

Maybe these sorts of things should be NOTREACHED() as well? *shrug*

[commit-bot: I haz the power, 2017-03-08 02:06:09]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1488932281949960,
"parent_rev": "5d5865702469fb03a3343898dfeaec73cfa973e3", "commit_rev":
"2c885e024b61f6f0bf1cb7b5624b2b202917c0de"}

[commit-bot: I haz the power, 2017-03-08 02:07:39]

Description was changed from

==========
Convert TrackedPreferenceValidationDelegate into a mojo interface.

Currently, the C++ interface TrackedPreferenceValidationDelegate is used
to communicate between two components, both running on the same thread.
Once prefs is servicified, this direct path will no longer be available.
In preparation for this, this CL converts this interface to be a mojo
interface.

This also removes the unused |dict_value| parameter from
OnSplitPreferenceValidation().

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

to

==========
Convert TrackedPreferenceValidationDelegate into a mojo interface.

Currently, the C++ interface TrackedPreferenceValidationDelegate is used
to communicate between two components, both running on the same thread.
Once prefs is servicified, this direct path will no longer be available.
In preparation for this, this CL converts this interface to be a mojo
interface.

This also removes the unused |dict_value| parameter from
OnSplitPreferenceValidation().

BUG=654988

Review-Url: https://codereview.chromium.org/2719833002
Cr-Commit-Position: refs/heads/master@{#455342}
Committed:
https://chromium.googlesource.com/chromium/src/+/2c885e024b61f6f0bf1cb7b5624b...
==========

[commit-bot: I haz the power, 2017-03-08 02:07:41]

Committed patchset #6 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/2c885e024b61f6f0bf1cb7b5624b...