Refactor BrowsingDataRemovalController

Move all the code related to clearing browsing data to
IOSChromeBrowsingDataRemover after remarking that method
-removeIOSSpecificIncognitoBrowsingDataFromBrowserState:..
is unnecessary as -removeBrowsingDataFromBrowserState:...
clear exactly the same data is BrowserState is incognito.

Refactor some method clearing browsing data to accept a
closure (those that are asynchronous).

Bug: none
Change-Id: I7dfbb7040b87343fdd3df3b473a7d4c2046d4c46
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/919264
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537809}

ios/chrome/app/main_controller.mm

@@ -833,9 +833,10 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
   };
   const BrowsingDataRemoveMask mask = BrowsingDataRemoveMask::REMOVE_ALL;
   [self.browsingDataRemovalController
-      removeIOSSpecificIncognitoBrowsingDataFromBrowserState:otrBrowserState
-                                                        mask:mask
-                                           completionHandler:completion];
+      removeBrowsingDataFromBrowserState:otrBrowserState
+                                    mask:mask
+                              timePeriod:browsing_data::TimePeriod::ALL_TIME
+                       completionHandler:completion];
 }
 
 - (void)deleteIncognitoBrowserState {

ios/chrome/browser/browsing_data/BUILD.gn

@@ -23,6 +23,7 @@ source_set("browsing_data") {
     "//components/keyed_service/core",
     "//components/language/core/browser",
     "//components/omnibox/browser",
+    "//components/open_from_clipboard",
     "//components/password_manager/core/browser",
     "//components/prefs",
     "//components/sessions",
@@ -37,8 +38,10 @@ source_set("browsing_data") {
     "//ios/chrome/browser/sessions",
     "//ios/chrome/browser/sessions:serialisation",
     "//ios/chrome/browser/signin",
+    "//ios/chrome/browser/snapshots",
     "//ios/chrome/browser/sync",
     "//ios/chrome/browser/translate:translate",
+    "//ios/chrome/browser/ui:external_files",
     "//ios/net",
     "//ios/public/provider/chrome/browser",
     "//ios/web",
@@ -65,6 +68,8 @@ source_set("unit_tests") {
     "//base",
     "//base/test:test_support",
     "//components/browsing_data/core",
+    "//components/open_from_clipboard",
+    "//components/open_from_clipboard:test_support",
     "//components/pref_registry",
     "//components/prefs",
     "//components/prefs:test_support",
@@ -76,7 +81,6 @@ source_set("unit_tests") {
     "//ios/web/public/test/fakes",
     "//net",
     "//testing/gtest",
-    "//third_party/ocmock",
   ]
 }
 

ios/chrome/browser/browsing_data/DEPS

 specific_include_rules = {
   # TODO(crbug.com/619783): Remove this exception.
-  "^browsing_data_removal_controller\.mm$": [
+  "^ios_chrome_browsing_data_remover\.mm$": [
     "+ios/web/web_state/ui/wk_web_view_configuration_provider.h",
   ],
 }

ios/chrome/browser/browsing_data/browsing_data_removal_controller.h

@@ -11,8 +11,6 @@
 #include "components/browsing_data/core/browsing_data_utils.h"
 #include "ios/chrome/browser/browsing_data/browsing_data_remove_mask.h"
 
-@class BrowserViewController;
-
 namespace ios {
 class ChromeBrowserState;
 }  // namespace ios
@@ -30,19 +28,6 @@ class ChromeBrowserState;
                                 timePeriod:(browsing_data::TimePeriod)timePeriod
                          completionHandler:(ProceduralBlock)completionHandler;
 
-// Removes browsing data that iOS has associated with |browserState| and which
-// is not removed when the |browserState| is destroyed. |mask| is obtained from
-// BrowsingDataRemoveMask. |browserState| cannot be null. |completionHandler| is
-// called when this operation finishes. This method finishes removal of the
-// browsing data even if |browserState| is destroyed after this method call.
-- (void)
-removeIOSSpecificIncognitoBrowsingDataFromBrowserState:
-    (ios::ChromeBrowserState*)browserState
-                                                  mask:(BrowsingDataRemoveMask)
-                                                           mask
-                                     completionHandler:
-                                         (ProceduralBlock)completionHandler;
-
 // Called when |browserState| is destroyed.
 - (void)browserStateDestroyed:(ios::ChromeBrowserState*)browserState;
 

ios/chrome/browser/browsing_data/browsing_data_removal_controller_unittest.mm

@@ -6,41 +6,63 @@
 
 #include <memory>
 
-#import "base/test/ios/wait_util.h"
+#include "base/logging.h"
+#include "base/run_loop.h"
+#include "components/open_from_clipboard/clipboard_recent_content.h"
+#include "components/open_from_clipboard/fake_clipboard_recent_content.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
-#include "ios/web/public/test/web_test.h"
+#include "ios/web/public/test/test_web_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#import "third_party/ocmock/OCMock/OCMock.h"
-#import "third_party/ocmock/gtest_support.h"
+#include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
-typedef web::WebTest BrowsingDataRemovalControllerTest;
+class BrowsingDataRemovalControllerTest : public PlatformTest {
+ public:
+  BrowsingDataRemovalControllerTest()
+      : browser_state_(TestChromeBrowserState::Builder().Build()) {
+    DCHECK_EQ(ClipboardRecentContent::GetInstance(), nullptr);
+    ClipboardRecentContent::SetInstance(
+        std::make_unique<FakeClipboardRecentContent>());
+  }
 
-// Tests that |removeIOSSpecificIncognitoBrowsingDataFromBrowserState:| can
+  ~BrowsingDataRemovalControllerTest() override {
+    DCHECK_NE(ClipboardRecentContent::GetInstance(), nullptr);
+    ClipboardRecentContent::SetInstance(nullptr);
+  }
+
+ protected:
+  web::TestWebThreadBundle thread_bundle_;
+  std::unique_ptr<ios::ChromeBrowserState> browser_state_;
+};
+
+// Tests that
+// -removeBrowsingDataFromBrowserState:mask:timePeriod:completionHandler: can
 // finish performing its operation even when a BrowserState is destroyed.
 TEST_F(BrowsingDataRemovalControllerTest, PerformAfterBrowserStateDestruction) {
-  __block BOOL block_was_called = NO;
+  base::RunLoop run_loop;
+  base::RepeatingClosure quit_run_loop = run_loop.QuitClosure();
+
   BrowsingDataRemovalController* removal_controller =
       [[BrowsingDataRemovalController alloc] init];
 
-  TestChromeBrowserState::Builder builder;
-  std::unique_ptr<TestChromeBrowserState> browser_state = builder.Build();
-  ios::ChromeBrowserState* otr_browser_state =
-      browser_state->GetOffTheRecordChromeBrowserState();
+  __block BOOL block_was_called = NO;
   const BrowsingDataRemoveMask mask = BrowsingDataRemoveMask::REMOVE_ALL;
   [removal_controller
-      removeIOSSpecificIncognitoBrowsingDataFromBrowserState:otr_browser_state
-                                                        mask:mask
-                                           completionHandler:^{
-                                             block_was_called = YES;
-                                           }];
+      removeBrowsingDataFromBrowserState:browser_state_.get()
+                                    mask:mask
+                              timePeriod:browsing_data::TimePeriod::ALL_TIME
+                       completionHandler:^{
+                         block_was_called = YES;
+                         quit_run_loop.Run();
+                       }];
+
   // Destroy the BrowserState immediately.
-  browser_state->DestroyOffTheRecordChromeBrowserState();
+  [removal_controller browserStateDestroyed:browser_state_.get()];
+  browser_state_.reset();
 
-  base::test::ios::WaitUntilCondition(^bool() {
-    return block_was_called;
-  });
+  run_loop.RunUntilIdle();
+  EXPECT_TRUE(block_was_called);
 }

ios/chrome/browser/browsing_data/ios_chrome_browsing_data_remover.h

@@ -81,6 +81,11 @@ class IOSChromeBrowsingDataRemover {
                   base::Time delete_end,
                   BrowsingDataRemoveMask mask);
 
+  // Removes the browsing data stored in WKWebsiteDataStore if needed.
+  void RemoveDataFromWKWebsiteDataStore(base::Time delete_begin,
+                                        base::Time delete_end,
+                                        BrowsingDataRemoveMask mask);
+
   // Invokes the current task callback that the removal has completed.
   void NotifyRemovalComplete();
 

ios/chrome/browser/sessions/session_service_ios.h

@@ -7,6 +7,7 @@
 
 #import <Foundation/Foundation.h>
 
+#include "base/callback.h"
 #include "base/sequenced_task_runner.h"
 
 @class SessionIOS;
@@ -44,7 +45,8 @@ using SessionIOSFactory = SessionIOS* (^)();
 - (SessionIOS*)loadSessionFromPath:(NSString*)sessionPath;
 
 // Schedules deletion of the file containing the last session in |directory|.
-- (void)deleteLastSessionFileInDirectory:(NSString*)directory;
+- (void)deleteLastSessionFileInDirectory:(NSString*)directory
+                              completion:(base::OnceClosure)callback;
 
 // Returns the path of the session file for |directory|.
 + (NSString*)sessionPathForDirectory:(NSString*)directory;

ios/chrome/browser/sessions/session_service_ios.mm

@@ -164,9 +164,10 @@ NSString* const kRootObjectKey = @"root";  // Key for the root object.
   return base::mac::ObjCCastStrict<SessionIOS>(rootObject);
 }
 
-- (void)deleteLastSessionFileInDirectory:(NSString*)directory {
+- (void)deleteLastSessionFileInDirectory:(NSString*)directory
+                              completion:(base::OnceClosure)callback {
   NSString* sessionPath = [[self class] sessionPathForDirectory:directory];
-  _taskRunner->PostTask(
+  _taskRunner->PostTaskAndReply(
       FROM_HERE, base::BindBlockArc(^{
         base::AssertBlockingAllowed();
         NSFileManager* fileManager = [NSFileManager defaultManager];
@@ -178,7 +179,8 @@ NSString* const kRootObjectKey = @"root";  // Key for the root object.
           CHECK(false) << "Unable to delete session file: "
                        << base::SysNSStringToUTF8(sessionPath) << ": "
                        << base::SysNSStringToUTF8([error description]);
-      }));
+      }),
+      std::move(callback));
 }
 
 + (NSString*)sessionPathForDirectory:(NSString*)directory {

ios/chrome/browser/sessions/session_util.h

@@ -8,6 +8,8 @@
 #include <memory>
 #include <vector>
 
+#include "base/callback.h"
+
 namespace ios {
 class ChromeBrowserState;
 }
@@ -24,8 +26,10 @@ class WebState;
 namespace session_util {
 
 // Deletes the file containing the commands for the last session. Finishes the
-// deletion even if |browser_state| is destroyed after this call.
-void DeleteLastSession(ios::ChromeBrowserState* browser_state);
+// deletion even if |browser_state| is destroyed after this call. |callback| is
+// invoked once the deletion completes.
+void DeleteLastSession(ios::ChromeBrowserState* browser_state,
+                       base::OnceClosure callback);
 
 // Create a WebState initialized with |browser_state| and serialized navigation.
 // The returned WebState has web usage enabled.

ios/chrome/browser/sessions/session_util.mm

@@ -22,11 +22,13 @@
 namespace session_util {
 
 // Deletes the file containing the commands for the last session.
-void DeleteLastSession(ios::ChromeBrowserState* browser_state) {
+void DeleteLastSession(ios::ChromeBrowserState* browser_state,
+                       base::OnceClosure callback) {
   NSString* state_path =
       base::SysUTF8ToNSString(browser_state->GetStatePath().AsUTF8Unsafe());
   [[SessionServiceIOS sharedService]
-      deleteLastSessionFileInDirectory:state_path];
+      deleteLastSessionFileInDirectory:state_path
+                            completion:std::move(callback)];
 }
 
 std::unique_ptr<web::WebState> CreateWebStateWithNavigationEntries(

ios/chrome/browser/snapshots/snapshots_util.h

@@ -7,10 +7,12 @@
 
 #include <vector>
 
+#include "base/callback.h"
 #include "base/files/file_path.h"
 
-// Clears the application snapshots taken by iOS.
-void ClearIOSSnapshots();
+// Clears the application snapshots taken by iOS and invoke |callback| when
+// the deletion has completed (asynchronously).
+void ClearIOSSnapshots(base::OnceClosure callback);
 
 // Adds to |snapshotsPaths| all the possible paths to the application's
 // snapshots taken by iOS.

ios/chrome/browser/snapshots/snapshots_util.mm

@@ -14,6 +14,7 @@
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_restrictions.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -26,41 +27,47 @@ const char* kOrientationDescriptions[] = {
     "Portrait",
     "PortraitUpsideDown",
 };
+
+// Delete all files in |paths|.
+void DeleteAllFiles(std::vector<base::FilePath> paths) {
+  base::AssertBlockingAllowed();
+  for (const auto& path : paths) {
+    ignore_result(base::DeleteFile(path, false));
+  }
+}
 }  // namespace
 
-void ClearIOSSnapshots() {
+void ClearIOSSnapshots(base::OnceClosure callback) {
   // Generates a list containing all the possible snapshot paths because the
   // list of snapshots stored on the device can't be obtained programmatically.
-  std::vector<base::FilePath> snapshotsPaths;
-  GetSnapshotsPaths(&snapshotsPaths);
-  for (base::FilePath snapshotPath : snapshotsPaths) {
-    base::PostTaskWithTraits(
-        FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
-        base::BindOnce(base::IgnoreResult(&base::DeleteFile), snapshotPath,
-                       false));
-  }
+  std::vector<base::FilePath> snapshots_paths;
+  GetSnapshotsPaths(&snapshots_paths);
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
+      base::BindOnce(&DeleteAllFiles, std::move(snapshots_paths)),
+      std::move(callback));
 }
 
-void GetSnapshotsPaths(std::vector<base::FilePath>* snapshotsPaths) {
-  DCHECK(snapshotsPaths);
-  base::FilePath snapshotsDir;
-  PathService::Get(base::DIR_CACHE, &snapshotsDir);
+void GetSnapshotsPaths(std::vector<base::FilePath>* snapshots_paths) {
+  DCHECK(snapshots_paths);
+  base::FilePath snapshots_dir;
+  PathService::Get(base::DIR_CACHE, &snapshots_dir);
   // Snapshots are located in a path with the bundle ID used twice.
-  snapshotsDir = snapshotsDir.Append("Snapshots")
-                     .Append(base::mac::BaseBundleID())
-                     .Append(base::mac::BaseBundleID());
-  const char* retinaSuffix = "";
+  snapshots_dir = snapshots_dir.Append("Snapshots")
+                      .Append(base::mac::BaseBundleID())
+                      .Append(base::mac::BaseBundleID());
+  const char* retina_suffix = "";
   CGFloat scale = [UIScreen mainScreen].scale;
   if (scale == 2) {
-    retinaSuffix = "@2x";
+    retina_suffix = "@2x";
   } else if (scale == 3) {
-    retinaSuffix = "@3x";
+    retina_suffix = "@3x";
   }
   for (unsigned int i = 0; i < arraysize(kOrientationDescriptions); i++) {
-    std::string snapshotFilename =
+    std::string snapshot_filename =
         base::StringPrintf("UIApplicationAutomaticSnapshotDefault-%s%s.png",
-                           kOrientationDescriptions[i], retinaSuffix);
-    base::FilePath snapshotPath = snapshotsDir.Append(snapshotFilename);
-    snapshotsPaths->push_back(snapshotPath);
+                           kOrientationDescriptions[i], retina_suffix);
+    base::FilePath snapshot_path = snapshots_dir.Append(snapshot_filename);
+    snapshots_paths->push_back(snapshot_path);
   }
 }

ios/chrome/browser/ui/BUILD.gn

@@ -232,6 +232,32 @@ bundle_data("resources") {
   ]
 }
 
+source_set("external_files") {
+  configs += [ "//build/config/compiler:enable_arc" ]
+  sources = [
+    "external_file_controller.h",
+    "external_file_controller.mm",
+    "external_file_remover.h",
+    "external_file_remover_factory.h",
+    "external_file_remover_factory.mm",
+    "external_file_remover_impl.h",
+    "external_file_remover_impl.mm",
+  ]
+  deps = [
+    ":ui",
+    "//base",
+    "//components/bookmarks/browser",
+    "//components/keyed_service/core",
+    "//components/keyed_service/ios",
+    "//components/sessions",
+    "//ios/chrome/browser",
+    "//ios/chrome/browser/bookmarks",
+    "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/sessions",
+    "//ios/chrome/browser/tabs",
+  ]
+}
+
 source_set("ui_internal") {
   configs += [ "//build/config/compiler:enable_arc" ]
   sources = [
@@ -245,13 +271,6 @@ source_set("ui_internal") {
     "browser_view_controller_helper.mm",
     "chrome_web_view_factory.h",
     "chrome_web_view_factory.mm",
-    "external_file_controller.h",
-    "external_file_controller.mm",
-    "external_file_remover.h",
-    "external_file_remover_factory.h",
-    "external_file_remover_factory.mm",
-    "external_file_remover_impl.h",
-    "external_file_remover_impl.mm",
     "fade_truncated_label.h",
     "fade_truncated_label.mm",
     "key_commands_provider.h",
@@ -281,7 +300,6 @@ source_set("ui_internal") {
     "//components/feature_engagement",
     "//components/image_fetcher/ios",
     "//components/infobars/core",
-    "//components/keyed_service/ios",
     "//components/language/ios/browser",
     "//components/payments/core",
     "//components/prefs",
@@ -414,6 +432,7 @@ source_set("ui_internal") {
     "//ios/chrome/browser/ui/settings",
   ]
   public_deps = [
+    ":external_files",
     "//ios/chrome/browser/ui/side_swipe",
     "//ios/chrome/browser/ui/toolbar",
   ]

ios/chrome/browser/ui/browser_list/browser_list_session_service_impl.mm

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
+#import "base/mac/bind_objc_block.h"
 #include "base/scoped_observer.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
 #import "ios/chrome/browser/sessions/session_ios.h"
@@ -246,7 +247,9 @@ bool BrowserListSessionServiceImpl::RestoreSession() {
 
 void BrowserListSessionServiceImpl::ScheduleLastSessionDeletion() {
   DCHECK(browser_list_) << "ScheduleLastSessionDeletion called after Shutdown.";
-  [session_service_ deleteLastSessionFileInDirectory:session_directory_];
+  [session_service_ deleteLastSessionFileInDirectory:session_directory_
+                                          completion:base::BindBlockArc(^{
+                                                     })];
 }
 
 void BrowserListSessionServiceImpl::ScheduleSaveSession(bool immediately) {