arc: Fix race when arc package sync service starts.

arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

Committed: https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5
Cr-Commit-Position: refs/heads/master@{#423362}

[lgcheng, 2016-10-03 23:01:23]

Description was changed from

==========
Race.

BUG=652470
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady()
to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test.
==========

[lgcheng, 2016-10-03 23:01:45]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady()
to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test.
==========

[lgcheng, 2016-10-03 23:04:00]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

[lgcheng, 2016-10-04 00:27:03]

The CQ bit was checked by lgcheng@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 00:28:06]

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, 2016-10-04 00:33:05]

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

[commit-bot: I haz the power, 2016-10-04 00:33:06]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[lgcheng, 2016-10-04 21:08:19]

Patchset #2 (id:20001) has been deleted

[lgcheng, 2016-10-04 22:05:01]

Patchset #2 (id:40001) has been deleted

[lgcheng, 2016-10-04 22:06:32]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race is between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

[lgcheng, 2016-10-04 22:06:56]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and sync
DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

[lgcheng, 2016-10-04 22:09:14]

lgcheng@google.com changed reviewers:
+ pavely@chromium.org, xiyuan@chromium.org

[lgcheng, 2016-10-04 22:10:12]

lgcheng@google.com changed reviewers:
+ khmel@chromium.org

[lgcheng, 2016-10-04 22:10:12]

Hi Xiyuan, Pavel, Yury

PTAL at the patch.

Thanks!

[lgcheng, 2016-10-04 22:11:22]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.
==========

