Commit 81a6856f authored by Mohammad Refaat's avatar Mohammad Refaat Committed by Commit Bot

Reland "Add Crash Restoration support for multi-window"

This reverts commit 55996955.

Reason for revert: Fixed the crash, which was caused by deleting
the temp folder.

Original change's description:
> Revert "Add Crash Restoration support for multi-window"
>
> This reverts commit 64875586.
>
> Reason for revert: Speculatively reverting due to https://crbug.com/1132229.
>
> Original change's description:
> > Add Crash Restoration support for multi-window
> >
> > After a crash info bar will be presented on all foreground scenes
> > if the user choose to restore all foreground windows will be restored.
> > if not all windows tabs for all windows which were opened on the last
> > run will be added to the recent closed tabs.
> >
> > Bug: 1126311, 1114179
> > Change-Id: I80623012dc132e2702004d2dbc0bbcb410a7d91c
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2415929
> > Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
> > Reviewed-by: Mark Cogan <marq@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#810406}
>
> TBR=marq@chromium.org,mrefaat@chromium.org
>
> Change-Id: I4045617e7eb138e0212f8b1d1062df7ce0b45f74
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1126311
> Bug: 1114179
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428972
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#810536}

TBR=marq@chromium.org,mrefaat@chromium.org,jdoerrie@chromium.org


