Upstream ChromeBrowserState sub-classes.

Upstream ChromeBrowserState sub-classes.

BUG=429756
Committed: https://crrev.com/cc7a86a5e040c3ebfa3a90d162558ab808d06988
Cr-Commit-Position: refs/heads/master@{#373568}

[sdefresne, 2016-02-04 13:37:37]

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

[sdefresne, 2016-02-04 13:37:38]

Please take a look.

[droger, 2016-02-04 13:56:14]

Only cleanup nits, optional.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/chrome_browser_state_impl.h (right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:8: #include
"base/gtest_prod_util.h"
Maybe not needed?

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:30: class
ChromeBrowserStateImpl : public ios::ChromeBrowserState {
Not related to this CL, but ChromeBrowserState should not be in the ios
namespace.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:57: explicit
ChromeBrowserStateImpl(const base::FilePath& path);
Nit: I think this can be private. No need to be protected.

Or alternately just move it to public, not sure why it is hidden.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h:26:
const std::string& GetRemovedBrowserStateGAIAId() const {
Optional: style for inlined method names.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h:31:
bool HasChangedLastUsedBrowserState() const {
Optional: style for inlined method names.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.cc
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.cc:24:
// Register on BrowserState.
Not sure what this comment means. Remove?

[droger, 2016-02-04 13:56:18]

lgtm

Only cleanup nits, optional.

[droger, 2016-02-04 13:58:54]

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.h
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.h:50:
OffTheRecordChromeBrowserStateImpl(
This should ideally be similar to the ChromeBrowserStateImpl constructor (i.e.
both private or both public).

[sdefresne, 2016-02-04 14:57:25]

+vabr: FYI, I remember you asked for an upstream TestChromeBrowserState a few
weeks ago. As promised, it is coming soon, I expect to land this CL by EOW.

[vabr (Chromium), 2016-02-04 15:01:16]

On 2016/02/04 14:57:25, sdefresne wrote:
> +vabr: FYI, I remember you asked for an upstream TestChromeBrowserState a few
> weeks ago. As promised, it is coming soon, I expect to land this CL by EOW.

Thank you, I am observing this with joy! :)

[sdefresne, 2016-02-04 15:27:41]

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/chrome_browser_state_impl.h (right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:8: #include
"base/gtest_prod_util.h"
On 2016/02/04 at 13:56:14, droger wrote:
> Maybe not needed?

Removed.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:30: class
ChromeBrowserStateImpl : public ios::ChromeBrowserState {
On 2016/02/04 at 13:56:14, droger wrote:
> Not related to this CL, but ChromeBrowserState should not be in the ios
namespace.

Ack (as ios::ChromeBrowserStateManager, ...). I think we should wait until we no
longer build anything from //chrome (not even for unit_tests) before doing that
cleanup.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_impl.h:57: explicit
ChromeBrowserStateImpl(const base::FilePath& path);
On 2016/02/04 at 13:56:14, droger wrote:
> Nit: I think this can be private. No need to be protected.
> 
> Or alternately just move it to public, not sure why it is hidden.

Made private because all ChromeBrowserStateImpl should be created through
ChromeBrowserStateManagerImpl.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h:26:
const std::string& GetRemovedBrowserStateGAIAId() const {
On 2016/02/04 at 13:56:14, droger wrote:
> Optional: style for inlined method names.

Done.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h:31:
bool HasChangedLastUsedBrowserState() const {
On 2016/02/04 at 13:56:14, droger wrote:
> Optional: style for inlined method names.

Done.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.cc
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.cc:24:
// Register on BrowserState.
On 2016/02/04 at 13:56:14, droger wrote:
> Not sure what this comment means. Remove?

Done.

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
File ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.h
(right):

https://codereview.chromium.org/1664823003/diff/1/ios/chrome/browser/browser_...
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_impl.h:50:
OffTheRecordChromeBrowserStateImpl(
On 2016/02/04 at 13:58:54, droger wrote:
> This should ideally be similar to the ChromeBrowserStateImpl constructor (i.e.
both private or both public).

Kept private.

[sdefresne, 2016-02-04 18:18:30]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-02-04 18:18:31]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org
Link to the patchset: https://codereview.chromium.org/1664823003/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-02-04 18:20:11]

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

[commit-bot: I haz the power, 2016-02-04 18:35:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-04 18:37:01]

Description was changed from

==========
Upstream ChromeBrowserState sub-classes.

BUG=429756
==========

to

==========
Upstream ChromeBrowserState sub-classes.

BUG=429756

Committed: https://crrev.com/cc7a86a5e040c3ebfa3a90d162558ab808d06988
Cr-Commit-Position: refs/heads/master@{#373568}
==========

[commit-bot: I haz the power, 2016-02-04 18:37:02]

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