Skip to content

Commit

Permalink
[iOS] Remove TabModel
Browse files Browse the repository at this point in the history
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 <marq@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908028}
  • Loading branch information
marcq authored and Chromium LUCI CQ committed Aug 3, 2021
1 parent 75ec0dc commit c02e385
Show file tree
Hide file tree
Showing 18 changed files with 271 additions and 361 deletions.
5 changes: 0 additions & 5 deletions ios/chrome/browser/main/browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand Down
3 changes: 0 additions & 3 deletions ios/chrome/browser/main/browser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

class ChromeBrowserState;
@class SceneState;
@class TabModel;
class WebStateList;
class WebStateListDelegate;

Expand All @@ -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;
Expand All @@ -48,7 +46,6 @@ class BrowserImpl : public Browser {
std::unique_ptr<WebStateList> web_state_list);

ChromeBrowserState* browser_state_;
__strong TabModel* tab_model_ = nil;
std::unique_ptr<WebStateListDelegate> web_state_list_delegate_;
std::unique_ptr<WebStateList> web_state_list_;
__strong CommandDispatcher* command_dispatcher_;
Expand Down
10 changes: 0 additions & 10 deletions ios/chrome/browser/main/browser_impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -30,10 +29,6 @@
std::make_unique<WebStateList>(web_state_list_delegate_.get());
}

void BrowserImpl::CreateTabModel() {
tab_model_ = [[TabModel alloc] initWithBrowser:this];
}

BrowserImpl::BrowserImpl(ChromeBrowserState* browser_state,
std::unique_ptr<WebStateList> web_state_list)
: browser_state_(browser_state),
Expand All @@ -51,10 +46,6 @@
return browser_state_;
}

TabModel* BrowserImpl::GetTabModel() const {
return tab_model_;
}

WebStateList* BrowserImpl::GetWebStateList() const {
return web_state_list_.get();
}
Expand All @@ -76,6 +67,5 @@
std::unique_ptr<BrowserImpl> browser =
std::make_unique<BrowserImpl>(browser_state);
AttachBrowserAgents(browser.get());
browser->CreateTabModel();
return browser;
}
6 changes: 0 additions & 6 deletions ios/chrome/browser/main/browser_impl_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -48,11 +47,6 @@
std::unique_ptr<Browser> browser =
std::make_unique<BrowserImpl>(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());
}
2 changes: 0 additions & 2 deletions ios/chrome/browser/main/test_browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<BrowserObserver, /* check_empty= */ true> observers_;

Expand Down
4 changes: 0 additions & 4 deletions ios/chrome/browser/main/test_browser.mm
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
return browser_state_;
}

TabModel* TestBrowser::GetTabModel() const {
return tab_model_;
}

WebStateList* TestBrowser::GetWebStateList() const {
return web_state_list_;
}
Expand Down
18 changes: 18 additions & 0 deletions ios/chrome/browser/sessions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 75 additions & 0 deletions ios/chrome/browser/sessions/README.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- Add details on how sessions are saved; this depends on if the iOS15 APIs
for saving WKWebView interactionState are being used or not. Also describe
how per-window session saving is done. -->

`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.
20 changes: 20 additions & 0 deletions ios/chrome/browser/sessions/session_saving_scene_agent.h
Original file line number Diff line number Diff line change
@@ -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 <Foundation/Foundation.h>

#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_
137 changes: 137 additions & 0 deletions ios/chrome/browser/sessions/session_saving_scene_agent.mm
Original file line number Diff line number Diff line change
@@ -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<BrowserInterfaceProvider> 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<BrowserInterfaceProvider> interfaceProvider =
self.sceneState.interfaceProvider;
if (!interfaceProvider)
return nullptr;

return [self
snapshotHelperForActiveWebStateInBrowser:interfaceProvider.mainInterface
.browser];
}

- (SnapshotTabHelper*)snapshotHelperForActiveWebStateInIncognitoBrowser {
id<BrowserInterfaceProvider> 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
Loading

0 comments on commit c02e385

Please sign in to comment.