Schedule discarded session cleanup on the app initialization.

the didDiscardSceneSessions callback can be called when the browser
state is not yet created, or it can be called when the browsing data
remover service is not available. Once the callback is received mark
the sessions for a later time removal by saving them on a plist on disk.
And at the app initialization after the browser state its keyed services
are initialized. schedule the cleanup for these sessions.

Bug: 1136761
Change-Id: Ia5190866d4af7a041f0e5507fb7a88ea0ace9901
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490788
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820022}

ios/chrome/app/application_delegate/app_state.mm

@@ -28,8 +28,7 @@
 #import "ios/chrome/app/main_application_delegate.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#include "ios/chrome/browser/browsing_data/browsing_data_remover.h"
-#include "ios/chrome/browser/browsing_data/browsing_data_remover_factory.h"
+#import "ios/chrome/browser/browsing_data/sessions_storage_util.h"
 #include "ios/chrome/browser/chrome_constants.h"
 #include "ios/chrome/browser/crash_report/breakpad_helper.h"
 #include "ios/chrome/browser/crash_report/crash_keys_helper.h"
@@ -460,14 +459,7 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
   for (UISceneSession* session in sceneSessions) {
     [sessionIDs addObject:session.persistentIdentifier];
   }
-  ChromeBrowserState* browserState =
-      _browserLauncher.interfaceProvider.mainInterface.browserState;
-  BrowsingDataRemoverFactory::GetForBrowserState(browserState)
-      ->RemoveSessionsData(sessionIDs);
-  ChromeBrowserState* incognitoBrowserState =
-      _browserLauncher.interfaceProvider.incognitoInterface.browserState;
-  BrowsingDataRemoverFactory::GetForBrowserState(incognitoBrowserState)
-      ->RemoveSessionsData(sessionIDs);
+  sessions_storage_util::MarkSessionsForRemoval(sessionIDs);
 }
 
 - (void)willResignActiveTabModel {

ios/chrome/app/main_controller.mm

@@ -46,6 +46,7 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h"
 #include "ios/chrome/browser/browsing_data/browsing_data_remover.h"
 #include "ios/chrome/browser/browsing_data/browsing_data_remover_factory.h"
+#import "ios/chrome/browser/browsing_data/sessions_storage_util.h"
 #include "ios/chrome/browser/chrome_paths.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
@@ -124,6 +125,9 @@ NSString* const kStartupAttemptReset = @"StartupAttempReset";
 // Constants for deferring memory debugging tools startup.
 NSString* const kMemoryDebuggingToolsStartup = @"MemoryDebuggingToolsStartup";
 
+// Constant for deferring the cleanup of discarded sessions on disk.
+NSString* const kCleanupDiscardedSessions = @"CleanupDiscardedSessions";
+
 // Constants for deferring mailto handling initialization.
 NSString* const kMailtoHandlingInitialization = @"MailtoHandlingInitialization";
 
@@ -273,6 +277,8 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
 - (bool)mustShowRestoreInfobar;
 // Cleanup snapshots on disk.
 - (void)cleanupSnapshots;
+// Cleanup discarded sessions on disk.
+- (void)cleanupDiscardedSessions;
 // Sets a LocalState pref marking the TOS EULA as accepted.
 - (void)markEulaAsAccepted;
 // Sends any feedback that happens to still be on local storage.
@@ -296,6 +302,8 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
 - (void)scheduleStartupAttemptReset;
 // Asynchronously schedules the cleanup of crash reports.
 - (void)scheduleCrashReportCleanup;
+// Asynchronously schedules the cleanup of discarded session files on disk.
+- (void)scheduleDiscardedSessionsCleanup;
 // Asynchronously schedules the cleanup of snapshots on disk.
 - (void)scheduleSnapshotsCleanup;
 // Schedules various cleanup tasks that are performed after launch.
@@ -832,6 +840,14 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
                   }];
 }
 
+- (void)scheduleDiscardedSessionsCleanup {
+  [[DeferredInitializationRunner sharedInstance]
+      enqueueBlockNamed:kCleanupDiscardedSessions
+                  block:^{
+                    [self cleanupDiscardedSessions];
+                  }];
+}
+
 - (void)scheduleSnapshotsCleanup {
   [[DeferredInitializationRunner sharedInstance]
       enqueueBlockNamed:kCleanupSnapshots
@@ -856,6 +872,9 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
     }
   }
 
+  // Remove all discarded sessions from disk.
+  [self scheduleDiscardedSessionsCleanup];
+
   // If the user chooses to restore their session, some cached snapshots may
   // be needed. Otherwise, cleanup the snapshots.
   if (![self mustShowRestoreInfobar]) {
@@ -1134,6 +1153,20 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
   }
 }
 
