Hi pavely,

Is there anything special to do for managed account?

I'm not sure. I'll take a look.

For managed accounts data removal gets invoked on sing out. Here is the stack I captured when signing out from google.com account:

3   Chromium                            0x0000000106575afc IOSChromeBrowsingDataRemover::RemoveImpl(int) + 268
4   Chromium                            0x00000001065759db IOSChromeBrowsingDataRemover::Remove(int) + 27
5   Chromium                            0x000000010656c8a4 BrowsingDataRemoverHelper::DoRemove(ios::ChromeBrowserState*, std::__1::unique_ptr<BrowsingDataRemoverHelper::BrowsingDataRemovalInfo, std::__1::default_delete<BrowsingDataRemoverHelper::BrowsingDataRemovalInfo> >) + 884
6   Chromium                            0x000000010656c136 BrowsingDataRemoverHelper::Remove(ios::ChromeBrowserState*, int, browsing_data::TimePeriod, base::Callback<void (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) + 3574
7   Chromium                            0x00000001069d07a3 -[BrowsingDataRemovalController removeBrowsingDataFromBrowserState:mask:timePeriod:completionHandler:] + 2275
8   Chromium                            0x0000000105c7c68e -[MainController removeBrowsingDataFromBrowserState:mask:timePeriod:completionHandler:] + 190
9   Chromium                            0x0000000105c782a5 -[MainController chromeExecuteCommand:] + 2309
10  Chromium                            0x0000000105c6eb6d -[MainApplicationDelegate chromeExecuteCommand:] + 61
11  Chromium                            0x00000001061e6ae5 -[UIWindow(ChromeExecuteCommand) chromeExecuteCommand:] + 213
12  Chromium                            0x00000001061cc552 BrowserStateDataRemover::RemoveBrowserStateData(void () block_pointer) + 770
13  Chromium                            0x00000001061cc20c BrowserStateDataRemover::ClearData(ios::ChromeBrowserState*, void () block_pointer) + 92
14  Chromium                            0x00000001061be299 AuthenticationService::SignOut(signin_metrics::ProfileSignout, void () block_pointer) + 217
15  Chromium                            0x00000001065dc57f -[AccountsCollectionViewController handleDisconnect] + 191
16  Chromium                            0x00000001065dc468 __50-[AccountsCollectionViewController showDisconnect]_block_invoke + 40
17  Chromium                            0x000000010649a654 __50-[AlertCoordinator addItemWithTitle:action:style:]_block_invoke + 148
18  UIKit                               0x0000000111dc5257 -[UIAlertController _invokeHandlersForAction:] + 98
19  UIKit                               0x0000000111dc5bf6 __85-[UIAlertController _dismissAnimated:triggeringAction:triggeredByPopoverDimmingView:]_block_invoke.443 + 16
20  UIKit                               0x0000000111bd5454 -[UIPresentationController transitionDidFinish:] + 1289
21  UIKit                               0x0000000111bd8ef0 __56-[UIPresentationController runTransitionForCurrentState]_block_invoke_2 + 183
22  UIKit                               0x000000011259a56c -[_UIViewControllerTransitionContext completeTransition:] + 102
23  UIKit                               0x0000000111b121fd -[UIViewAnimationBlockDelegate _didEndBlockAnimation:finished:context:] + 528
24  UIKit                               0x0000000111ae4bd5 -[UIViewAnimationState sendDelegateAnimationDidStop:finished:] + 222
25  UIKit                               0x0000000111ae512a -[UIViewAnimationState animationDidStop:finished:] + 136
26  QuartzCore                          0x0000000114a94648 CA::Layer::run_animation_callbacks(void*) + 316


I think reading list data should be handled in IOSChromeBrowsingDataRemover::RemoveImpl as well. Looking at blame on ios_chrome_browsing_data_remover.mm seems like Colin(blundell@) or Sylvian(sdefresne@) would know how to correctly clear dat. 

+sdefresne

+msarda, +bzanotti: I think removing data from managed account has to be implemented locally for each type of data, right?

So here I guess that we just need to implement clearing the READING_LIST data (or if this exists, call the code to clear them).

+Eli to evaluate if this needs to be cherry-picked to M57 or not.

Eli: The impact of this bug is that it looks like Chrome iOS does not remove the reading list data in the following cases:
* when a user signs out of a enterprise managed account
* when the user switches from an account A to account B.
This means that the user may merge reading list data between two accounts.

I think this should be an RBS for M57, even though the affected population is very small (only iOS uses that are either managed and sign out or switch account).

I Can do a fix for Reading List, but I think this should be done either by profile code, or sync code.

I am submitting M57 to the AppStore today and if this fix has to be in M57, the submission will have to be delayed. If the percentage of users that will be affected by this issue is very small, then it may not be worth delaying the M57 release and we should rather have this fix in the next release whether M57 Refresh or M58 release.

Can we fix this in a M57 refresh instead? I'm not certain this is RBS. 

I'll defer the decision if this bug is RBS to to the Chrome iOS leads. Let me know if you need any input/data from me.

The fix is in review.
I filed bug 700051 to have a generic solution.

#12: the IOSChromeBrowsingDataRemover class is responsible for clearing the data, so I guess this is the generic solution and where the clearing code should be added. This is used by the clear data in the settings, and when switching from a managed account to a non-managed one (or in the other direction).

If you check the code, you'll see that it call to the different services storing data on behalf of the user (history, autofill, host name resolution, tabs, ...) and issue command to request the data to be cleared.

Given the size of the affected population and the nature of the data that is not being deleted, this should not be RBS but should be fixed for our next release. 

Agreed, would not call this RBS, but seems worth fixing as soon as possible.

We should make sure (as Mihai mentioned) that the solution we implement works not only for managed accounts, but also when switching between two non-managed accounts and opting to clear data.

Thank you.

I put msarda as reviewer of the CL and will work with him on this.
Thanks to  srikanthg@chromium.org for reporting the bug and sorry for having missed it.

Thank you

I checked that the fix is handling correctly the case while switching accounts.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85c6bf63ddea1f0a18f2404090acb96f67369ca2

commit 85c6bf63ddea1f0a18f2404090acb96f67369ca2
Author: olivierrobin <olivierrobin@chromium.org>
Date: Tue Mar 21 12:03:14 2017

Remove all ReadingList entries on managed account signout.

Clearing happens after the actual signout, so only local data is deleted.
Also interrupt current distillation on signout as profile may change.

BUG= 699590 

Review-Url: https://codereview.chromium.org/2745313004
Cr-Commit-Position: refs/heads/master@{#458377}

[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/components/reading_list/ios/favicon_web_state_dispatcher.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/components/reading_list/ios/reading_list_model.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/components/reading_list/ios/reading_list_model_impl.cc
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/components/reading_list/ios/reading_list_model_impl.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/BUILD.gn
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/favicon_web_state_dispatcher_impl.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/favicon_web_state_dispatcher_impl.mm
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_download_service.h
[add] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_remover_helper.cc
[add] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/reading_list_remover_helper.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/url_downloader.cc
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/reading_list/url_downloader.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/signin/BUILD.gn
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/signin/browser_state_data_remover.h
[modify] https://crrev.com/85c6bf63ddea1f0a18f2404090acb96f67369ca2/ios/chrome/browser/signin/browser_state_data_remover.mm

Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40

commit 8e3637fbb0c5adf4a37f36c059a85dccae1b1f40
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Thu Mar 23 09:06:30 2017

Remove all ReadingList entries on managed account signout.

Clearing happens after the actual signout, so only local data is deleted.
Also interrupt current distillation on signout as profile may change.

BUG= 699590 

Review-Url: https://codereview.chromium.org/2745313004
Cr-Commit-Position: refs/heads/master@{#458377}
(cherry picked from commit 85c6bf63ddea1f0a18f2404090acb96f67369ca2)

Review-Url: https://codereview.chromium.org/2770723004 .
Cr-Commit-Position: refs/branch-heads/3029@{#380}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/components/reading_list/ios/favicon_web_state_dispatcher.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/components/reading_list/ios/reading_list_model.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/components/reading_list/ios/reading_list_model_impl.cc
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/components/reading_list/ios/reading_list_model_impl.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/BUILD.gn
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/favicon_web_state_dispatcher_impl.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/favicon_web_state_dispatcher_impl.mm
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_distiller_page_factory.mm
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_download_service.cc
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_download_service.h
[add] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_remover_helper.cc
[add] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/reading_list_remover_helper.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/url_downloader.cc
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/reading_list/url_downloader.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/signin/BUILD.gn
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/signin/browser_state_data_remover.h
[modify] https://crrev.com/8e3637fbb0c5adf4a37f36c059a85dccae1b1f40/ios/chrome/browser/signin/browser_state_data_remover.mm

Verified on chrome canary version 59.0.3054.0 on iPhone 7 plus with iOS 10.2.1 and iPad Air with iOS 10.2.1, following the steps mentioned in comment #0.  Reading list entries are deleted after signing out of account.  Looks good.

Verified on  version 58.0.3029.39 beta, on iPhone 6s+ 10.2.1,iPad Air 10.2.1.Reading list entries are deleted after signing out from managed account.