[khmel, 2016-10-04 22:31:59]

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1136: if
(package_list_initial_refreshed_)
I don't really understand meaning of package list initially refreshed. As I
understand from Android side implementation package list is sent only once. All
other events are package added/updated/removed.

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.h (right):

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:237: bool
PackageListInitialRefreshed() const {
nit: package_list_initial_refreshed()

[pavely, 2016-10-05 00:02:06]

lgtm

[lgcheng, 2016-10-05 00:24:36]

Thanks Yury for comments.

PTAL.

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1136: if
(package_list_initial_refreshed_)
On 2016/10/04 22:31:59, khmel wrote:
> I don't really understand meaning of package list initially refreshed. As I
> understand from Android side implementation package list is sent only once.
All
> other events are package added/updated/removed.

As we discussed offline, more work needs to be done before we can remove the
flag. I will remove this flag after I work a proper solution on Android side.

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.h (right):

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.h:237: bool
PackageListInitialRefreshed() const {
On 2016/10/04 22:31:59, khmel wrote:
> nit: package_list_initial_refreshed()

Done.

[khmel, 2016-10-05 00:30:41]

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):

https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1136: if
(package_list_initial_refreshed_)
On 2016/10/05 00:24:36, lgcheng wrote:
> On 2016/10/04 22:31:59, khmel wrote:
> > I don't really understand meaning of package list initially refreshed. As I
> > understand from Android side implementation package list is sent only once.
> All
> > other events are package added/updated/removed.
> 
> As we discussed offline, more work needs to be done before we can remove the
> flag. I will remove this flag after I work a proper solution on Android side.

This bug is not marked for M54, so that means we have no rush making it properly
(also change on Android side should not be big) and my recommendation do it
properly. As we discussed current CL is just workaround. However I won't block
if you insist with this CL and defer this to Xiyuan.

lgtm

[lgcheng, 2016-10-05 00:40:02]

On 2016/10/05 00:30:41, khmel wrote:
>
https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
> File chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc (right):
> 
>
https://codereview.chromium.org/2389763003/diff/60001/chrome/browser/ui/app_l...
> chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc:1136: if
> (package_list_initial_refreshed_)
> On 2016/10/05 00:24:36, lgcheng wrote:
> > On 2016/10/04 22:31:59, khmel wrote:
> > > I don't really understand meaning of package list initially refreshed. As
I
> > > understand from Android side implementation package list is sent only
once.
> > All
> > > other events are package added/updated/removed.
> > 
> > As we discussed offline, more work needs to be done before we can remove the
> > flag. I will remove this flag after I work a proper solution on Android
side.
> 
> This bug is not marked for M54, so that means we have no rush making it
properly
> (also change on Android side should not be big) and my recommendation do it
> properly. As we discussed current CL is just workaround. However I won't block
> if you insist with this CL and defer this to Xiyuan.
> 
> lgtm

Thanks for review! The race is introduced in my last CL(
https://codereview.chromium.org/2353213002/ which is approved for M54 merge).
I'd
like to merge two CLs together for short time.

[xiyuan, 2016-10-05 16:38:58]

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:71:
arc_prefs_->RemoveObserver(this);
Why do we need RemoveObserver then AddObserver?

[lgcheng, 2016-10-05 16:50:54]

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:71:
arc_prefs_->RemoveObserver(this);
On 2016/10/05 16:38:58, xiyuan wrote:
> Why do we need RemoveObserver then AddObserver?

When user disable ARC and then enable ARC without reboot, both ArcAppListPrefs
and ArcPackageSyncDataTypeController will not be deconstructed. But
StartModels() will be called again to restart sync . Currently if we already add
ArcPackageSyncDataTypeController as observer of ArcAppListPrefs and re-add it
again there will be an ERROR:
  DCHECK(obs);
  if (ContainsValue(observers_, obs)) {
    NOTREACHED() << "Observers can only be added once!";
    return;
  }
And the reason why I don't AddObserver in constructor is because sync data type
controllers are constructed very early(by chrome_sync_client) when
ArcAppListPrefs was not enabled to construct for the profile by
ArcAuthenService.

[khmel, 2016-10-05 16:55:20]

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:71:
arc_prefs_->RemoveObserver(this);
On 2016/10/05 16:50:54, lgcheng wrote:
> On 2016/10/05 16:38:58, xiyuan wrote:
> > Why do we need RemoveObserver then AddObserver?
> 
> When user disable ARC and then enable ARC without reboot, both ArcAppListPrefs
> and ArcPackageSyncDataTypeController will not be deconstructed. But
> StartModels() will be called again to restart sync . Currently if we already
add
> ArcPackageSyncDataTypeController as observer of ArcAppListPrefs and re-add it
> again there will be an ERROR:
>   DCHECK(obs);
>   if (ContainsValue(observers_, obs)) {
>     NOTREACHED() << "Observers can only be added once!";
>     return;
>   }
> And the reason why I don't AddObserver in constructor is because sync data
type
> controllers are constructed very early(by chrome_sync_client) when
> ArcAppListPrefs was not enabled to construct for the profile by
> ArcAuthenService. 

Do we need kind of StopModel if Arc is stopped? In stop model we may remove
observer

[xiyuan, 2016-10-05 17:00:57]

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:71:
arc_prefs_->RemoveObserver(this);
On 2016/10/05 16:55:20, khmel wrote:
> On 2016/10/05 16:50:54, lgcheng wrote:
> > On 2016/10/05 16:38:58, xiyuan wrote:
> > > Why do we need RemoveObserver then AddObserver?
> > 
> > When user disable ARC and then enable ARC without reboot, both
ArcAppListPrefs
> > and ArcPackageSyncDataTypeController will not be deconstructed. But
> > StartModels() will be called again to restart sync . Currently if we already
> add
> > ArcPackageSyncDataTypeController as observer of ArcAppListPrefs and re-add
it
> > again there will be an ERROR:
> >   DCHECK(obs);
> >   if (ContainsValue(observers_, obs)) {
> >     NOTREACHED() << "Observers can only be added once!";
> >     return;
> >   }
> > And the reason why I don't AddObserver in constructor is because sync data
> type
> > controllers are constructed very early(by chrome_sync_client) when
> > ArcAppListPrefs was not enabled to construct for the profile by
> > ArcAuthenService. 
> 
> Do we need kind of StopModel if Arc is stopped? In stop model we may remove
> observer

That would be better than doing this trick. But if we have to use the trick, can
we move RemoveObserver close to AddObserver and add a commnet? The current code
layout is a bit confusing. It looks like we are trying to avoid being observer
callback while reading the package_list_initial_refreshed(), a bit odd.

[lgcheng, 2016-10-05 17:09:11]

On 2016/10/05 17:00:57, xiyuan wrote:
>
https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
> File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
> (right):
> 
>
https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
> chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:71:
> arc_prefs_->RemoveObserver(this);
> On 2016/10/05 16:55:20, khmel wrote:
> > On 2016/10/05 16:50:54, lgcheng wrote:
> > > On 2016/10/05 16:38:58, xiyuan wrote:
> > > > Why do we need RemoveObserver then AddObserver?
> > > 
> > > When user disable ARC and then enable ARC without reboot, both
> ArcAppListPrefs
> > > and ArcPackageSyncDataTypeController will not be deconstructed. But
> > > StartModels() will be called again to restart sync . Currently if we
already
> > add
> > > ArcPackageSyncDataTypeController as observer of ArcAppListPrefs and re-add
> it
> > > again there will be an ERROR:
> > >   DCHECK(obs);
> > >   if (ContainsValue(observers_, obs)) {
> > >     NOTREACHED() << "Observers can only be added once!";
> > >     return;
> > >   }
> > > And the reason why I don't AddObserver in constructor is because sync data
> > type
> > > controllers are constructed very early(by chrome_sync_client) when
> > > ArcAppListPrefs was not enabled to construct for the profile by
> > > ArcAuthenService. 
> > 
> > Do we need kind of StopModel if Arc is stopped? In stop model we may remove
> > observer
> 
> That would be better than doing this trick. But if we have to use the trick,
can
> we move RemoveObserver close to AddObserver and add a commnet? The current
code
> layout is a bit confusing. It looks like we are trying to avoid being observer
> callback while reading the package_list_initial_refreshed(), a bit odd.

Yes, that would be a nice solution. 
But I am also concerning about if there will be the scenario that Arc starts too
slowly and sync services tries to call DataTypeManagerImpl::Restart without
waiting
of OnModelLoaded(). I'll investigate a little more before I move forward.

[lgcheng, 2016-10-05 19:50:16]

Move RemoveObservers to overrided StopModels. 

PTAL

Thanks!

[xiyuan, 2016-10-05 19:53:26]

lgtm

[khmel, 2016-10-05 19:55:12]

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/80001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:44: if
(arc_prefs_)
Can we guarantee that StopModel is always called?

https://codereview.chromium.org/2389763003/diff/100001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/100001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:75:
arc_prefs_->RemoveObserver(this);
Please use  DCHECK(arc_prefs_); as in StartModels.
Can we guarantee that ArcPrefs is not deleted first?
Do we need to call base method?

[lgcheng, 2016-10-05 20:17:00]

Check added in StopModels()

Thanks for review!

https://codereview.chromium.org/2389763003/diff/100001/chrome/browser/ui/app_...
File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
(right):

https://codereview.chromium.org/2389763003/diff/100001/chrome/browser/ui/app_...
chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:75:
arc_prefs_->RemoveObserver(this);
On 2016/10/05 19:55:12, khmel wrote:
> Please use  DCHECK(arc_prefs_); as in StartModels.
> Can we guarantee that ArcPrefs is not deleted first?
> Do we need to call base method?

Use if() to check in case ArcAppListPrefs gets deleted early. 
Base method of this is no-op.

[lgcheng, 2016-10-06 00:21:49]

Hi Yury,

This patch fixes discussion offline.

[khmel, 2016-10-06 00:23:37]

On 2016/10/06 00:21:49, lgcheng wrote:
> Hi Yury,
> 
> This patch fixes discussion offline.

lgtm

[khmel, 2016-10-06 00:23:40]

On 2016/10/06 00:21:49, lgcheng wrote:
> Hi Yury,
> 
> This patch fixes discussion offline.

lgtm

[lgcheng, 2016-10-06 00:24:27]

The CQ bit was checked by lgcheng@google.com

[lgcheng, 2016-10-06 00:24:27]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org,
xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/2389763003/#ps140001
(title: "Add Check in case ArcAppListPrefs gets deleted early.")

[commit-bot: I haz the power, 2016-10-06 00:25:23]

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

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.
==========

[commit-bot: I haz the power, 2016-10-06 00:53:02]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-10-06 00:56:03]

Description was changed from

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.
==========

to

==========
arc: Fix race when arc package sync service starts.
Fix the race which happens between initial packagelist refreshed and
sync DataTypeController::OnModelStarted by changing the observer of
OnAppInstanceReady() to OnPackageListInitialRefreshed().

BUG=652470
Test=Manual Test. Sync service always starts after arc packagelist
refreshes correctly.
Test=Pass sync integration test.

Committed: https://crrev.com/88a14008509adefe2b764ccb573ba629ba68dac5
Cr-Commit-Position: refs/heads/master@{#423362}
==========

[commit-bot: I haz the power, 2016-10-06 00:56:04]

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