Bug: 1126311, 1132229
Bug: 1114179
Change-Id: I911b88c3b57b54aff678648d2da7a6b03038610f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431454Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarMohammad Refaat <mrefaat@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811813}
parent dda72628
......@@ -469,8 +469,16 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
// browser state.
BOOL needRestoration = NO;
if (isPostCrashLaunch) {
needRestoration = [CrashRestoreHelper
moveAsideSessionInformationForBrowserState:chromeBrowserState];
if (IsMultiwindowSupported()) {
NSSet<NSString*>* sessions =
[[PreviousSessionInfo sharedInstance] connectedSceneSessionsIDs];
needRestoration =
[CrashRestoreHelper moveAsideSessions:sessions
forBrowserState:chromeBrowserState];
} else {
needRestoration = [CrashRestoreHelper
moveAsideSessionInformationForBrowserState:chromeBrowserState];
}
}
[[PreviousSessionInfo sharedInstance] resetConnectedSceneSessionIDs];
......
......@@ -75,6 +75,7 @@ source_set("crash_report_internal") {
"//ios/chrome/browser/sessions:serialisation",
"//ios/chrome/browser/sessions:session_service",
"//ios/chrome/browser/ui/infobars:feature_flags",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/util:multiwindow_util",
"//ios/chrome/browser/web:tab_id_tab_helper",
"//ios/chrome/browser/web_state_list",
......@@ -108,6 +109,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/metrics:previous_session_info",
"//ios/chrome/browser/sessions:serialisation",
"//ios/chrome/browser/sessions:session_service",
"//ios/chrome/browser/ui/util:multiwindow_util",
"//ios/chrome/browser/web_state_list:test_support",
"//ios/chrome/browser/web_state_list:web_state_list",
"//ios/chrome/test/ocmock",
......
......@@ -15,12 +15,21 @@ class ChromeBrowserState;
- (instancetype)initWithBrowser:(Browser*)browser;
// Saves the session information stored on disk in temporary files and will
// then delete those from their default location. This will ensure that the
// user will then start from scratch, while allowing restoring their old
// sessions. This method has to be called before the browser is created, or the
// session information will be overwritten.
// Returns |YES| if the deletetion and backup was successful.
// Saves the session information stored on disk for sessions with |sessionIDs|
// in temporary files and will then delete those from their default location.
// This will ensure that the user will then start from scratch, while allowing
// restoring their old sessions. This method has to be called before the browser
// is created, or the session information will be overwritten.
// |sessionIDs| can be nil when multiple windows are not supported, and in that
// case only the default session will be moved.
// Returns |YES| if the at least one session deletion was successful.
+ (BOOL)moveAsideSessions:(NSSet<NSString*>*)sessionIDs
forBrowserState:(ChromeBrowserState*)browserState;
// Move the session information for Legacy non multiwindow supported OS.
// This method deletes the session from its default location, while
// allowing restoring it back later.
// Returns |YES| if the delettion and backup was successful.
+ (BOOL)moveAsideSessionInformationForBrowserState:
(ChromeBrowserState*)browserState;
......
......@@ -14,6 +14,7 @@
#include "ios/chrome/browser/crash_report/crash_restore_helper.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/sessions/session_service_ios.h"
#import "ios/chrome/browser/ui/util/multi_window_support.h"
#include "ios/web/public/test/web_task_environment.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -28,7 +29,7 @@
using testing::Return;
@interface CrashRestoreHelper (Test)
+ (NSString*)sessionBackupPath;
+ (NSString*)backupPathForSessionID:(NSString*)sessionID;
@end
namespace {
......@@ -45,6 +46,73 @@ class CrashRestoreHelperTest : public PlatformTest {
}
protected:
// Creates the session for |session_id|, if |session_id| is nil a session
// will be created in the default location.
// Returns |true| if the creation was successful.
bool CreateSession(NSString* session_id) {
NSFileManager* file_manager = [NSFileManager defaultManager];
ChromeBrowserState* browser_states[] = {
chrome_browser_state_.get(),
off_the_record_chrome_browser_state_,
};
NSString* backup_path =
[CrashRestoreHelper backupPathForSessionID:session_id];
[file_manager removeItemAtPath:backup_path error:nil];
NSData* data = [NSData dataWithBytes:"hello" length:5];
for (size_t index = 0; index < base::size(browser_states); ++index) {
NSString* state_path = base::SysUTF8ToNSString(
browser_states[index]->GetStatePath().value());
NSString* session_path =
[SessionServiceIOS sessionPathForSessionID:session_id
directory:state_path];
NSString* directory = [session_path stringByDeletingLastPathComponent];
if (![file_manager fileExistsAtPath:directory]) {
[file_manager createDirectoryAtPath:directory
withIntermediateDirectories:YES
attributes:nil
error:nil];
}
[file_manager createFileAtPath:session_path contents:data attributes:nil];
if (![file_manager fileExistsAtPath:session_path])
return false;
}
return true;
}
// Returns |true| if session for |session_id| was erased from its default
// location. if |session_id| is nil, the default session location is used.
bool IsSessionErased(NSString* session_id) {
NSFileManager* file_manager = [NSFileManager defaultManager];
ChromeBrowserState* browser_states[] = {
chrome_browser_state_.get(),
off_the_record_chrome_browser_state_,
};
for (size_t index = 0; index < base::size(browser_states); ++index) {
NSString* state_path = base::SysUTF8ToNSString(
browser_states[index]->GetStatePath().value());
NSString* session_path =
[SessionServiceIOS sessionPathForSessionID:session_id
directory:state_path];
if ([file_manager fileExistsAtPath:session_path])
return false;
}
return true;
}
// Returns |true| if the session with |session_id| was backed up correctly,
// and deletes the backup file. if |session_id| is nil, the default backup
// session location is used.
bool CheckAndDeleteSessionBackedUp(NSString* session_id) {
NSFileManager* file_manager = [NSFileManager defaultManager];
NSString* backup_path =
[CrashRestoreHelper backupPathForSessionID:session_id];
if (![file_manager fileExistsAtPath:backup_path])
return false;
[file_manager removeItemAtPath:backup_path error:nil];
return true;
}
web::WebTaskEnvironment task_environment_;
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
std::unique_ptr<TestBrowser> test_browser_;
......@@ -52,39 +120,33 @@ class CrashRestoreHelperTest : public PlatformTest {
CrashRestoreHelper* helper_;
};
TEST_F(CrashRestoreHelperTest, MoveAsideTest) {
NSString* backup_path = [CrashRestoreHelper sessionBackupPath];
NSFileManager* file_manager = [NSFileManager defaultManager];
[file_manager removeItemAtPath:backup_path error:NULL];
NSData* data = [NSData dataWithBytes:"hello" length:5];
ChromeBrowserState* browser_states[] = {
chrome_browser_state_.get(),
off_the_record_chrome_browser_state_,
};
for (size_t index = 0; index < base::size(browser_states); ++index) {
NSString* state_path =
base::SysUTF8ToNSString(browser_states[index]->GetStatePath().value());
NSString* session_path =
[SessionServiceIOS sessionPathForDirectory:state_path];
[file_manager createFileAtPath:session_path contents:data attributes:nil];
ASSERT_EQ(YES, [file_manager fileExistsAtPath:session_path]);
}
// Tests that moving session work correctly when multiple windows are not
// supported.
TEST_F(CrashRestoreHelperTest, MoveAsideSingleSession) {
// This test is only enabled when multi-window is disabled.
if (IsMultiwindowSupported())
return;
ASSERT_TRUE(CreateSession(nil));
[CrashRestoreHelper
moveAsideSessionInformationForBrowserState:chrome_browser_state_.get()];
EXPECT_TRUE(IsSessionErased(nil));
EXPECT_EQ(YES, CheckAndDeleteSessionBackedUp(nil));
}
for (size_t index = 0; index < base::size(browser_states); ++index) {
NSString* state_path =
base::SysUTF8ToNSString(browser_states[index]->GetStatePath().value());
NSString* session_path =
[SessionServiceIOS sessionPathForDirectory:state_path];
EXPECT_EQ(NO, [file_manager fileExistsAtPath:session_path]);
// Tests that moving session work correctly when multiple windows are supported.
TEST_F(CrashRestoreHelperTest, MoveAsideMultipleSessions) {
NSSet<NSString*>* session_ids =
[NSSet setWithObjects:@"session_1", @"session_2", nil];
for (NSString* session_id in session_ids) {
ASSERT_TRUE(CreateSession(session_id));
}
EXPECT_EQ(YES, [file_manager fileExistsAtPath:backup_path]);
[file_manager removeItemAtPath:backup_path error:NULL];
[CrashRestoreHelper moveAsideSessions:session_ids
forBrowserState:chrome_browser_state_.get()];
for (NSString* session_id in session_ids) {
EXPECT_TRUE(IsSessionErased(session_id));
EXPECT_EQ(YES, CheckAndDeleteSessionBackedUp(session_id));
}
}
} // namespace
......@@ -112,6 +112,11 @@ enum class DeviceBatteryState {
// Reset to NO after resetSessionRestorationFlag call.
@property(nonatomic, readonly) BOOL terminatedDuringSessionRestoration;
// The list of the session IDs for all the connected scenes, used for crash
// restoration.
@property(nonatomic, readonly)
NSMutableSet<NSString*>* connectedSceneSessionsIDs;
// Singleton PreviousSessionInfo. During the lifetime of the app, the returned
// object is the same, and describes the previous session, even after a new
// session has started (by calling beginRecordingCurrentSession).
......
......@@ -116,10 +116,6 @@ NSString* const kPreviousSessionInfoConnectedSceneSessionIDs =
// Can be greater than one if multiple sessions are being restored in parallel.
@property(atomic, assign) int numberOfSessionsBeingRestored;
// The list of the session IDs for all the connected scenes, used for crash
// restoration.
@property(nonatomic, strong) NSMutableSet<NSString*>* connectedSceneSessionsIDs;
// Redefined to be read-write.
@property(nonatomic, assign) NSInteger availableDeviceStorage;
@property(nonatomic, assign) float deviceBatteryLevel;
......@@ -134,6 +130,7 @@ NSString* const kPreviousSessionInfoConnectedSceneSessionIDs =
@property(nonatomic, strong) NSString* OSVersion;
@property(nonatomic, strong) NSDate* sessionEndTime;
@property(nonatomic, assign) BOOL terminatedDuringSessionRestoration;
@property(nonatomic, strong) NSMutableSet<NSString*>* connectedSceneSessionsIDs;
@end
......
......@@ -13,6 +13,10 @@
@class SessionIOS;
@class SessionIOSFactory;
namespace session_constants {
NSString* const kSessionsDirectory = @"Sessions";
}
// A singleton service for saving the current session. Can either save on a
// delay or immediately. Saving is always performed on a separate thread.
@interface SessionServiceIOS : NSObject
......@@ -52,6 +56,10 @@
- (void)deleteSessions:(NSArray<NSString*>*)sessionIDs
fromBrowserStateDirectory:(NSString*)directory;
// Returns the path of the session with |sessionID| within a |directory|.
+ (NSString*)sessionPathForSessionID:(NSString*)sessionID
directory:(NSString*)directory;
// Returns the path of the session file for |directory|.
+ (NSString*)sessionPathForDirectory:(NSString*)directory;
......
......@@ -48,6 +48,8 @@ NSString* const kRootObjectKey = @"root"; // Key for the root object.
NSString* const kSessionDirectory =
@"Sessions"; // The directory name inside BrowserState directory which
// contain all sessions directories.
NSString* const kSessionFileName =
@"session.plist"; // The session file name on disk.
}
@implementation NSKeyedUnarchiver (CrLegacySessionCompatibility)
......@@ -190,7 +192,16 @@ NSString* const kSessionDirectory =
}
+ (NSString*)sessionPathForDirectory:(NSString*)directory {
return [directory stringByAppendingPathComponent:@"session.plist"];
return [directory stringByAppendingPathComponent:kSessionFileName];
}
+ (NSString*)sessionPathForSessionID:(NSString*)sessionID
directory:(NSString*)directory {
if (!sessionID)
return [[self class] sessionPathForDirectory:directory];
return [NSString pathWithComponents:@[
directory, kSessionDirectory, sessionID, kSessionFileName
]];
}
#pragma mark - Private methods
......
......@@ -181,7 +181,7 @@ source_set("main") {
"//ios/chrome/browser/ui/thumb_strip:feature_flags",
"//ios/chrome/browser/ui/translate:legacy_translate",
"//ios/chrome/browser/ui/util:multiwindow_util",
"//ios/chrome/browser/url_loading",
"//ios/chrome/browser/url_loading:url_loading_params_header",
"//ios/chrome/browser/web",
"//ios/chrome/browser/web:tab_helper_delegates",
"//ios/chrome/browser/web:web_internal",
......
......@@ -76,6 +76,9 @@ typedef NS_ENUM(NSUInteger, SceneActivationLevel) {
@property(nonatomic, strong, readonly) id<BrowserInterfaceProvider>
interfaceProvider;
// The persistent identifier for the scene session.
@property(nonatomic, readonly) NSString* sceneSessionID;
// True if First Run UI (terms of service & sync sign-in) is being presented
// in a modal dialog.
@property(nonatomic, assign) BOOL presentingFirstRunUI;
......
......@@ -96,6 +96,14 @@
return _window;
}
- (NSString*)sceneSessionID {
NSString* sessionID = nil;
if (@available(ios 13, *)) {
sessionID = _scene.session.persistentIdentifier;
}
return sessionID;
}
- (void)setActivationLevel:(SceneActivationLevel)newLevel {
if (_activationLevel == newLevel) {
return;
......
......@@ -48,6 +48,16 @@ source_set("url_loading") {
]
}
source_set("url_loading_params_header") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [ "url_loading_params.h" ]
deps = [
"//ios/chrome/browser/ui/commands",
"//ios/web/public",
"//ui/base",
]
}
source_set("test_support") {
configs += [ "//build/config/compiler:enable_arc" ]
testonly = true
......
......@@ -15,7 +15,7 @@ source_set("window_activities") {
":ios_move_tab_activity_type_buildflags",
"//base",
"//ios/chrome/browser:chrome_url_constants",
"//ios/chrome/browser/url_loading",
"//ios/chrome/browser/url_loading:url_loading_params_header",
"//ios/web/public/navigation",
"//net",
"//url",
......
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