From c02e3852530f690b7a6645708d5fa0eb992d2685 Mon Sep 17 00:00:00 2001 From: Mark Cogan Date: Tue, 3 Aug 2021 17:07:00 +0000 Subject: [PATCH] [iOS] Remove TabModel This CL factors out the remaining responsibilities of TabModel and removes it. It introduces a functional change in how sessions are saved when scenes background. For the purposes of this code, "saving sessions" involves both updating the session store and writing out snapshots of the active web states. TabModels -- which were per-browser objects -- listed for the UIApplication lifecycle notifications to determine when to save sessions. This means that when the application backgrounds, all of the connected scenes save the sessions for the active webstates in their browsers. It also means that sessions are *not* saved when individual scenes background. (This is not the only trigger for saving sessions in Chrome, just the one determined by app/scene lifecycles). This CL changes this backgrounding logic, moving it to a scene state agent. This agent observes the scene state transitions, and now it saves sessions per-scene, whenever that scene backgrounds. The other responsibility of TabModel was to close all of the web states in a web state list when the owning Browser is shut down. This CL moves that responsibility to BrowserViewWrangler, which is the current owner of Browser lifecycles. Potentially it could be migrated to the BrowserImpl class in the future for even better encapsulation. Because there are some circumstances where the app may shut down without being backgrounded (if it is force-quit in the app switcher when no other apps are present, for example), this CL adds a check to the shutdown code in SceneController that asks the scene to save the session if needed. (Sessions are marked as needing saving if the scene has foregrounded since the previous save). Bug: 1045575, 1115611, 1110966, 783777 Change-Id: Ic70f4d1b839ab95757196a805ccac21c36b7dc83 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3063619 Commit-Queue: Mark Cogan Reviewed-by: Rohit Rao Reviewed-by: Mohammad Refaat Cr-Commit-Position: refs/heads/master@{#908028} --- ios/chrome/browser/main/browser.h | 5 - ios/chrome/browser/main/browser_impl.h | 3 - ios/chrome/browser/main/browser_impl.mm | 10 -- .../browser/main/browser_impl_unittest.mm | 6 - ios/chrome/browser/main/test_browser.h | 2 - ios/chrome/browser/main/test_browser.mm | 4 - ios/chrome/browser/sessions/BUILD.gn | 18 ++ ios/chrome/browser/sessions/README.md | 75 +++++++++ .../sessions/session_saving_scene_agent.h | 20 +++ .../sessions/session_saving_scene_agent.mm | 137 +++++++++++++++ ios/chrome/browser/tabs/BUILD.gn | 3 - ios/chrome/browser/tabs/tab_model.h | 31 ---- ios/chrome/browser/tabs/tab_model.mm | 157 ------------------ ios/chrome/browser/tabs/tab_model_unittest.mm | 136 --------------- ios/chrome/browser/ui/main/BUILD.gn | 1 + .../browser/ui/main/browser_view_wrangler.mm | 19 ++- .../ui/main/browser_view_wrangler_unittest.mm | 1 - .../browser/ui/main/scene_controller.mm | 4 + 18 files changed, 271 insertions(+), 361 deletions(-) create mode 100644 ios/chrome/browser/sessions/README.md create mode 100644 ios/chrome/browser/sessions/session_saving_scene_agent.h create mode 100644 ios/chrome/browser/sessions/session_saving_scene_agent.mm delete mode 100644 ios/chrome/browser/tabs/tab_model.h delete mode 100644 ios/chrome/browser/tabs/tab_model.mm delete mode 100644 ios/chrome/browser/tabs/tab_model_unittest.mm diff --git a/ios/chrome/browser/main/browser.h b/ios/chrome/browser/main/browser.h index 8e5c9ecb4811e3..b58d8be6d3bc91 100644 --- a/ios/chrome/browser/main/browser.h +++ b/ios/chrome/browser/main/browser.h @@ -13,7 +13,6 @@ class BrowserObserver; class ChromeBrowserState; @class CommandDispatcher; -@class TabModel; class WebStateList; // Browser is the model for a window containing multiple tabs. Instances @@ -31,10 +30,6 @@ class Browser : public base::SupportsUserData { // Accessor for the owning ChromeBrowserState. virtual ChromeBrowserState* GetBrowserState() const = 0; - // Accessor for the TabModel. DEPRECATED: prefer GetWebStateList() whenever - // possible. - virtual TabModel* GetTabModel() const = 0; - // Accessor for the WebStateList. virtual WebStateList* GetWebStateList() const = 0; diff --git a/ios/chrome/browser/main/browser_impl.h b/ios/chrome/browser/main/browser_impl.h index a8a37d7b3413e4..9ecf6084e8eca0 100644 --- a/ios/chrome/browser/main/browser_impl.h +++ b/ios/chrome/browser/main/browser_impl.h @@ -14,7 +14,6 @@ class ChromeBrowserState; @class SceneState; -@class TabModel; class WebStateList; class WebStateListDelegate; @@ -35,7 +34,6 @@ class BrowserImpl : public Browser { // Browser. ChromeBrowserState* GetBrowserState() const override; - TabModel* GetTabModel() const override; WebStateList* GetWebStateList() const override; CommandDispatcher* GetCommandDispatcher() const override; void AddObserver(BrowserObserver* observer) override; @@ -48,7 +46,6 @@ class BrowserImpl : public Browser { std::unique_ptr web_state_list); ChromeBrowserState* browser_state_; - __strong TabModel* tab_model_ = nil; std::unique_ptr web_state_list_delegate_; std::unique_ptr web_state_list_; __strong CommandDispatcher* command_dispatcher_; diff --git a/ios/chrome/browser/main/browser_impl.mm b/ios/chrome/browser/main/browser_impl.mm index cd38991b9158cb..246785ee729276 100644 --- a/ios/chrome/browser/main/browser_impl.mm +++ b/ios/chrome/browser/main/browser_impl.mm @@ -11,7 +11,6 @@ #import "ios/chrome/browser/main/browser_observer.h" #import "ios/chrome/browser/main/browser_web_state_list_delegate.h" #import "ios/chrome/browser/sessions/session_service_ios.h" -#import "ios/chrome/browser/tabs/tab_model.h" #import "ios/chrome/browser/ui/commands/command_dispatcher.h" #import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list_delegate.h" @@ -30,10 +29,6 @@ std::make_unique(web_state_list_delegate_.get()); } -void BrowserImpl::CreateTabModel() { - tab_model_ = [[TabModel alloc] initWithBrowser:this]; -} - BrowserImpl::BrowserImpl(ChromeBrowserState* browser_state, std::unique_ptr web_state_list) : browser_state_(browser_state), @@ -51,10 +46,6 @@ return browser_state_; } -TabModel* BrowserImpl::GetTabModel() const { - return tab_model_; -} - WebStateList* BrowserImpl::GetWebStateList() const { return web_state_list_.get(); } @@ -76,6 +67,5 @@ std::unique_ptr browser = std::make_unique(browser_state); AttachBrowserAgents(browser.get()); - browser->CreateTabModel(); return browser; } diff --git a/ios/chrome/browser/main/browser_impl_unittest.mm b/ios/chrome/browser/main/browser_impl_unittest.mm index 3e1f9d5a07e27d..bc2d79f7d7a9fc 100644 --- a/ios/chrome/browser/main/browser_impl_unittest.mm +++ b/ios/chrome/browser/main/browser_impl_unittest.mm @@ -6,7 +6,6 @@ #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #import "ios/chrome/browser/main/fake_browser_observer.h" -#import "ios/chrome/browser/tabs/tab_model.h" #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h" #include "ios/chrome/browser/web_state_list/web_state_list.h" #include "ios/web/public/test/web_task_environment.h" @@ -48,11 +47,6 @@ std::unique_ptr browser = std::make_unique(chrome_browser_state_.get()); FakeBrowserObserver observer(browser.get()); - // Simulate shut down order from BrowserViewWrangler, where the TabModel's - // |-browserStateDestroyed| is expected to be executed before the - // TabModelList's destructor. - // TODO(crbug.com/783777): Remove when TabModel is no longer used. - [browser->GetTabModel() disconnect]; browser = nullptr; EXPECT_TRUE(observer.browser_destroyed()); } diff --git a/ios/chrome/browser/main/test_browser.h b/ios/chrome/browser/main/test_browser.h index 8e9ee244b5bbe2..6cf229693cb3c6 100644 --- a/ios/chrome/browser/main/test_browser.h +++ b/ios/chrome/browser/main/test_browser.h @@ -32,7 +32,6 @@ class TestBrowser : public Browser { // Browser. ChromeBrowserState* GetBrowserState() const override; - TabModel* GetTabModel() const override; WebStateList* GetWebStateList() const override; CommandDispatcher* GetCommandDispatcher() const override; void AddObserver(BrowserObserver* observer) override; @@ -47,7 +46,6 @@ class TestBrowser : public Browser { // Used in all cases. __strong CommandDispatcher* command_dispatcher_ = nil; ChromeBrowserState* browser_state_ = nullptr; - TabModel* tab_model_ = nil; WebStateList* web_state_list_ = nullptr; base::ObserverList observers_; diff --git a/ios/chrome/browser/main/test_browser.mm b/ios/chrome/browser/main/test_browser.mm index c67057b73cdc5b..9affd01c769a5a 100644 --- a/ios/chrome/browser/main/test_browser.mm +++ b/ios/chrome/browser/main/test_browser.mm @@ -53,10 +53,6 @@ return browser_state_; } -TabModel* TestBrowser::GetTabModel() const { - return tab_model_; -} - WebStateList* TestBrowser::GetWebStateList() const { return web_state_list_; } diff --git a/ios/chrome/browser/sessions/BUILD.gn b/ios/chrome/browser/sessions/BUILD.gn index 806ec05cbaf680..7c767dc5c60f89 100644 --- a/ios/chrome/browser/sessions/BUILD.gn +++ b/ios/chrome/browser/sessions/BUILD.gn @@ -128,6 +128,24 @@ source_set("serialisation") { configs += [ "//build/config/compiler:enable_arc" ] } +source_set("session_saving") { + sources = [ + "session_saving_scene_agent.h", + "session_saving_scene_agent.mm", + ] + deps = [ + ":restoration_agent", + "//ios/chrome/browser/browser_state", + "//ios/chrome/browser/main:public", + "//ios/chrome/browser/snapshots", + "//ios/chrome/browser/ui/main:browser_interface_provider", + "//ios/chrome/browser/ui/main:observing_scene_agent", + "//ios/chrome/browser/web_state_list", + "//ios/chrome/browser/web_state_list/web_usage_enabler", + ] + configs += [ "//build/config/compiler:enable_arc" ] +} + source_set("test_support") { configs += [ "//build/config/compiler:enable_arc" ] testonly = true diff --git a/ios/chrome/browser/sessions/README.md b/ios/chrome/browser/sessions/README.md new file mode 100644 index 00000000000000..ee5ebb68ce9272 --- /dev/null +++ b/ios/chrome/browser/sessions/README.md @@ -0,0 +1,75 @@ +## Session saving and restoration. + +A *Session* is the data Chrome saves to an iOS device's storage that preserves +the state of open tabs in a given Browser. On a device that only supports a +single Chrome window, there will be one saved session for the regular tabs, +and one saved session for the Incognito tabs. On devices that support multiple +windows, there will be one or two (two if there are Incognito tabs) saved +sessions for each window. + +Sessions are the mechanism by which a user's open tabs are restored when they +laucnh Chrome. Given that iOS can terminate Chrome in the background at any +time -- and can disconnect individual scenes when there are multiple windows -- +it's critical that sessions are saved and restored in a way that is quick and +lossless for the user. + +(Don't confuse this Chrome-specific sense of *session* with the UIKit concept +of a *scene session*, implemented in the `UISceneSession` class. While both are +concerned with persisting application state data, they are not interchangeable, +and Chrome's sessions are not implemented using UISceneSessions for storage.) + +### Session saving + +The design intent is that sessions are saved before Chrome enters a state where +it might be suddenly terminated. + +Saving a session is done by means of the `SessionRestorationBrowserAgent` for +the browser being saved, using the `SaveSession()` method. Calling this saves +the session for the browser the agent is attached to. + + + +`SaveSession()` is called in the following circumstances: +1. Whenever a sene moves to the background, `SaveSession()` is called on its + browsers. +2. When the UI for a scene is destroyed (when the scene's `SceneController` is + shutting down), `SaveSession()` is called if it hasn't been called aready. +3. When a tab is inserted, removed, activated, replaced, re-ordered, or + completes a navigation, `SaveSession()` on the Browser that owns its + WebStateList. +4. When a prerendered tab is loaded, replacing an existing webState, + `SaveSession()` on the browser whose WebStateList contains the new tab. + +Case (1) is handled by `SessionSavingSceneAgent`, which watches for scene state +changes and tracks if the current scene has been foregrounded since the last +time it was saved. Such scenes are marked as needing to have their sessions +saved the next time they background. + +Case (2) is handled by `SceneController` itself when it shuts down; it in turn +asks its `SessionSavingSceneAgent` to save its sessions if needed. + +Case (3) is handled by `SessionRestorationBrowserAgent` via WebState and +WebStateList observation. + +Case (4) is handled by the `PrerenderService`; it directly calls +`SaveSession()`, regardless of whether the session is in the background. + +Cases (3) and (4) save after a short delay, and repeated session saves within +that delay are ignored. Cases (1) and (2) save immediately (canceling any +pending delayed saves). `SessionServiceIOS` handles this. + +### Session Restoration + +TODO: Describe when sessions are restored. + +### Session Recovery + +TODO: Describe the logic for post-crash session recovery and how sessions are +set aside and (possibly) discarded. + +### Incognito Sessions + +TODO: Describe how Incognito sessions are stored, and how it differs from how +regular session are. diff --git a/ios/chrome/browser/sessions/session_saving_scene_agent.h b/ios/chrome/browser/sessions/session_saving_scene_agent.h new file mode 100644 index 00000000000000..6a7a49f1c8d8b2 --- /dev/null +++ b/ios/chrome/browser/sessions/session_saving_scene_agent.h @@ -0,0 +1,20 @@ +// Copyright 2021 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_SESSIONS_SESSION_SAVING_SCENE_AGENT_H_ +#define IOS_CHROME_BROWSER_SESSIONS_SESSION_SAVING_SCENE_AGENT_H_ + +#import + +#import "ios/chrome/browser/ui/main/observing_scene_state_agent.h" + +@interface SessionSavingSceneAgent : ObservingSceneAgent + +// Saves the scene's sessions if they haven't been saved since the last time +// the scene was foregrounded. +- (void)saveSessionsIfNeeded; + +@end + +#endif // IOS_CHROME_BROWSER_SESSIONS_SESSION_SAVING_SCENE_AGENT_H_ diff --git a/ios/chrome/browser/sessions/session_saving_scene_agent.mm b/ios/chrome/browser/sessions/session_saving_scene_agent.mm new file mode 100644 index 00000000000000..9d7d8d684d8dee --- /dev/null +++ b/ios/chrome/browser/sessions/session_saving_scene_agent.mm @@ -0,0 +1,137 @@ +// Copyright 2021 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/sessions/session_saving_scene_agent.h" + +#import "ios/chrome/browser/browser_state/chrome_browser_state.h" +#import "ios/chrome/browser/main/browser.h" +#import "ios/chrome/browser/sessions/session_restoration_browser_agent.h" +#import "ios/chrome/browser/snapshots/snapshot_tab_helper.h" +#import "ios/chrome/browser/ui/main/browser_interface_provider.h" +#import "ios/chrome/browser/web_state_list/web_state_list.h" +#import "ios/chrome/browser/web_state_list/web_usage_enabler/web_usage_enabler_browser_agent.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +@implementation SessionSavingSceneAgent { + // YES when sessions need saving -- specifically after the scene has + // foregrounded. Initially NO, so session saving isn't triggered as the + // scene initially launches. + BOOL _sessionsNeedSaving; +} + +#pragma mark - SceneStateObserver + +- (void)sceneState:(SceneState*)sceneState + transitionedToActivationLevel:(SceneActivationLevel)level { + switch (level) { + case SceneActivationLevelUnattached: + // no-op. + break; + case SceneActivationLevelBackground: + [self saveSessionsIfNeeded]; + break; + case SceneActivationLevelForegroundInactive: + [self prepareSessionsForBackgrounding]; + break; + case SceneActivationLevelForegroundActive: + _sessionsNeedSaving = YES; + break; + } +} + +#pragma mark - Public + +- (void)saveSessionsIfNeeded { + // No need to do this if the session is already saved. + if (!_sessionsNeedSaving) + return; + + id interfaceProvider = + self.sceneState.interfaceProvider; + if (!interfaceProvider) + return; + + // Since the app is about to be backgrounded or terminated, save the sessions + // immediately. + Browser* mainBrowser = interfaceProvider.mainInterface.browser; + SessionRestorationBrowserAgent::FromBrowser(mainBrowser) + ->SaveSession(/*immediately=*/true); + if (interfaceProvider.hasIncognitoInterface) { + Browser* incognitoBrowser = interfaceProvider.incognitoInterface.browser; + SessionRestorationBrowserAgent::FromBrowser(incognitoBrowser) + ->SaveSession(/*immediately=*/true); + } + + // Save a grey version of the active webstates. + SnapshotTabHelper* mainSnapshotHelper = + [self snapshotHelperForActiveWebStateInMainBrowser]; + if (mainSnapshotHelper) { + mainSnapshotHelper->SaveGreyInBackground(); + } + + SnapshotTabHelper* incognitoSnapshotHelper = + [self snapshotHelperForActiveWebStateInIncognitoBrowser]; + if (incognitoSnapshotHelper) { + incognitoSnapshotHelper->SaveGreyInBackground(); + } + + _sessionsNeedSaving = NO; +} + +#pragma mark - Private + +- (void)prepareSessionsForBackgrounding { + SnapshotTabHelper* mainSnapshotHelper = + [self snapshotHelperForActiveWebStateInMainBrowser]; + if (mainSnapshotHelper) { + mainSnapshotHelper->WillBeSavedGreyWhenBackgrounding(); + } + + SnapshotTabHelper* incognitoSnapshotHelper = + [self snapshotHelperForActiveWebStateInIncognitoBrowser]; + if (incognitoSnapshotHelper) { + incognitoSnapshotHelper->WillBeSavedGreyWhenBackgrounding(); + } +} + +#pragma mark - Utility + +- (SnapshotTabHelper*)snapshotHelperForActiveWebStateInMainBrowser { + id interfaceProvider = + self.sceneState.interfaceProvider; + if (!interfaceProvider) + return nullptr; + + return [self + snapshotHelperForActiveWebStateInBrowser:interfaceProvider.mainInterface + .browser]; +} + +- (SnapshotTabHelper*)snapshotHelperForActiveWebStateInIncognitoBrowser { + id interfaceProvider = + self.sceneState.interfaceProvider; + if (!interfaceProvider.hasIncognitoInterface) + return nullptr; + + return [self + snapshotHelperForActiveWebStateInBrowser:interfaceProvider + .incognitoInterface.browser]; +} + +- (SnapshotTabHelper*)snapshotHelperForActiveWebStateInBrowser: + (Browser*)browser { + WebUsageEnablerBrowserAgent* webEnabler = + WebUsageEnablerBrowserAgent::FromBrowser(browser); + web::WebState* webState = browser->GetWebStateList()->GetActiveWebState(); + if (webEnabler->IsWebUsageEnabled() && webState != nullptr) { + return SnapshotTabHelper::FromWebState(webState); + } + + return nullptr; +} + +@end diff --git a/ios/chrome/browser/tabs/BUILD.gn b/ios/chrome/browser/tabs/BUILD.gn index 7c55c55c62cdf8..7f8361eaeffdb2 100644 --- a/ios/chrome/browser/tabs/BUILD.gn +++ b/ios/chrome/browser/tabs/BUILD.gn @@ -9,7 +9,6 @@ source_set("tabs") { "synced_window_delegate_browser_agent.h", "tab_helper_delegate_installer.h", "tab_helper_util.h", - "tab_model.h", "tab_parenting_browser_agent.h", "tab_parenting_browser_agent.mm", "tab_parenting_global_observer.cc", @@ -36,7 +35,6 @@ source_set("tabs_internal") { "ios_synced_window_delegate_getter.mm", "synced_window_delegate_browser_agent.mm", "tab_helper_util.mm", - "tab_model.mm", "tab_title_util.h", "tab_title_util.mm", ] @@ -126,7 +124,6 @@ source_set("unit_tests") { testonly = true sources = [ "tab_helper_delegate_installer_unittest.mm", - "tab_model_unittest.mm", "tab_title_util_unittest.mm", ] deps = [ diff --git a/ios/chrome/browser/tabs/tab_model.h b/ios/chrome/browser/tabs/tab_model.h deleted file mode 100644 index 7fa440b73e0939..00000000000000 --- a/ios/chrome/browser/tabs/tab_model.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2012 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_TABS_TAB_MODEL_H_ -#define IOS_CHROME_BROWSER_TABS_TAB_MODEL_H_ - -#import -#import - -class Browser; - -// A legacy class that encapsulates some lifecycle business logic for tabs in -// a WebStateList. This class is moribund (see crbug.com/783777). -@interface TabModel : NSObject -// Initializes a TabModel from a browser. -- (instancetype)initWithBrowser:(Browser*)browser; - -- (instancetype)init NS_UNAVAILABLE; - -// Tells the receiver to disconnect from the model object it depends on. This -// should be called before destroying the browser that the receiver was -// initialized with. -// It is safe to call this method multiple times. -// At this point the tab model will no longer ever be active, and will likely be -// deallocated soon. -- (void)disconnect; - -@end - -#endif // IOS_CHROME_BROWSER_TABS_TAB_MODEL_H_ diff --git a/ios/chrome/browser/tabs/tab_model.mm b/ios/chrome/browser/tabs/tab_model.mm deleted file mode 100644 index 2c9890e35b7ddd..00000000000000 --- a/ios/chrome/browser/tabs/tab_model.mm +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright 2012 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/tabs/tab_model.h" - -#include "ios/chrome/browser/browser_state/chrome_browser_state.h" -#import "ios/chrome/browser/sessions/session_restoration_browser_agent.h" -#import "ios/chrome/browser/snapshots/snapshot_tab_helper.h" -#import "ios/chrome/browser/web_state_list/web_state_list.h" -#import "ios/chrome/browser/web_state_list/web_usage_enabler/web_usage_enabler_browser_agent.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - - -@interface TabModel () { - // Weak reference to the underlying shared model implementation. - WebStateList* _webStateList; - - // Enabler for |_webStateList| - WebUsageEnablerBrowserAgent* _webEnabler; - - // Weak reference to the session restoration agent. - SessionRestorationBrowserAgent* _sessionRestorationBrowserAgent; - - ChromeBrowserState* _browserState; -} - -@end - -@implementation TabModel { - BOOL _savedSessionDuringBackgrounding; -} - -#pragma mark - Overriden - -- (void)dealloc { - // -disconnect should always have been called before destruction. - DCHECK(!_browserState); -} - -#pragma mark - Public methods - -- (instancetype)initWithBrowser:(Browser*)browser { - if ((self = [super init])) { - _webStateList = browser->GetWebStateList(); - _browserState = browser->GetBrowserState(); - DCHECK(_browserState); - - _sessionRestorationBrowserAgent = - SessionRestorationBrowserAgent::FromBrowser(browser); - _webEnabler = WebUsageEnablerBrowserAgent::FromBrowser(browser); - - _savedSessionDuringBackgrounding = NO; - - // Register for resign active notification. - [[NSNotificationCenter defaultCenter] - addObserver:self - selector:@selector(applicationWillResignActive:) - name:UIApplicationWillResignActiveNotification - object:nil]; - // Register for background notification. - [[NSNotificationCenter defaultCenter] - addObserver:self - selector:@selector(applicationDidEnterBackground:) - name:UIApplicationDidEnterBackgroundNotification - object:nil]; - // Register for foregrounding notification. - [[NSNotificationCenter defaultCenter] - addObserver:self - selector:@selector(applicationWillEnterForeground:) - name:UIApplicationWillEnterForegroundNotification - object:nil]; - } - return self; -} - -// NOTE: This can be called multiple times, so must be robust against that. -- (void)disconnect { - if (!_browserState) - return; - - if (!_savedSessionDuringBackgrounding) { - [self saveSessionOnBackgroundingOrTermination]; - _savedSessionDuringBackgrounding = YES; - } - - [[NSNotificationCenter defaultCenter] removeObserver:self]; - - _sessionRestorationBrowserAgent = nullptr; - _browserState = nullptr; - - // Close all tabs. Do this in an @autoreleasepool as WebStateList observers - // will be notified (they are unregistered later). As some of them may be - // implemented in Objective-C and unregister themselves in their -dealloc - // method, ensure they -autorelease introduced by ARC are processed before - // the WebStateList destructor is called. - @autoreleasepool { - _webStateList->CloseAllWebStates(WebStateList::CLOSE_NO_FLAGS); - } - - _webStateList = nullptr; -} - -#pragma mark - Notification Handlers - -// Called when UIApplicationWillResignActiveNotification is received. -// TODO(crbug.com/1115611): Move to SceneController. -- (void)applicationWillResignActive:(NSNotification*)notify { - if (_webEnabler->IsWebUsageEnabled() && _webStateList->GetActiveWebState()) { - SnapshotTabHelper::FromWebState(_webStateList->GetActiveWebState()) - ->WillBeSavedGreyWhenBackgrounding(); - } -} - -// Called when UIApplicationDidEnterBackgroundNotification is received. -// TODO(crbug.com/1115611): Move to SceneController. -- (void)applicationDidEnterBackground:(NSNotification*)notify { - // When using the Scene API (which requires iOS 13.0 or later and a recent - // enough device), UIApplicationDidEnterBackgroundNotification is not sent - // to the application if it is terminated by swipe gesture while it is in - // the foreground. The notification is send if Scene API is not used. In - // order to avoid saving twice the session on app termination, use a flag - // to record that the session was saved. - [self saveSessionOnBackgroundingOrTermination]; - _savedSessionDuringBackgrounding = YES; -} - -// Called when UIApplicationWillEnterForegroundNotification is received. -// TODO(crbug.com/1115611): Move to SceneController. -- (void)applicationWillEnterForeground:(NSNotification*)notify { - // Reset the boolean to allow saving the session state the next time the - // application is backgrounded or terminated. - _savedSessionDuringBackgrounding = NO; -} - -#pragma mark - Saving session on backgrounding or termination - -- (void)saveSessionOnBackgroundingOrTermination { - if (!_browserState) - return; - - // Normally, the session is saved after some timer expires but since the app - // is about to be backgrounded or terminated send true to save the session - // immediately. - _sessionRestorationBrowserAgent->SaveSession(/*immediately=*/true); - - // Write out a grey version of the current website to disk. - if (_webEnabler->IsWebUsageEnabled() && _webStateList->GetActiveWebState()) { - SnapshotTabHelper::FromWebState(_webStateList->GetActiveWebState()) - ->SaveGreyInBackground(); - } -} - -@end diff --git a/ios/chrome/browser/tabs/tab_model_unittest.mm b/ios/chrome/browser/tabs/tab_model_unittest.mm deleted file mode 100644 index 867fb60bbf41fc..00000000000000 --- a/ios/chrome/browser/tabs/tab_model_unittest.mm +++ /dev/null @@ -1,136 +0,0 @@ -// Copyright 2012 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/tabs/tab_model.h" - -#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" -#include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h" -#import "ios/chrome/browser/main/browser_web_state_list_delegate.h" -#import "ios/chrome/browser/main/test_browser.h" -#include "ios/chrome/browser/sessions/session_restoration_browser_agent.h" -#import "ios/chrome/browser/sessions/session_window_ios.h" -#import "ios/chrome/browser/sessions/test_session_service.h" -#import "ios/chrome/browser/web_state_list/tab_insertion_browser_agent.h" -#import "ios/chrome/browser/web_state_list/web_state_list.h" -#import "ios/chrome/browser/web_state_list/web_usage_enabler/web_usage_enabler_browser_agent.h" -#include "ios/chrome/test/ios_chrome_scoped_testing_chrome_browser_state_manager.h" -#include "ios/web/public/test/web_task_environment.h" -#include "ios/web/public/thread/web_thread.h" -#include "testing/platform_test.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -const char kURL1[] = "https://www.some.url.com"; - -class TabModelTest : public PlatformTest { - public: - TabModelTest() - : scoped_browser_state_manager_( - std::make_unique(base::FilePath())), - web_state_list_delegate_( - std::make_unique()), - web_state_list_( - std::make_unique(web_state_list_delegate_.get())) { - DCHECK_CURRENTLY_ON(web::WebThread::UI); - - TestChromeBrowserState::Builder test_cbs_builder; - chrome_browser_state_ = test_cbs_builder.Build(); - - browser_ = std::make_unique(chrome_browser_state_.get(), - web_state_list_.get()); - // Web usage is disabled during these tests. - WebUsageEnablerBrowserAgent::CreateForBrowser(browser_.get()); - web_usage_enabler_ = - WebUsageEnablerBrowserAgent::FromBrowser(browser_.get()); - web_usage_enabler_->SetWebUsageEnabled(false); - - session_window_ = [[SessionWindowIOS alloc] init]; - - TabInsertionBrowserAgent::CreateForBrowser(browser_.get()); - - agent_ = TabInsertionBrowserAgent::FromBrowser(browser_.get()); - session_service_ = [[TestSessionService alloc] init]; - // Create session restoration agent with just a dummy session - // service so the async state saving doesn't trigger unless actually - // wanted. - SessionRestorationBrowserAgent::CreateForBrowser(browser_.get(), - session_service_); - SessionRestorationBrowserAgent::FromBrowser(browser_.get()) - ->SetSessionID([[NSUUID UUID] UUIDString]); - SetTabModel(CreateTabModel(nil)); - } - - ~TabModelTest() override = default; - - void TearDown() override { - SetTabModel(nil); - PlatformTest::TearDown(); - } - - void SetTabModel(TabModel* tab_model) { - if (tab_model_) { - @autoreleasepool { - [tab_model_ disconnect]; - tab_model_ = nil; - } - } - - tab_model_ = tab_model; - } - - TabModel* CreateTabModel(SessionWindowIOS* session_window) { - TabModel* tab_model([[TabModel alloc] initWithBrowser:browser_.get()]); - return tab_model; - } - - const web::NavigationManager::WebLoadParams Params(GURL url) { - return Params(url, ui::PAGE_TRANSITION_TYPED); - } - - const web::NavigationManager::WebLoadParams Params( - GURL url, - ui::PageTransition transition) { - web::NavigationManager::WebLoadParams loadParams(url); - loadParams.referrer = web::Referrer(); - loadParams.transition_type = transition; - return loadParams; - } - - protected: - web::WebTaskEnvironment task_environment_; - IOSChromeScopedTestingChromeBrowserStateManager scoped_browser_state_manager_; - std::unique_ptr web_state_list_delegate_; - std::unique_ptr web_state_list_; - std::unique_ptr browser_; - SessionWindowIOS* session_window_; - std::unique_ptr chrome_browser_state_; - TabInsertionBrowserAgent* agent_; - TestSessionService* session_service_; - WebUsageEnablerBrowserAgent* web_usage_enabler_; - TabModel* tab_model_; -}; - -TEST_F(TabModelTest, InsertWithSessionController) { - EXPECT_EQ(web_state_list_->count(), 0); - - web::WebState* new_web_state = - agent_->InsertWebState(Params(GURL(kURL1)), - /*parent=*/nil, - /*opened_by_dom=*/false, - /*index=*/web_state_list_->count(), - /*in_background=*/false, - /*inherit_opener=*/false); - - EXPECT_EQ(web_state_list_->count(), 1); - EXPECT_EQ(new_web_state, web_state_list_->GetWebStateAt(0)); - web_state_list_->ActivateWebStateAt(0); - web::WebState* current_web_state = web_state_list_->GetActiveWebState(); - EXPECT_TRUE(current_web_state); -} - -} // anonymous namespace diff --git a/ios/chrome/browser/ui/main/BUILD.gn b/ios/chrome/browser/ui/main/BUILD.gn index 6853f581bb407e..09649c64b6e169 100644 --- a/ios/chrome/browser/ui/main/BUILD.gn +++ b/ios/chrome/browser/ui/main/BUILD.gn @@ -151,6 +151,7 @@ source_set("scene") { "//ios/chrome/browser/policy:policy_util", "//ios/chrome/browser/screenshot", "//ios/chrome/browser/sessions:scene_util", + "//ios/chrome/browser/sessions:session_saving", "//ios/chrome/browser/signin", "//ios/chrome/browser/snapshots", "//ios/chrome/browser/ui:feature_flags", diff --git a/ios/chrome/browser/ui/main/browser_view_wrangler.mm b/ios/chrome/browser/ui/main/browser_view_wrangler.mm index 393a8e65515351..e7a60e61e755b7 100644 --- a/ios/chrome/browser/ui/main/browser_view_wrangler.mm +++ b/ios/chrome/browser/ui/main/browser_view_wrangler.mm @@ -19,7 +19,6 @@ #import "ios/chrome/browser/sessions/scene_util.h" #import "ios/chrome/browser/sessions/session_restoration_browser_agent.h" #import "ios/chrome/browser/snapshots/snapshot_browser_agent.h" -#import "ios/chrome/browser/tabs/tab_model.h" #import "ios/chrome/browser/ui/browser_view/browser_coordinator.h" #import "ios/chrome/browser/ui/browser_view/browser_view_controller.h" #import "ios/chrome/browser/ui/browser_view/browser_view_controller_dependency_factory.h" @@ -240,7 +239,14 @@ - (void)clearMainBrowser { WebStateList* webStateList = self.mainBrowser->GetWebStateList(); breakpad::StopMonitoringTabStateForWebStateList(webStateList); breakpad::StopMonitoringURLsForWebStateList(webStateList); - [self.mainBrowser->GetTabModel() disconnect]; + // Close all webstates in |webStateList|. Do this in an @autoreleasepool as + // WebStateList observers will be notified (they are unregistered later). As + // some of them may be implemented in Objective-C and unregister themselves + // in their -dealloc method, ensure they -autorelease introduced by ARC are + // processed before the WebStateList destructor is called. + @autoreleasepool { + webStateList->CloseAllWebStates(WebStateList::CLOSE_NO_FLAGS); + } } _mainBrowser = nullptr; @@ -250,7 +256,14 @@ - (void)setOtrBrowser:(std::unique_ptr)otrBrowser { if (_otrBrowser.get()) { WebStateList* webStateList = self.otrBrowser->GetWebStateList(); breakpad::StopMonitoringTabStateForWebStateList(webStateList); - [self.otrBrowser->GetTabModel() disconnect]; + // Close all webstates in |webStateList|. Do this in an @autoreleasepool as + // WebStateList observers will be notified (they are unregistered later). As + // some of them may be implemented in Objective-C and unregister themselves + // in their -dealloc method, ensure they -autorelease introduced by ARC are + // processed before the WebStateList destructor is called. + @autoreleasepool { + webStateList->CloseAllWebStates(WebStateList::CLOSE_NO_FLAGS); + } } _otrBrowser = std::move(otrBrowser); diff --git a/ios/chrome/browser/ui/main/browser_view_wrangler_unittest.mm b/ios/chrome/browser/ui/main/browser_view_wrangler_unittest.mm index 18acc17ab26a1d..d2dbbf2368aaff 100644 --- a/ios/chrome/browser/ui/main/browser_view_wrangler_unittest.mm +++ b/ios/chrome/browser/ui/main/browser_view_wrangler_unittest.mm @@ -19,7 +19,6 @@ #include "ios/chrome/browser/sessions/session_restoration_browser_agent.h" #import "ios/chrome/browser/sessions/test_session_service.h" #import "ios/chrome/browser/sync/send_tab_to_self_sync_service_factory.h" -#import "ios/chrome/browser/tabs/tab_model.h" #import "ios/chrome/browser/ui/browser_view/browser_view_controller.h" #import "ios/chrome/browser/ui/main/scene_state.h" #import "ios/chrome/browser/ui/main/scene_state_browser_agent.h" diff --git a/ios/chrome/browser/ui/main/scene_controller.mm b/ios/chrome/browser/ui/main/scene_controller.mm index 7f9cf25c7d529a..aae2bbc886f9ee 100644 --- a/ios/chrome/browser/ui/main/scene_controller.mm +++ b/ios/chrome/browser/ui/main/scene_controller.mm @@ -66,6 +66,7 @@ #include "ios/chrome/browser/policy/policy_watcher_browser_agent.h" #import "ios/chrome/browser/policy/policy_watcher_browser_agent_observer_bridge.h" #include "ios/chrome/browser/screenshot/screenshot_delegate.h" +#import "ios/chrome/browser/sessions/session_saving_scene_agent.h" #import "ios/chrome/browser/signin/authentication_service.h" #import "ios/chrome/browser/signin/authentication_service_factory.h" #import "ios/chrome/browser/signin/chrome_account_manager_service.h" @@ -299,6 +300,7 @@ - (instancetype)initWithSceneState:(SceneState*)sceneState { [_sceneState addAgent:[[StartSurfaceSceneAgent alloc] init]]; [_sceneState addAgent:[[ReadingListBackgroundSessionSceneAgent alloc] init]]; + [_sceneState addAgent:[[SessionSavingSceneAgent alloc] init]]; } return self; } @@ -1135,6 +1137,8 @@ - (void)teardownUI { // agent). self.sceneState.UIEnabled = NO; + [[SessionSavingSceneAgent agentFromScene:self.sceneState] + saveSessionsIfNeeded]; [self.browserViewWrangler shutdown]; self.browserViewWrangler = nil;