+- (void)cleanupDiscardedSessions {
+  NSArray<NSString*>* sessionIDs =
+      sessions_storage_util::GetDiscardedSessions();
+  if (!sessionIDs)
+    return;
+  BrowsingDataRemoverFactory::GetForBrowserState(
+      self.appState.mainBrowserState->GetOriginalChromeBrowserState())
+      ->RemoveSessionsData(sessionIDs);
+  BrowsingDataRemoverFactory::GetForBrowserState(
+      self.appState.mainBrowserState->GetOffTheRecordChromeBrowserState())
+      ->RemoveSessionsData(sessionIDs);
+  sessions_storage_util::ResetDiscardedSessions();
+}
+
 - (void)markEulaAsAccepted {
   PrefService* prefs = GetApplicationContext()->GetLocalState();
   if (!prefs->GetBoolean(prefs::kEulaAccepted))

ios/chrome/browser/browsing_data/BUILD.gn

@@ -29,6 +29,8 @@ source_set("browsing_data") {
     "browsing_data_remover_observer.h",
     "browsing_data_remover_observer_bridge.h",
     "browsing_data_remover_observer_bridge.mm",
+    "sessions_storage_util.h",
+    "sessions_storage_util.mm",
   ]
   deps = [
     ":counters",
@@ -138,11 +140,13 @@ source_set("test_support") {
     "fake_browsing_data_remover.mm",
     "fake_browsing_data_remover_observer.h",
     "fake_browsing_data_remover_observer.mm",
+    "session_storage_util_unittest.mm",
   ]
 
   deps = [
     ":browsing_data",
     "//base",
+    "//testing/gtest",
   ]
 
   configs += [ "//build/config/compiler:enable_arc" ]

ios/chrome/browser/browsing_data/session_storage_util_unittest.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/browsing_data/sessions_storage_util.h"
+
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+// Test fixture for the session storage utility functions.
+using SessionStorageUtilTest = PlatformTest;
+
+// Tests |MarkSessionsForRemoval| adds to the existing list of sessions that are
+// marked for removal.
+TEST_F(SessionStorageUtilTest, MarkSessionRemovalTest) {
+  EXPECT_FALSE(sessions_storage_util::GetDiscardedSessions());
+  NSArray<NSString*>* session_ids_1 =
+      @[ @"session_1", @"session_2", @"session_3" ];
+  sessions_storage_util::MarkSessionsForRemoval(session_ids_1);
+  EXPECT_EQ(3u, sessions_storage_util::GetDiscardedSessions().count);
+  // Make sure that adding new sessions to the list doesn't overwrite existing
+  // sessions.
+  NSArray<NSString*>* session_ids_2 = @[ @"session_4", @"session_5" ];
+  sessions_storage_util::MarkSessionsForRemoval(session_ids_2);
+  EXPECT_EQ(5u, sessions_storage_util::GetDiscardedSessions().count);
+  sessions_storage_util::ResetDiscardedSessions();
+}
+
+// Makes sure that |ResetDiscardedSessions| removes the list of sessions that
+// are marked for removal.
+TEST_F(SessionStorageUtilTest, ResetDiscardedSessionsTest) {
+  EXPECT_FALSE(sessions_storage_util::GetDiscardedSessions());
+  NSArray<NSString*>* session_ids_1 =
+      @[ @"session_1", @"session_2", @"session_3" ];
+  sessions_storage_util::MarkSessionsForRemoval(session_ids_1);
+  EXPECT_EQ(3u, sessions_storage_util::GetDiscardedSessions().count);
+  sessions_storage_util::ResetDiscardedSessions();
+  EXPECT_FALSE(sessions_storage_util::GetDiscardedSessions());
+  // Make sure that re-adding sessions work after reseting the sessions list.
+  NSArray<NSString*>* session_ids_2 = @[ @"session_4", @"session_5" ];
+  sessions_storage_util::MarkSessionsForRemoval(session_ids_2);
+  EXPECT_EQ(2u, sessions_storage_util::GetDiscardedSessions().count);
+  sessions_storage_util::ResetDiscardedSessions();
+  EXPECT_FALSE(sessions_storage_util::GetDiscardedSessions());
+}

ios/chrome/browser/browsing_data/sessions_storage_util.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_BROWSING_DATA_SESSIONS_STORAGE_UTIL_H_
+#define IOS_CHROME_BROWSER_BROWSING_DATA_SESSIONS_STORAGE_UTIL_H_
+
+#import <Foundation/Foundation.h>
+
+namespace sessions_storage_util {
+
+// Mark the sessions with |session_ids| for their files to be removed from the
+// disk at some point later.
+void MarkSessionsForRemoval(NSArray<NSString*>* session_ids);
+// Get the list of session ids for the sessions that was marked for removal.
+NSArray<NSString*>* GetDiscardedSessions();
+// Empties the list of sessions that are marked for removal.
+void ResetDiscardedSessions();
+
+}  // namespace sessions_storage_util
+
+#endif  // IOS_CHROME_BROWSER_BROWSING_DATA_SESSIONS_STORAGE_UTIL_H_

ios/chrome/browser/browsing_data/sessions_storage_util.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/browsing_data/sessions_storage_util.h"
+
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "base/strings/sys_string_conversions.h"
+#include "ios/chrome/browser/chrome_paths.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+
+// Returns the path of the list of sessions that are marked for removal.
+NSString* GetDiscardedSessionsFilePath() {
+  base::FilePath directory_path;
+  base::PathService::Get(ios::DIR_USER_DATA, &directory_path);
+  NSString* file_path = base::SysUTF8ToNSString(
+      directory_path.Append(FILE_PATH_LITERAL("DiscardedSessions"))
+          .AsUTF8Unsafe());
+  return file_path;
+}
+
+}  // namespace
+
+namespace sessions_storage_util {
+
+void MarkSessionsForRemoval(NSArray<NSString*>* session_ids) {
+  NSString* file_path = GetDiscardedSessionsFilePath();
+  NSMutableArray* sessions = [NSMutableArray arrayWithContentsOfFile:file_path];
+  if (!sessions)
+    sessions = [[NSMutableArray alloc] init];
+  [sessions addObjectsFromArray:session_ids];
+  [sessions writeToFile:file_path atomically:YES];
+}
+
+NSArray<NSString*>* GetDiscardedSessions() {
+  return [NSArray arrayWithContentsOfFile:GetDiscardedSessionsFilePath()];
+}
+
+void ResetDiscardedSessions() {
+  NSFileManager* file_manager = [[NSFileManager alloc] init];
+  [file_manager removeItemAtPath:GetDiscardedSessionsFilePath() error:nil];
+}
+
+}  // namespace sessions_storage_util