Register Physical Web preference in Chrome's local state

Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
Committed: https://crrev.com/90ef657480c7abc793b2358ddb564af90a35f763
Cr-Commit-Position: refs/heads/master@{#436441}

[mattreynolds, 2016-11-11 19:51:13]

Description was changed from

==========
Register Physical Web preference in Chrome's local state

BUG=630769
==========

to

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preferences for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

[mattreynolds, 2016-11-11 19:51:27]

mattreynolds@chromium.org changed reviewers:
+ sdefresne@chromium.org

[mattreynolds, 2016-11-11 19:51:27]

Hi Sylvain, PTAL

[mattreynolds, 2016-11-11 21:35:07]

Description was changed from

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preferences for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

to

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

[sdefresne, 2016-11-14 16:13:35]

https://codereview.chromium.org/2494193002/diff/1/ios/chrome/browser/physical...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/1/ios/chrome/browser/physical...
ios/chrome/browser/physical_web/physical_web_local_state.h:17: +
(void)registerLocalState:(PrefRegistrySimple*)registry;
Can you change this to a free function? I don't think there is need to have a
class just for the purpose of a class method.

[mattreynolds, 2016-11-14 22:02:52]

https://codereview.chromium.org/2494193002/diff/1/ios/chrome/browser/physical...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/1/ios/chrome/browser/physical...
ios/chrome/browser/physical_web/physical_web_local_state.h:17: +
(void)registerLocalState:(PrefRegistrySimple*)registry;
On 2016/11/14 16:13:34, sdefresne wrote:
> Can you change this to a free function? I don't think there is need to have a
> class just for the purpose of a class method.

Done.

[sdefresne, 2016-11-16 18:00:09]

lgtm once comments have been adressed

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.h:8: #import
<Foundation/Foundation.h>
Remove.

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.mm (right):

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.mm:1: // Copyright 2016
The Chromium Authors. All rights reserved.
I think this can be changed to be a pure C++ file (i.e. named
physical_web_local_state.cc) once you remove the #import
<Foundation/Foundation.h> from the header.

[mattreynolds, 2016-11-16 18:41:57]

Thanks Sylvain!

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.h:8: #import
<Foundation/Foundation.h>
On 2016/11/16 18:00:09, sdefresne wrote:
> Remove.

Done.

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.mm (right):

https://codereview.chromium.org/2494193002/diff/20001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.mm:1: // Copyright 2016
The Chromium Authors. All rights reserved.
On 2016/11/16 18:00:09, sdefresne wrote:
> I think this can be changed to be a pure C++ file (i.e. named
> physical_web_local_state.cc) once you remove the #import
> <Foundation/Foundation.h> from the header.

Done.

[rohitrao (ping after 24h), 2016-11-16 21:29:53]

rohitrao@chromium.org changed reviewers:
+ rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-11-16 21:29:54]

https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.h:13: void
RegisterPrefs(PrefRegistrySimple* registry);
I just CQed a change that adds a file named physical_web_prefs_registration.h. 
Could you move this function into that file (and perhaps rename it to
RegisterPhysicalWebLocalStatePrefs()?)

Sylvain, I've been omitting the namespace.  Do you prefer to have it or to omit
it?

[sdefresne, 2016-11-17 09:50:14]

On 2016/11/16 21:29:54, rohitrao wrote:
>
https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
> File ios/chrome/browser/physical_web/physical_web_local_state.h (right):
> 
>
https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
> ios/chrome/browser/physical_web/physical_web_local_state.h:13: void
> RegisterPrefs(PrefRegistrySimple* registry);
> I just CQed a change that adds a file named physical_web_prefs_registration.h.

> Could you move this function into that file (and perhaps rename it to
> RegisterPhysicalWebLocalStatePrefs()?)
> 
> Sylvain, I've been omitting the namespace.  Do you prefer to have it or to
omit
> it?

RegisterPhysicalWebLocalStatePrefs without the namespace looks good.

[mattreynolds, 2016-11-28 20:18:54]

https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
File ios/chrome/browser/physical_web/physical_web_local_state.h (right):

https://codereview.chromium.org/2494193002/diff/40001/ios/chrome/browser/phys...
ios/chrome/browser/physical_web/physical_web_local_state.h:13: void
RegisterPrefs(PrefRegistrySimple* registry);
On 2016/11/16 21:29:53, rohitrao wrote:
> I just CQed a change that adds a file named physical_web_prefs_registration.h.

> Could you move this function into that file (and perhaps rename it to
> RegisterPhysicalWebLocalStatePrefs()?)

Done.

[rohitrao (ping after 24h), 2016-12-01 20:27:50]

lgtm

[mattreynolds, 2016-12-05 21:39:51]

The CQ bit was checked by mattreynolds@chromium.org

[mattreynolds, 2016-12-05 21:39:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2494193002/#ps60001
(title: "move to physical_web_prefs_registration")

[commit-bot: I haz the power, 2016-12-05 21:40:37]

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, 2016-12-05 23:07:21]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480973991142120,
"parent_rev": "042d426c7e0ffc8b3dc85b7aff64ffbee6d29d36", "commit_rev":
"12c3f69f23f5d448d52c4953b2961c3423a36914"}

[commit-bot: I haz the power, 2016-12-05 23:07:51]

Description was changed from

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

to

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

[commit-bot: I haz the power, 2016-12-05 23:07:52]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-05 23:10:24]

Description was changed from

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769
==========

to

==========
Register Physical Web preference in Chrome's local state

This change adds a new local state preference for the Physical Web.
The new preference is intended to replace the existing browser state
preference, which will be removed in a future CL.

BUG=630769

Committed: https://crrev.com/90ef657480c7abc793b2358ddb564af90a35f763
Cr-Commit-Position: refs/heads/master@{#436441}
==========

[commit-bot: I haz the power, 2016-12-05 23:10:25]

Patchset 4 (id:??) landed as
https://crrev.com/90ef657480c7abc793b2358ddb564af90a35f763
Cr-Commit-Position: refs/heads/master@{#436441}