[USS] Break NonBlockingDataTypeController into UI specific one and non-UI specific one

Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000
Committed: https://crrev.com/17c8d2773c3744b185e0b82c09f45332fc4d1928
Cr-Commit-Position: refs/heads/master@{#377754}

[Gang Wu, 2016-02-18 23:56:05]

Patchset #1 (id:1) has been deleted

[Gang Wu, 2016-02-19 17:13:13]

Patchset #1 (id:20001) has been deleted

[Gang Wu, 2016-02-23 08:00:28]

Patchset #2 (id:60001) has been deleted

[Gang Wu, 2016-02-23 18:04:32]

Description was changed from

==========
upload


first checkin

BUG=
==========

to

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.


first checkin

BUG=558000
==========

[Gang Wu, 2016-02-23 18:04:32]

gangwu@chromium.org changed reviewers:
+ maxbogue@chromium.org, pavely@chromium.org, skym@chromium.org

[Gang Wu, 2016-02-23 18:04:44]

PTAL

[skym, 2016-02-23 19:01:01]

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/device_info_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/device_info_model_type_controller.cc:26:
InitializeProcessor();
I don't understand why this is the right place to call InitializeProcessor()

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/non_blocking_data_type_controller.h:90: virtual void
OnProcessorStarted(
I don't get it. Why do the ProcessorStarted methods use one pattern, while the
InitializeProcessor methods use a different pattern. (I like the
InitializeProcessor pattern more btw)

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/non_ui_model_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/non_ui_model_type_controller.h:16: // Implementation for
Unified Sync and Storage datatypes that reside on the
Implementation for Unified Sync and Storage datatypes whose model thread is not
the UI thread.

[Gang Wu, 2016-02-23 20:22:33]

updated

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/device_info_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/device_info_model_type_controller.cc:26:
InitializeProcessor();
On 2016/02/23 19:01:00, skym wrote:
> I don't understand why this is the right place to call InitializeProcessor()

you are right, this is not right place. I should keep the comment here.

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/non_blocking_data_type_controller.h:90: virtual void
OnProcessorStarted(
On 2016/02/23 19:01:01, skym wrote:
> I don't get it. Why do the ProcessorStarted methods use one pattern, while the
> InitializeProcessor methods use a different pattern. (I like the
> InitializeProcessor pattern more btw)

The reason I use different pattern is, I want to use DCHECK(BelongsToUIThread())
and DCHECK(!BelongsToUIThread()) in ProcessorStarted, but unfortunately, I did
not do it. I think you are right, I should use same pattern.

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
File components/sync_driver/non_ui_model_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/80001/components/sync_driver/...
components/sync_driver/non_ui_model_type_controller.h:16: // Implementation for
Unified Sync and Storage datatypes that reside on the
On 2016/02/23 19:01:01, skym wrote:
> Implementation for Unified Sync and Storage datatypes whose model thread is
not
> the UI thread.

Done.

[skym, 2016-02-23 20:35:01]

lgtm

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:73: // The error
indicates whether the loading completed successfully.
Can this be private?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:89: void
OnProcessorStarted(
Can this be private?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:95: void
OnProcessorStartedOnUIThread(
Can this be private?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:100: void
InitializeProcessor();
Should this be public?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:103:
base::WeakPtr<syncer_v2::SharedModelTypeProcessor> type_processor_;
Can this be private?

[maxbogue, 2016-02-24 00:02:38]

lgtm w/ nits

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_ui_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_ui_model_type_controller.cc:24: sync_client) {}
Can this DCHECK that it is not called on the UI thread? Or do we not want to
enforce that?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_ui_model_type_controller_unittest.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_ui_model_type_controller_unittest.cc:57: void
InitialzeProcessorInTest() { InitializeProcessor(); }
s/InitialzeProcessorInTest/InitializeProcessorInTest

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/ui_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller.cc:24: sync_client) {}
Should you DCHECK that the constructor is being called on the UI thread here? Or
can it actually be constructed from other threads?

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/ui_model_type_controller_unittest.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller_unittest.cc:42: void
InitialzeProcessorInTest() { InitializeProcessor(); }
s/InitialzeProcessorInTest/InitializeProcessorInTest

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller_unittest.cc:208: // These
threads can ping-pong for a bit so we run the model thread twice.
s/model/UI

[Gang Wu, 2016-02-24 01:04:51]

Description was changed from

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.


first checkin

BUG=558000
==========

to

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000
==========

[Gang Wu, 2016-02-24 01:05:13]

updated and rebased

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:73: // The error
indicates whether the loading completed successfully.
On 2016/02/23 20:35:01, skym wrote:
> Can this be private?

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:89: void
OnProcessorStarted(
On 2016/02/23 20:35:01, skym wrote:
> Can this be private?

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:95: void
OnProcessorStartedOnUIThread(
On 2016/02/23 20:35:01, skym wrote:
> Can this be private?

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:100: void
InitializeProcessor();
On 2016/02/23 20:35:00, skym wrote:
> Should this be public?

do not know who will call this function to initial processor yet, so let keep it
here.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:103:
base::WeakPtr<syncer_v2::SharedModelTypeProcessor> type_processor_;
On 2016/02/23 20:35:01, skym wrote:
> Can this be private?

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_ui_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_ui_model_type_controller.cc:24: sync_client) {}
On 2016/02/24 00:02:37, maxbogue wrote:
> Can this DCHECK that it is not called on the UI thread? Or do we not want to
> enforce that?

In most functions in UIDataTypeController/NonUIDataTypeController, there have a
DCHECK in the beginning of the functions. I think it is for enforce to that. so
I try to keep do samething.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/non_ui_model_type_controller_unittest.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/non_ui_model_type_controller_unittest.cc:57: void
InitialzeProcessorInTest() { InitializeProcessor(); }
On 2016/02/24 00:02:37, maxbogue wrote:
> s/InitialzeProcessorInTest/InitializeProcessorInTest

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/ui_model_type_controller.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller.cc:24: sync_client) {}
On 2016/02/24 00:02:38, maxbogue wrote:
> Should you DCHECK that the constructor is being called on the UI thread here?
Or
> can it actually be constructed from other threads?

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
File components/sync_driver/ui_model_type_controller_unittest.cc (right):

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller_unittest.cc:42: void
InitialzeProcessorInTest() { InitializeProcessor(); }
On 2016/02/24 00:02:38, maxbogue wrote:
> s/InitialzeProcessorInTest/InitializeProcessorInTest

Done.

https://codereview.chromium.org/1717433002/diff/100001/components/sync_driver...
components/sync_driver/ui_model_type_controller_unittest.cc:208: // These
threads can ping-pong for a bit so we run the model thread twice.
On 2016/02/24 00:02:38, maxbogue wrote:
> s/model/UI

Done.

[Gang Wu, 2016-02-24 01:34:21]

Patchset #5 (id:140001) has been deleted

[pavely, 2016-02-25 21:58:53]

lgtm

https://codereview.chromium.org/1717433002/diff/160001/components/sync_driver...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/160001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:81: // The funtion
will create SharedModelTypeProcessor on model thread.
funtion => function

[Gang Wu, 2016-02-26 00:05:58]

The CQ bit was checked by gangwu@chromium.org

[Gang Wu, 2016-02-26 00:05:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
maxbogue@chromium.org, skym@chromium.org, pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/1717433002/#ps180001
(title: "typo")

[Gang Wu, 2016-02-26 00:06:29]

https://codereview.chromium.org/1717433002/diff/160001/components/sync_driver...
File components/sync_driver/non_blocking_data_type_controller.h (right):

https://codereview.chromium.org/1717433002/diff/160001/components/sync_driver...
components/sync_driver/non_blocking_data_type_controller.h:81: // The funtion
will create SharedModelTypeProcessor on model thread.
On 2016/02/25 21:58:53, pavely wrote:
> funtion => function

Done.

[commit-bot: I haz the power, 2016-02-26 00:13:52]

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

[commit-bot: I haz the power, 2016-02-26 01:24:44]

Description was changed from

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000
==========

to

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000
==========

[commit-bot: I haz the power, 2016-02-26 01:24:45]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2016-02-26 01:26:06]

Description was changed from

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000
==========

to

==========
Break NonBlockingDataTypeController into UI specific one and
non-UI specific one. Also break
non_blocking_data_type_controller_unittest into ui and
non_ui.

BUG=558000

Committed: https://crrev.com/17c8d2773c3744b185e0b82c09f45332fc4d1928
Cr-Commit-Position: refs/heads/master@{#377754}
==========

[commit-bot: I haz the power, 2016-02-26 01:26:07]

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