Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2353213002:
arc: Fix Android App resurrected by sync. (Closed)
DescriptionFix Android App resurrected by sync.
This patch makes ReadyForStart + ReenableDatatype to be called only when the
type is being disabled/re-enabled for some external to sync condition.
StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal.
Also moves re-enable sync data type related code from sync service to sync data
type controller.
BUG=650483
Test=Pass exsiting Test.
Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When
Chromebook2 is on, the app is removed by sync.
Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline.
When Chromebook2 is back online, app is removed by sync.
Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When
Chromebook1 is back online, app is removed by sync on Chromebook2.
Committed: https://crrev.com/077a23cfcc11a665a41e878be2270218a0a0f429
Cr-Commit-Position: refs/heads/master@{#421876}
Patch Set 1 #Patch Set 2 : Android App resurrected by sync. #Patch Set 3 : Android App resurrected by sync. #Patch Set 4 : Rebase. #Patch Set 5 : Nits. #Patch Set 6 : Remove conflict resolver related change. #Patch Set 7 : Nit. #
Total comments: 12
Patch Set 8 : Address Xiyuan's comments. #Patch Set 9 : Address Yury's comment. #
Total comments: 2
Patch Set 10 : Android App resurrected by sync. #
Total comments: 4
Patch Set 11 : Address Pavel's comments. #
Total comments: 2
Patch Set 12 : Remove redundant pref observer. #
Messages
Total messages: 54 (40 generated)
Description was changed from ========== Android App resurrected by sync. BUG= ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves the observer from Arc sync service to data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ==========
Patchset #2 (id:20001) has been deleted
Patchset #4 (id:80001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #4 (id:100001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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...) 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_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #4 (id:120001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #4 (id:140001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by lgcheng@google.com
Patchset #4 (id:160001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: 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...)
Patchset #4 (id:180001) has been deleted
The CQ bit was checked by lgcheng@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves the observer from Arc sync service to data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves the observer of OnAppInstanceReady from Arc sync service to data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ==========
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves the observer of OnAppInstanceReady from Arc sync service to data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ==========
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels() is now called when ARC AppInstanceReady is observed. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady is observed. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ==========
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady is observed. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ==========
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. ==========
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When Chromebook1 is back online, app is removed by sync on Chromebook2. ==========
lgcheng@google.com changed reviewers: + jhorwich@chromium.org, khmel@chromium.org, pavely@chromium.org, xiyuan@chromium.org
Hi Pavel, xiyuan PTAL at the patch. CC Yury and Josh. Welcome to leave comments.
LGTM with nits Please wait for pavely@ before submitting since I am not a sync expert. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:54: model_normal_start_(true), nit: move this initializer to header https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:80: return ValidateEnableArcPackageSyncPref(profile_) && IsArcEnabled(profile_); nit: swap the two conditions to check IsArcEnabled first ? https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:88: // called. nit: mention that |model_normal_start_| is initialized as true https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h:39: bool StartModels() override; nit: Usually we keep all overrides from one interface together. That is, this should declare with ReadyForStart().
Thanks Xiyuan for review. Comments addressed. Pavel, would you PTAL when you have a moment? Thanks! https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:54: model_normal_start_(true), On 2016/09/28 16:31:14, xiyuan wrote: > nit: move this initializer to header Done. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:80: return ValidateEnableArcPackageSyncPref(profile_) && IsArcEnabled(profile_); On 2016/09/28 16:31:14, xiyuan wrote: > nit: swap the two conditions to check IsArcEnabled first ? Thanks for pointing out that swap the order normally result in early return. Done. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:88: // called. On 2016/09/28 16:31:14, xiyuan wrote: > nit: mention that |model_normal_start_| is initialized as true Done. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h:39: bool StartModels() override; On 2016/09/28 16:31:14, xiyuan wrote: > nit: Usually we keep all overrides from one interface together. That is, this > should declare with ReadyForStart(). Done.
https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:27: bool apps_sync_enable = pref_service->GetBoolean( nit: const bool apps_... https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:43: return profile->GetPrefs()->GetBoolean(prefs::kArcEnabled); This could be dangerous. ArcEnabled may be set in profile but profile itself may be denied. You need to use ArcAuthService::IsEnabledForProfile() && your condition here.
Thanks Yury for comments. Issue addressed. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:27: bool apps_sync_enable = pref_service->GetBoolean( On 2016/09/28 17:01:56, khmel wrote: > nit: const bool apps_... Done. https://codereview.chromium.org/2353213002/diff/260001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:43: return profile->GetPrefs()->GetBoolean(prefs::kArcEnabled); On 2016/09/28 17:01:56, khmel wrote: > This could be dangerous. ArcEnabled may be set in profile but profile itself may > be denied. > You need to use ArcAuthService::IsEnabledForProfile() && your condition here. Thanks for pointing out this logic! Done. But did you mean ArcAuthService::IsAllowedForProfile()?
lgtm with one nit https://codereview.chromium.org/2353213002/diff/300001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/300001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:56: model_normal_start_(true), nit: move init of model_normal_start_ to header. If it already there please remove
https://codereview.chromium.org/2353213002/diff/300001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/300001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:56: model_normal_start_(true), On 2016/09/28 17:25:49, khmel wrote: > nit: move init of model_normal_start_ to header. If it already there please > remove Done. Sorry I miss the remove.
https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:23: PrefService* pref_service = profile->GetPrefs(); You can use SyncService::GetPreferredDataTypes() and check if ARC_PACKAGE is in returned ModelTypeSet. It will cover checking "Sync everything" as well as including ARC_PACKAGE when APPS checkbox is enabled. https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:36: pref_service->SetBoolean(arc_sync_path, apps_sync_enable); Why do you need to implement this logic? Is there a scenario you ran into that you want to cover?
Issue addressed. PTAL, Thanks! https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:23: PrefService* pref_service = profile->GetPrefs(); On 2016/09/28 20:19:01, pavely wrote: > You can use SyncService::GetPreferredDataTypes() and check if ARC_PACKAGE is in > returned ModelTypeSet. It will cover checking "Sync everything" as well as > including ARC_PACKAGE when APPS checkbox is enabled. Done. https://codereview.chromium.org/2353213002/diff/320001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:36: pref_service->SetBoolean(arc_sync_path, apps_sync_enable); On 2016/09/28 20:19:01, pavely wrote: > Why do you need to implement this logic? Is there a scenario you ran into that > you want to cover? There was a scenario that user chose what to sync(not sync everything) before updating the patch which associates ARC_PACKAGE to APPS. Then after update with the patch ARC_PACKAGE sync pref is still unchecked while APPS was checked before the patch and ARC_PACKAGE sync service will not start. But it would not reproduce if I use SyncService::GetPreferredDataTypes() and check if ARC_PACKAGE is inreturned ModelTypeSet.
lgtm https://codereview.chromium.org/2353213002/diff/340001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/340001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:39: pref_registrar_.Add( You don't need to register for notifications on sync pref changes. There is no need to generate unrecoverable error when the pref turns to false or to explicitly reenable type when pref changes to true. Sync engine monitors these changes and will start/stop datatypes as needed. Basically I'm proposing to remove observing SyncPrefs::GetPrefNameForDataType(type) pref and OnArcAppsSyncPrefChanged function. You still need to monitor prefs::kArcEnabled though. Feel free to make this change in separate CL if it is more convenient.
Thanks for review! https://codereview.chromium.org/2353213002/diff/340001/chrome/browser/ui/app_... File chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc (right): https://codereview.chromium.org/2353213002/diff/340001/chrome/browser/ui/app_... chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc:39: pref_registrar_.Add( On 2016/09/29 08:49:02, pavely wrote: > You don't need to register for notifications on sync pref changes. There is no > need to generate unrecoverable error when the pref turns to false or to > explicitly reenable type when pref changes to true. Sync engine monitors these > changes and will start/stop datatypes as needed. > > Basically I'm proposing to remove observing > SyncPrefs::GetPrefNameForDataType(type) pref and OnArcAppsSyncPrefChanged > function. > > You still need to monitor prefs::kArcEnabled though. > > Feel free to make this change in separate CL if it is more convenient. I think this is very neat suggestion to remove the redundancy. As I am going to merge the patch to M54, I'd like to remove unnecessary observer in this patch.
The CQ bit was checked by lgcheng@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org, khmel@chromium.org, pavely@chromium.org Link to the patchset: https://codereview.chromium.org/2353213002/#ps360001 (title: "Remove redundant pref observe.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When Chromebook1 is back online, app is removed by sync on Chromebook2. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When Chromebook1 is back online, app is removed by sync on Chromebook2. ==========
Message was sent while issue was closed.
Committed patchset #12 (id:360001)
Message was sent while issue was closed.
Description was changed from ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When Chromebook1 is back online, app is removed by sync on Chromebook2. ========== to ========== Fix Android App resurrected by sync. This patch makes ReadyForStart + ReenableDatatype to be called only when the type is being disabled/re-enabled for some external to sync condition. StartModels/OnModelLoaded is now used to track ARC AppInstanceReady signal. Also moves re-enable sync data type related code from sync service to sync data type controller. BUG=650483 Test=Pass exsiting Test. Test1=Manually remove app on Chromebook1 while Chromebook2 is off. When Chromebook2 is on, the app is removed by sync. Test2=Manually remove app on Chromebook1 while Chromebook2 is on but offline. When Chromebook2 is back online, app is removed by sync. Test3=Manually remove app on Chromebook1 while Chromebook1 is offline. When Chromebook1 is back online, app is removed by sync on Chromebook2. Committed: https://crrev.com/077a23cfcc11a665a41e878be2270218a0a0f429 Cr-Commit-Position: refs/heads/master@{#421876} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/077a23cfcc11a665a41e878be2270218a0a0f429 Cr-Commit-Position: refs/heads/master@{#421876} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
