Commit 0f8fb861 authored by Mohammad Refaat's avatar Mohammad Refaat Committed by Commit Bot

Remove sessions from disk when they are discarded by the OS

Right now sessions data on disk are never deleted. Instead delete them
whenever the app receives a signal that this session can no longer be
brought back via "didDiscardSceneSessions" appdelegate.

This is done by adding a new method to BrowsingDataCommands protocol
and allowing the mainController to conform to it.

Bug: 1114847
Change-Id: I767f75c996fa8554943c09f84c9c637851beee32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347248
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797908}
parent 2370aba4
...@@ -177,6 +177,7 @@ source_set("application_delegate_internal") { ...@@ -177,6 +177,7 @@ source_set("application_delegate_internal") {
"//ios/chrome/app/startup", "//ios/chrome/app/startup",
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/browsing_data",
"//ios/chrome/browser/crash_report", "//ios/chrome/browser/crash_report",
"//ios/chrome/browser/device_sharing", "//ios/chrome/browser/device_sharing",
"//ios/chrome/browser/feature_engagement", "//ios/chrome/browser/feature_engagement",
......
...@@ -98,6 +98,12 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher ...@@ -98,6 +98,12 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
// mainChrome instance. // mainChrome instance.
- (void)applicationWillTerminate:(UIApplication*)application; - (void)applicationWillTerminate:(UIApplication*)application;
// Called when the application discards set of scene sessions, these sessions
// can no longer be accessed and all their associated data should be destroyed.
- (void)application:(UIApplication*)application
didDiscardSceneSessions:(NSSet<UISceneSession*>*)sceneSessions
API_AVAILABLE(ios(13));
// Resumes the session: reinitializing metrics and opening new tab if necessary. // Resumes the session: reinitializing metrics and opening new tab if necessary.
// User sessions are defined in terms of BecomeActive/ResignActive so that // User sessions are defined in terms of BecomeActive/ResignActive so that
// session boundaries include things like turning the screen off or getting a // session boundaries include things like turning the screen off or getting a
......
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
#import "ios/chrome/app/main_application_delegate.h" #import "ios/chrome/app/main_application_delegate.h"
#include "ios/chrome/browser/application_context.h" #include "ios/chrome/browser/application_context.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.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"
#include "ios/chrome/browser/chrome_constants.h" #include "ios/chrome/browser/chrome_constants.h"
#include "ios/chrome/browser/crash_report/breakpad_helper.h" #include "ios/chrome/browser/crash_report/breakpad_helper.h"
#include "ios/chrome/browser/crash_report/crash_keys_helper.h" #include "ios/chrome/browser/crash_report/crash_keys_helper.h"
...@@ -468,6 +470,24 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher ...@@ -468,6 +470,24 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
[_startupInformation stopChromeMain]; [_startupInformation stopChromeMain];
} }
- (void)application:(UIApplication*)application
didDiscardSceneSessions:(NSSet<UISceneSession*>*)sceneSessions
API_AVAILABLE(ios(13)) {
NSMutableArray<NSString*>* sessionIDs =
[NSMutableArray arrayWithCapacity:sceneSessions.count];
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);
}
- (void)willResignActiveTabModel { - (void)willResignActiveTabModel {
if ([_browserLauncher browserInitializationStage] < if ([_browserLauncher browserInitializationStage] <
INITIALIZATION_STAGE_FOREGROUND) { INITIALIZATION_STAGE_FOREGROUND) {
......
...@@ -226,6 +226,7 @@ ...@@ -226,6 +226,7 @@
ios::GetChromeBrowserProvider() ios::GetChromeBrowserProvider()
->GetChromeIdentityService() ->GetChromeIdentityService()
->ApplicationDidDiscardSceneSessions(sceneSessions); ->ApplicationDidDiscardSceneSessions(sceneSessions);
[_appState application:application didDiscardSceneSessions:sceneSessions];
} }
#endif // BUILDFLAG(IOS_MULTIWINDOW_ENABLED) #endif // BUILDFLAG(IOS_MULTIWINDOW_ENABLED)
......
...@@ -20,8 +20,8 @@ source_set("browsing_data") { ...@@ -20,8 +20,8 @@ source_set("browsing_data") {
sources = [ sources = [
"browsing_data_counter_wrapper.cc", "browsing_data_counter_wrapper.cc",
"browsing_data_counter_wrapper.h", "browsing_data_counter_wrapper.h",
"browsing_data_remover.cc",
"browsing_data_remover.h", "browsing_data_remover.h",
"browsing_data_remover.mm",
"browsing_data_remover_factory.h", "browsing_data_remover_factory.h",
"browsing_data_remover_factory.mm", "browsing_data_remover_factory.mm",
"browsing_data_remover_impl.h", "browsing_data_remover_impl.h",
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef IOS_CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_H_ #ifndef IOS_CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_H_
#define IOS_CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_H_ #define IOS_CHROME_BROWSER_BROWSING_DATA_BROWSING_DATA_REMOVER_H_
#import <Foundation/Foundation.h>
#include "base/callback.h" #include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/observer_list.h" #include "base/observer_list.h"
...@@ -31,6 +33,9 @@ class BrowsingDataRemover : public KeyedService { ...@@ -31,6 +33,9 @@ class BrowsingDataRemover : public KeyedService {
BrowsingDataRemoveMask remove_mask, BrowsingDataRemoveMask remove_mask,
base::OnceClosure callback) = 0; base::OnceClosure callback) = 0;
// Removes all persisted data for sessions with |session_ids|.
virtual void RemoveSessionsData(NSArray<NSString*>* session_ids) = 0;
// Adds/removes |observer| from the list of observers notified when data is // Adds/removes |observer| from the list of observers notified when data is
// removed by BrowsingDataRemover. // removed by BrowsingDataRemover.
void AddObserver(BrowsingDataRemoverObserver* observer); void AddObserver(BrowsingDataRemoverObserver* observer);
......
...@@ -2,10 +2,14 @@ ...@@ -2,10 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "ios/chrome/browser/browsing_data/browsing_data_remover.h" #import "ios/chrome/browser/browsing_data/browsing_data_remover.h"
#include "ios/chrome/browser/browsing_data/browsing_data_remover_observer.h" #include "ios/chrome/browser/browsing_data/browsing_data_remover_observer.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
BrowsingDataRemover::BrowsingDataRemover() = default; BrowsingDataRemover::BrowsingDataRemover() = default;
BrowsingDataRemover::~BrowsingDataRemover() = default; BrowsingDataRemover::~BrowsingDataRemover() = default;
......
...@@ -46,6 +46,7 @@ class BrowsingDataRemoverImpl : public BrowsingDataRemover { ...@@ -46,6 +46,7 @@ class BrowsingDataRemoverImpl : public BrowsingDataRemover {
void Remove(browsing_data::TimePeriod time_period, void Remove(browsing_data::TimePeriod time_period,
BrowsingDataRemoveMask remove_mask, BrowsingDataRemoveMask remove_mask,
base::OnceClosure callback) override; base::OnceClosure callback) override;
void RemoveSessionsData(NSArray<NSString*>* session_ids) override;
private: private:
// Represents a single removal task. Contains all parameters to execute it. // Represents a single removal task. Contains all parameters to execute it.
......
...@@ -557,6 +557,16 @@ void BrowsingDataRemoverImpl::RemoveImpl(base::Time delete_begin, ...@@ -557,6 +557,16 @@ void BrowsingDataRemoverImpl::RemoveImpl(base::Time delete_begin,
MAX_CHOICE_VALUE); MAX_CHOICE_VALUE);
} }
// Removes directories for sessions with |SessionIDs|
void BrowsingDataRemoverImpl::RemoveSessionsData(
NSArray<NSString*>* session_ids) {
[[SessionServiceIOS sharedService]
deleteSessions:session_ids
fromBrowserStateDirectory:base::SysUTF8ToNSString(
browser_state_->GetStatePath()
.AsUTF8Unsafe())];
}
// TODO(crbug.com/619783): removing data from WkWebsiteDataStore should be // TODO(crbug.com/619783): removing data from WkWebsiteDataStore should be
// implemented by //ios/web. Once this is available remove this and use the // implemented by //ios/web. Once this is available remove this and use the
// new API. // new API.
......
...@@ -17,6 +17,7 @@ class FakeBrowsingDataRemover : public BrowsingDataRemover { ...@@ -17,6 +17,7 @@ class FakeBrowsingDataRemover : public BrowsingDataRemover {
void Remove(browsing_data::TimePeriod time_period, void Remove(browsing_data::TimePeriod time_period,
BrowsingDataRemoveMask remove_mask, BrowsingDataRemoveMask remove_mask,
base::OnceClosure callback) override; base::OnceClosure callback) override;
void RemoveSessionsData(NSArray<NSString*>* session_ids) override;
}; };
#endif // IOS_CHROME_BROWSER_BROWSING_DATA_FAKE_BROWSING_DATA_REMOVER_H_ #endif // IOS_CHROME_BROWSER_BROWSING_DATA_FAKE_BROWSING_DATA_REMOVER_H_
...@@ -15,3 +15,6 @@ bool FakeBrowsingDataRemover::IsRemoving() const { ...@@ -15,3 +15,6 @@ bool FakeBrowsingDataRemover::IsRemoving() const {
void FakeBrowsingDataRemover::Remove(browsing_data::TimePeriod time_period, void FakeBrowsingDataRemover::Remove(browsing_data::TimePeriod time_period,
BrowsingDataRemoveMask remove_mask, BrowsingDataRemoveMask remove_mask,
base::OnceClosure callback) {} base::OnceClosure callback) {}
void FakeBrowsingDataRemover::RemoveSessionsData(
NSArray<NSString*>* session_ids) {}
...@@ -47,6 +47,11 @@ ...@@ -47,6 +47,11 @@
- (void)deleteLastSessionFileInDirectory:(NSString*)directory - (void)deleteLastSessionFileInDirectory:(NSString*)directory
completion:(base::OnceClosure)callback; completion:(base::OnceClosure)callback;
// Schedule deletion of session directories with |sessionIDs| which resides in
// a specific browser state |directory|.
- (void)deleteSessions:(NSArray<NSString*>*)sessionIDs
fromBrowserStateDirectory:(NSString*)directory;
// Returns the path of the session file for |directory|. // Returns the path of the session file for |directory|.
+ (NSString*)sessionPathForDirectory:(NSString*)directory; + (NSString*)sessionPathForDirectory:(NSString*)directory;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/format_macros.h" #include "base/format_macros.h"
#include "base/location.h" #include "base/location.h"
...@@ -44,6 +45,9 @@ ...@@ -44,6 +45,9 @@
namespace { namespace {
const NSTimeInterval kSaveDelay = 2.5; // Value taken from Desktop Chrome. const NSTimeInterval kSaveDelay = 2.5; // Value taken from Desktop Chrome.
NSString* const kRootObjectKey = @"root"; // Key for the root object. NSString* const kRootObjectKey = @"root"; // Key for the root object.
NSString* const kSessionDirectory =
@"Sessions"; // The directory name inside BrowserState directory which
// contain all sessions directories.
} }
@implementation NSKeyedUnarchiver (CrLegacySessionCompatibility) @implementation NSKeyedUnarchiver (CrLegacySessionCompatibility)
...@@ -168,30 +172,52 @@ NSString* const kRootObjectKey = @"root"; // Key for the root object. ...@@ -168,30 +172,52 @@ NSString* const kRootObjectKey = @"root"; // Key for the root object.
- (void)deleteLastSessionFileInDirectory:(NSString*)directory - (void)deleteLastSessionFileInDirectory:(NSString*)directory
completion:(base::OnceClosure)callback { completion:(base::OnceClosure)callback {
NSString* sessionPath = [[self class] sessionPathForDirectory:directory]; NSString* sessionPath = [[self class] sessionPathForDirectory:directory];
[self deletePaths:[NSArray arrayWithObject:sessionPath]
completion:std::move(callback)];
}
- (void)deleteSessions:(NSArray<NSString*>*)sessionIDs
fromBrowserStateDirectory:(NSString*)directory {
NSString* sessionsDirectoryPath =
[directory stringByAppendingPathComponent:kSessionDirectory];
NSMutableArray<NSString*>* paths =
[NSMutableArray arrayWithCapacity:sessionIDs.count];
for (NSString* sessionID : sessionIDs) {
[paths addObject:[sessionsDirectoryPath
stringByAppendingPathComponent:sessionID]];
}
[self deletePaths:paths completion:base::DoNothing()];
}
+ (NSString*)sessionPathForDirectory:(NSString*)directory {
return [directory stringByAppendingPathComponent:@"session.plist"];
}
#pragma mark - Private methods
// Delete files/folders of the given |paths|.
- (void)deletePaths:(NSArray<NSString*>*)paths
completion:(base::OnceClosure)callback {
_taskRunner->PostTaskAndReply( _taskRunner->PostTaskAndReply(
FROM_HERE, base::BindOnce(^{ FROM_HERE, base::BindOnce(^{
base::ScopedBlockingCall scoped_blocking_call( base::ScopedBlockingCall scoped_blocking_call(
FROM_HERE, base::BlockingType::MAY_BLOCK); FROM_HERE, base::BlockingType::MAY_BLOCK);
NSFileManager* fileManager = [NSFileManager defaultManager]; NSFileManager* fileManager = [NSFileManager defaultManager];
if (![fileManager fileExistsAtPath:sessionPath]) for (NSString* path : paths) {
return; if (![fileManager fileExistsAtPath:path])
continue;
NSError* error = nil;
if (![fileManager removeItemAtPath:sessionPath error:&error] || error) { NSError* error = nil;
CHECK(false) << "Unable to delete session file: " if (![fileManager removeItemAtPath:path error:&error] || error) {
<< base::SysNSStringToUTF8(sessionPath) << ": " CHECK(false) << "Unable to delete path: "
<< base::SysNSStringToUTF8([error description]); << base::SysNSStringToUTF8(path) << ": "
<< base::SysNSStringToUTF8([error description]);
}
} }
}), }),
std::move(callback)); std::move(callback));
} }
+ (NSString*)sessionPathForDirectory:(NSString*)directory {
return [directory stringByAppendingPathComponent:@"session.plist"];
}
#pragma mark - Private methods
// Do the work of saving on a background thread. // Do the work of saving on a background thread.
- (void)performSaveToPathInBackground:(NSString*)sessionPath { - (void)performSaveToPathInBackground:(NSString*)sessionPath {
DCHECK(sessionPath); DCHECK(sessionPath);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment