Disable folder UI when app sync is disabled

Disable folder UI when app sync is disabled

BUG=376565
for c/b/sync/test:

R=tapted@chromium.org, xiyuan@chromium.org
TBR=zea@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274864

[xiyuan, 2014-06-02 19:57:49]

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
File ui/app_list/app_list_model.cc (right):

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
ui/app_list/app_list_model.cc:267: // Remove child items from folders.
I am not sure what is the best to do here. Can we leave the user data untouched?
This is a bit too destructive.

[stevenjb, 2014-06-02 20:25:11]

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
File ui/app_list/app_list_model.cc (right):

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
ui/app_list/app_list_model.cc:267: // Remove child items from folders.
On 2014/06/02 19:57:49, xiyuan wrote:
> I am not sure what is the best to do here. Can we leave the user data
untouched?
> This is a bit too destructive. 

We are only changing the in-memory model to match what the result will be after
a restart. If sync is re-enabled, the model will be re-populated with the
restored folder info.

[xiyuan, 2014-06-02 21:23:26]

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
File ui/app_list/app_list_model.cc (right):

https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
ui/app_list/app_list_model.cc:267: // Remove child items from folders.
On 2014/06/02 20:25:11, stevenjb wrote:
> We are only changing the in-memory model to match what the result will be
after
> a restart. If sync is re-enabled, the model will be re-populated with the
> restored folder info.

What if user disables sync then re-enables it in the same session? If the sync
was enabled at the beginning, the syncable service is monitoring model change.
Would that cause the changes here become permanent?

[stevenjb, 2014-06-02 22:49:16]

On 2014/06/02 21:23:26, xiyuan wrote:
>
https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
> File ui/app_list/app_list_model.cc (right):
> 
>
https://codereview.chromium.org/305013014/diff/20001/ui/app_list/app_list_mod...
> ui/app_list/app_list_model.cc:267: // Remove child items from folders.
> On 2014/06/02 20:25:11, stevenjb wrote:
> > We are only changing the in-memory model to match what the result will be
> after
> > a restart. If sync is re-enabled, the model will be re-populated with the
> > restored folder info.
> 
> What if user disables sync then re-enables it in the same session? If the sync
> was enabled at the beginning, the syncable service is monitoring model change.
> Would that cause the changes here become permanent?

It should work like any other changes when sync is disabled, i.e. it should
behave as one would expect: When sync is stopped, StopSync() will get called,
setting sync_processor_ to NULL before we call SetFoldersEnabled(false), so
those changes will not get synced. When sync starts up again,
MergeDataAndStartSyncing() will get called with the data on the server and the
folders will be restored. I tested this on a device and it worked as expected.

[xiyuan, 2014-06-02 23:10:58]

Thanks for clarification. LGTM

[tapted, 2014-06-02 23:33:47]

lgtm

[stevenjb, 2014-06-03 15:35:58]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-06-03 15:36:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/stevenjb@chromium.org/305013014/40001

[stevenjb, 2014-06-03 16:28:44]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-06-03 16:29:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/stevenjb@chromium.org/305013014/60001

[commit-bot: I haz the power, 2014-06-03 20:58:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[stevenjb, 2014-06-03 21:08:50]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-06-03 21:10:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/stevenjb@chromium.org/305013014/80001

[commit-bot: I haz the power, 2014-06-04 02:15:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[stevenjb, 2014-06-04 18:03:40]

Committed patchset #6 manually as r274864 (presubmit successful).