Commit 24afdd17 authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Chromium LUCI CQ

[ios] Use -[SceneState sceneSessionID] as session identifier

For device that do not support multi-window, the persistent identifier
returned by -[UISceneSession persistentIdentifier] may change when the
user "swipe" the app (as this gesture is interpreted as "close window"
under certain circumstances).

In preparation for using a stable identifier (i.e. a constant) for
devices that do not support multi-window, do not access the value from
UISceneSession directly in Chrome but instead through SceneState.

There is one location that is still using -persistentIdentifier: the
code responsible for recording the OS message asking Chrome to purge
old session data. It is find to use -persistentIdentifier there because
the value is correct for device that support multi-window, for other
device the value is not used as part of a path to save data thus it is
benign to try to delete data from a place that does not exists.

Bug: 1165798
Change-Id: I5169b5c0be961987ef8e1edc9c122947848268e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626440Reviewed-by: default avatarsebsg <sebsg@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844014}
parent b6b21a5f
...@@ -468,6 +468,19 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher ...@@ -468,6 +468,19 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
API_AVAILABLE(ios(13)) { API_AVAILABLE(ios(13)) {
NSMutableArray<NSString*>* sessionIDs = NSMutableArray<NSString*>* sessionIDs =
[NSMutableArray arrayWithCapacity:sceneSessions.count]; [NSMutableArray arrayWithCapacity:sceneSessions.count];
// This method is invoked by iOS to inform the application that the sessions
// for "closed windows" is garbage collected and that any data associated with
// them by the application needs to be deleted.
//
// Usually Chrome uses -[SceneState sceneSessionID] as identifier to properly
// support devices that do not support multi-window (and which use a constant
// identifier). For devices that do not support multi-window the session is
// saved at a constant path, so it is harmnless to delete files at a path
// derived from -persistentIdentifier (since there won't be files deleted).
// For devices that do support multi-window, there is data to delete once the
// session is garbage collected.
//
// Thus it is always correct to use -persistentIdentifier here.
for (UISceneSession* session in sceneSessions) { for (UISceneSession* session in sceneSessions) {
[sessionIDs addObject:session.persistentIdentifier]; [sessionIDs addObject:session.persistentIdentifier];
} }
......
...@@ -105,9 +105,6 @@ ...@@ -105,9 +105,6 @@
std::unique_ptr<Browser> _otrBrowser; std::unique_ptr<Browser> _otrBrowser;
} }
// Opaque session ID from _sceneState, nil when multi-window isn't enabled.
@property(nonatomic, readonly) NSString* sessionID;
@property(nonatomic, strong, readwrite) WrangledBrowser* mainInterface; @property(nonatomic, strong, readwrite) WrangledBrowser* mainInterface;
@property(nonatomic, strong, readwrite) WrangledBrowser* incognitoInterface; @property(nonatomic, strong, readwrite) WrangledBrowser* incognitoInterface;
...@@ -171,16 +168,6 @@ ...@@ -171,16 +168,6 @@
#pragma mark - BrowserViewInformation property implementations #pragma mark - BrowserViewInformation property implementations
- (NSString*)sessionID {
NSString* sessionID = nil;
if (IsMultiwindowSupported()) {
if (@available(iOS 13, *)) {
sessionID = _sceneState.scene.session.persistentIdentifier;
}
}
return sessionID;
}
- (void)setCurrentInterface:(WrangledBrowser*)interface { - (void)setCurrentInterface:(WrangledBrowser*)interface {
DCHECK(interface); DCHECK(interface);
// |interface| must be one of the interfaces this class already owns. // |interface| must be one of the interfaces this class already owns.
...@@ -415,7 +402,7 @@ ...@@ -415,7 +402,7 @@
- (void)setSessionIDForBrowser:(Browser*)browser - (void)setSessionIDForBrowser:(Browser*)browser
restoreSession:(BOOL)restoreSession { restoreSession:(BOOL)restoreSession {
SnapshotBrowserAgent::FromBrowser(browser)->SetSessionID( SnapshotBrowserAgent::FromBrowser(browser)->SetSessionID(
base::SysNSStringToUTF8(self.sessionID)); base::SysNSStringToUTF8(_sceneState.sceneSessionID));
SessionRestorationBrowserAgent* restorationAgent = SessionRestorationBrowserAgent* restorationAgent =
SessionRestorationBrowserAgent::FromBrowser(browser); SessionRestorationBrowserAgent::FromBrowser(browser);
...@@ -443,12 +430,13 @@ ...@@ -443,12 +430,13 @@
NSString* previousSessionID = NSString* previousSessionID =
_sceneState.appState.previousSingleWindowSessionID; _sceneState.appState.previousSingleWindowSessionID;
if (previousSessionID && if (previousSessionID &&
![self.sessionID isEqualToString:previousSessionID]) { ![_sceneState.sceneSessionID isEqualToString:previousSessionID]) {
restorationAgent->SetSessionID( restorationAgent->SetSessionID(
base::SysNSStringToUTF8(previousSessionID)); base::SysNSStringToUTF8(previousSessionID));
restorationAgent->RestoreSession(); restorationAgent->RestoreSession();
restorationAgent->SetSessionID(base::SysNSStringToUTF8(self.sessionID)); restorationAgent->SetSessionID(
base::SysNSStringToUTF8(_sceneState.sceneSessionID));
restorationAgent->SaveSession(true); restorationAgent->SaveSession(true);
// Fallback to the normal codepath. It will set the session identifier // Fallback to the normal codepath. It will set the session identifier
...@@ -458,7 +446,8 @@ ...@@ -458,7 +446,8 @@
} }
} }
restorationAgent->SetSessionID(base::SysNSStringToUTF8(self.sessionID)); restorationAgent->SetSessionID(
base::SysNSStringToUTF8(_sceneState.sceneSessionID));
if (restoreSession) if (restoreSession)
restorationAgent->RestoreSession(); restorationAgent->RestoreSession();
} }
......
...@@ -348,7 +348,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] = ...@@ -348,7 +348,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
// Add the scene to the list of connected scene, to restore in case of // Add the scene to the list of connected scene, to restore in case of
// crashes. // crashes.
[[PreviousSessionInfo sharedInstance] [[PreviousSessionInfo sharedInstance]
addSceneSessionID:sceneState.scene.session.persistentIdentifier]; addSceneSessionID:sceneState.sceneSessionID];
} }
} }
} }
...@@ -396,8 +396,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] = ...@@ -396,8 +396,7 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
// If Multiple scenes are not supported, the session shouldn't be // If Multiple scenes are not supported, the session shouldn't be
// removed as it can be used for normal restoration. // removed as it can be used for normal restoration.
[[PreviousSessionInfo sharedInstance] [[PreviousSessionInfo sharedInstance]
removeSceneSessionID:sceneState.scene.session removeSceneSessionID:sceneState.sceneSessionID];
.persistentIdentifier];
} }
} }
} }
......
...@@ -76,7 +76,8 @@ typedef NS_ENUM(NSUInteger, SceneActivationLevel) { ...@@ -76,7 +76,8 @@ typedef NS_ENUM(NSUInteger, SceneActivationLevel) {
@property(nonatomic, strong, readonly) id<BrowserInterfaceProvider> @property(nonatomic, strong, readonly) id<BrowserInterfaceProvider>
interfaceProvider; interfaceProvider;
// The persistent identifier for the scene session. // The persistent identifier for the scene session. This should be used instead
// of -[UISceneSession persistentIdentifier].
@property(nonatomic, readonly) NSString* sceneSessionID; @property(nonatomic, readonly) NSString* sceneSessionID;
// True if First Run UI (terms of service & sync sign-in) is being presented // True if First Run UI (terms of service & sync sign-in) is being presented
......
...@@ -130,7 +130,9 @@ ContentVisibility ContentVisibilityForIncognito(BOOL isIncognito) { ...@@ -130,7 +130,9 @@ ContentVisibility ContentVisibilityForIncognito(BOOL isIncognito) {
- (NSString*)sceneSessionID { - (NSString*)sceneSessionID {
NSString* sessionID = nil; NSString* sessionID = nil;
if (@available(ios 13, *)) { if (@available(ios 13, *)) {
sessionID = _scene.session.persistentIdentifier; if (IsMultiwindowSupported()) {
sessionID = _scene.session.persistentIdentifier;
}
} }
return sessionID; return sessionID;
} }
......
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