Skip to content

GitLab

T
tangled
Commit 6eecb42b authored by Mohammad Refaat's avatar Mohammad Refaat Committed by Commit Bot
Browse files
Options

Remove SessionWindowRestoring protocol and the rest of its usages. 

while at it i removed tabModel mentions from perf_test_with_bvc_ios
Also i deleted tests from tab_model_unittest that are sessions related
and moved the one related to NTP to session_restoration_agent_unittest,
however it's disabled as it needs to be converted to eg test.

Bug: 1050110, 1045226, 1049843
Change-Id: I84e137efa14258bed002094ad87dd0db6ca7ea74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2045759Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740746}
parent 6cb35c70
Hide whitespace changes
Inline Side-by-side
Showing with 38 additions and 159 deletions
ios/chrome/browser/sessions/BUILD.gn
View file @ 6eecb42b
......@@ -91,7 +91,6 @@ source_set("serialisation") {
"session_util.mm",
"session_window_ios.h",
"session_window_ios.mm",
"session_window_restoring.h",
]
deps = [
"//base",
......@@ -139,6 +138,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ntp",
"//ios/chrome/browser/web:web_internal",
"//ios/chrome/browser/web_state_list",
"//ios/chrome/browser/web_state_list:test_support",
......
ios/chrome/browser/sessions/session_restoration_browser_agent_unittest.mm
View file @ 6eecb42b
......@@ -12,6 +12,8 @@
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/main/browser_web_state_list_delegate.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper_delegate.h"
#include "ios/chrome/browser/sessions/ios_chrome_session_tab_helper.h"
#import "ios/chrome/browser/sessions/session_ios.h"
#import "ios/chrome/browser/sessions/session_restoration_browser_agent.h"
......@@ -32,6 +34,8 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gtest_mac.h"
#include "testing/platform_test.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#include "third_party/ocmock/gtest_support.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
......@@ -176,6 +180,30 @@ TEST_F(SessionRestorationBrowserAgentTest,
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(3));
}
// TODO(crbug.com/888674): This test requires commiting item to
// WKBasedNavigationManager which is not possible, migrate this to EG test so
// it can be tested.
TEST_F(SessionRestorationBrowserAgentTest, DISABLED_RestoreSessionOnNTPTest) {
web::WebState* web_state =
InsertNewWebState(GURL(kChromeUINewTabURL), /*parent=*/nullptr,
/*index=*/0, /*background=*/false);
// Create NTPTabHelper to ensure VisibleURL is set to kChromeUINewTabURL.
id delegate = OCMProtocolMock(@protocol(NewTabPageTabHelperDelegate));
NewTabPageTabHelper::CreateForWebState(web_state, delegate);
SessionWindowIOS* window(
CreateSessionWindow(/*sessions_count=*/3, /*selected_index=*/2));
session_restoration_agent_->RestoreSessionWindow(window);
ASSERT_EQ(3, web_state_list_->count());
EXPECT_EQ(web_state_list_->GetWebStateAt(2),
web_state_list_->GetActiveWebState());
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(0));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(1));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(2));
}
// Tests that saving a non-empty session, then saving an empty session, then
// restoring, restores zero web states, and not the non-empty session.
TEST_F(SessionRestorationBrowserAgentTest, SaveAndRestoreEmptySession) {
......
ios/chrome/browser/sessions/session_window_restoring.h deleted 100644 → 0
View file @ 6cb35c70
// Copyright 2019 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_WINDOW_RESTORING_H_
#define IOS_CHROME_BROWSER_SESSIONS_SESSION_WINDOW_RESTORING_H_
@class SessionWindowIOS;
// API for an object that can save and restore session windows. Typically an
// object confoming to this protocol is responsible for a list of tabs, and
// calls to these methopds affect that list.
@protocol SessionWindowRestoring
// Restores the |window| (for example, after a crash). If there is only one tab,
// showing the NTP, then this tab should be clobbered, otherwise, the tabs from
// the restored sessions should be added at the end of the current list of tabs.
// If |initialRestore| is YES, this method does not log metrics for operations
// conducted during the restore and must be called before any observers are
// registered on the TabModel. Returns YES if the single NTP tab is closed.
- (BOOL)restoreSessionWindow:(SessionWindowIOS*)window
forInitialRestore:(BOOL)initialRestore;
@end
#endif // IOS_CHROME_BROWSER_SESSIONS_SESSION_WINDOW_RESTORING_H_
ios/chrome/browser/tabs/tab_model.h
View file @ 6eecb42b
......@@ -8,10 +8,7 @@
#import <Foundation/Foundation.h>
#import <UIKit/UIKit.h>
#import "ios/chrome/browser/sessions/session_window_restoring.h"
class ChromeBrowserState;
@class SessionServiceIOS;
class TabModelSyncedWindowDelegate;
class TabUsageRecorder;
class WebStateList;
......@@ -22,7 +19,7 @@ class Browser;
// The model knows about the currently selected tab in order to maintain
// consistency between multiple views that need the current tab to be
// synchronized.
@interface TabModel : NSObject <SessionWindowRestoring>
@interface TabModel : NSObject
// The delegate for sync.
@property(nonatomic, readonly)
......
ios/chrome/browser/tabs/tab_model.mm
View file @ 6eecb42b
......@@ -415,7 +415,6 @@ void RecordMainFrameNavigationMetric(web::WebState* web_state) {
_webStateObserver.reset();
}
#pragma mark - SessionWindowRestoring(public)
- (BOOL)isWebUsageEnabled {
DCHECK(_browserState);
......@@ -424,15 +423,6 @@ void RecordMainFrameNavigationMetric(web::WebState* web_state) {
->IsWebUsageEnabled();
}
- (BOOL)restoreSessionWindow:(SessionWindowIOS*)window
forInitialRestore:(BOOL)initialRestore {
DCHECK(_browserState);
// It is only ok to pass a nil |window| during the initial restore.
DCHECK(window || initialRestore);
return _sessionRestorationBrowserAgent->RestoreSessionWindow(window);
}
#pragma mark - Notification Handlers
// Called when UIApplicationWillResignActiveNotification is received.
......
ios/chrome/browser/tabs/tab_model_unittest.mm
View file @ 6eecb42b
......@@ -88,7 +88,6 @@ class TabModelTest : public PlatformTest {
// 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_);
SetTabModel(CreateTabModel(nil));
......@@ -116,7 +115,6 @@ class TabModelTest : public PlatformTest {
TabModel* CreateTabModel(SessionWindowIOS* session_window) {
TabModel* tab_model([[TabModel alloc] initWithBrowser:browser_.get()]);
[tab_model restoreSessionWindow:session_window forInitialRestore:YES];
[tab_model setPrimary:YES];
return tab_model;
}
......@@ -135,24 +133,10 @@ class TabModelTest : public PlatformTest {
}
protected:
// Creates a session window with entries named "restored window 1",
// "restored window 2" and "restored window 3" and the second entry
// marked as selected.
SessionWindowIOS* CreateSessionWindow() {
NSMutableArray<CRWSessionStorage*>* sessions = [NSMutableArray array];
for (int i = 0; i < 3; i++) {
CRWSessionStorage* session_storage = [[CRWSessionStorage alloc] init];
session_storage.lastCommittedItemIndex = -1;
[sessions addObject:session_storage];
}
return [[SessionWindowIOS alloc] initWithSessions:sessions selectedIndex:1];
}
web::WebTaskEnvironment task_environment_;
IOSChromeScopedTestingChromeBrowserStateManager scoped_browser_state_manager_;
web::ScopedTestingWebClient web_client_;
std::unique_ptr<WebStateListDelegate> web_state_list_delegate_;
std::unique_ptr<WebStateList> web_state_list_;
std::unique_ptr<Browser> browser_;
SessionWindowIOS* session_window_;
......@@ -263,92 +247,6 @@ TEST_F(TabModelTest, CloseTabAtIndexOnlyOne) {
EXPECT_EQ(0, web_state_list_->count());
}
// TODO(crbug.com/888674): migrate this to EG test so it can be tested with
// WKBasedNavigationManager.
TEST_F(TabModelTest, DISABLED_RestoreSessionOnNTPTest) {
web::WebState* web_state = agent_->InsertWebState(Params(GURL(kURL1)),
/*parent=*/nil,
/*opened_by_dom=*/false,
/*index=*/0,
/*in_background=*/false);
web::WebStateImpl* web_state_impl =
static_cast<web::WebStateImpl*>(web_state);
// Create NTPTabHelper to ensure VisibleURL is set to kChromeUINewTabURL.
id delegate = OCMProtocolMock(@protocol(NewTabPageTabHelperDelegate));
NewTabPageTabHelper::CreateForWebState(web_state, delegate);
web_state_impl->GetNavigationManagerImpl().CommitPendingItem();
SessionWindowIOS* window(CreateSessionWindow());
[tab_model_ restoreSessionWindow:window forInitialRestore:NO];
ASSERT_EQ(3, web_state_list_->count());
EXPECT_EQ(web_state_list_->GetWebStateAt(1),
web_state_list_->GetActiveWebState());
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(0));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(1));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(2));
}
// TODO(crbug.com/888674): migrate this to EG test so it can be tested with
// WKBasedNavigationManager.
TEST_F(TabModelTest, DISABLED_RestoreSessionOn2NtpTest) {
web::WebState* web_state0 = agent_->InsertWebState(Params(GURL(kURL1)),
/*parent=*/nil,
/*opened_by_dom=*/false,
/*index=*/0,
/*in_background=*/false);
web::WebStateImpl* web_state_impl =
static_cast<web::WebStateImpl*>(web_state0);
web_state_impl->GetNavigationManagerImpl().CommitPendingItem();
web::WebState* web_state1 = agent_->InsertWebState(Params(GURL(kURL1)),
/*parent=*/nil,
/*opened_by_dom=*/false,
/*index=*/0,
/*in_background=*/false);
web_state_impl = static_cast<web::WebStateImpl*>(web_state1);
web_state_impl->GetNavigationManagerImpl().CommitPendingItem();
SessionWindowIOS* window(CreateSessionWindow());
[tab_model_ restoreSessionWindow:window forInitialRestore:NO];
ASSERT_EQ(5, web_state_list_->count());
EXPECT_EQ(web_state_list_->GetWebStateAt(3),
web_state_list_->GetActiveWebState());
EXPECT_EQ(web_state0, web_state_list_->GetWebStateAt(0));
EXPECT_EQ(web_state1, web_state_list_->GetWebStateAt(1));
EXPECT_NE(web_state0, web_state_list_->GetWebStateAt(2));
EXPECT_NE(web_state0, web_state_list_->GetWebStateAt(3));
EXPECT_NE(web_state0, web_state_list_->GetWebStateAt(4));
EXPECT_NE(web_state1, web_state_list_->GetWebStateAt(2));
EXPECT_NE(web_state1, web_state_list_->GetWebStateAt(3));
EXPECT_NE(web_state1, web_state_list_->GetWebStateAt(4));
}
// TODO(crbug.com/888674): migrate this to EG test so it can be tested with
// WKBasedNavigationManager.
TEST_F(TabModelTest, DISABLED_RestoreSessionOnAnyTest) {
web::WebState* web_state = agent_->InsertWebState(Params(GURL(kURL1)),
/*parent=*/nil,
/*opened_by_dom=*/false,
/*index=*/0,
/*in_background=*/false);
web::WebStateImpl* web_state_impl =
static_cast<web::WebStateImpl*>(web_state);
web_state_impl->GetNavigationManagerImpl().CommitPendingItem();
SessionWindowIOS* window(CreateSessionWindow());
[tab_model_ restoreSessionWindow:window forInitialRestore:NO];
ASSERT_EQ(4, web_state_list_->count());
EXPECT_EQ(web_state_list_->GetWebStateAt(2),
web_state_list_->GetActiveWebState());
EXPECT_EQ(web_state, web_state_list_->GetWebStateAt(0));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(1));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(2));
EXPECT_NE(web_state, web_state_list_->GetWebStateAt(3));
}
TEST_F(TabModelTest, CloseAllTabs) {
agent_->InsertWebState(Params(GURL(kURL1)),
/*parent=*/nil,
......
ios/chrome/browser/test/perf_test_with_bvc_ios.mm
View file @ 6eecb42b
......@@ -18,7 +18,6 @@
#include "ios/chrome/browser/sessions/session_restoration_browser_agent.h"
#import "ios/chrome/browser/sessions/session_service_ios.h"
#import "ios/chrome/browser/sessions/session_window_ios.h"
#import "ios/chrome/browser/tabs/tab_model.h"
#import "ios/chrome/browser/ui/browser_container/browser_container_view_controller.h"
#import "ios/chrome/browser/ui/browser_view/browser_view_controller+private.h"
#import "ios/chrome/browser/ui/browser_view/browser_view_controller.h"
......@@ -96,7 +95,7 @@ void PerfTestWithBVC::SetUp() {
ios::AutocompleteClassifierFactory::GetForBrowserState(
chrome_browser_state_.get());
// Use the session to create a window which will contain the tab models.
// Use the session to create a window which will contain the tabs.
NSString* state_path = base::SysUTF8ToNSString(
chrome_browser_state_->GetStatePath().AsUTF8Unsafe());
SessionIOS* session =
......@@ -112,21 +111,16 @@ void PerfTestWithBVC::SetUp() {
browser_.get(), [SessionServiceIOS sharedService]);
SessionRestorationBrowserAgent::CreateForBrowser(
otr_browser_.get(), [SessionServiceIOS sharedService]);
// Tab models. The off-the-record (OTR) tab model is required for the stack
// view controller, which is created in OpenStackView().
tab_model_ = [[TabModel alloc] initWithBrowser:browser_.get()];
[tab_model_ restoreSessionWindow:session.sessionWindows[0]
forInitialRestore:YES];
otr_tab_model_ = [[TabModel alloc] initWithBrowser:otr_browser_.get()];
[otr_tab_model_ restoreSessionWindow:session.sessionWindows[0]
forInitialRestore:YES];
SessionRestorationBrowserAgent::FromBrowser(browser_.get())
->RestoreSessionWindow(session.sessionWindows[0]);
SessionRestorationBrowserAgent::FromBrowser(otr_browser_.get())
->RestoreSessionWindow(session.sessionWindows[0]);
command_dispatcher_ = [[CommandDispatcher alloc] init];
// Create the browser view controller with its testing factory.
bvc_factory_ = [[BrowserViewControllerDependencyFactory alloc]
initWithBrowserState:chrome_browser_state_.get()
webStateList:[tab_model_ webStateList]];
webStateList:browser_->GetWebStateList()];
bvc_ = [[BrowserViewController alloc]
initWithBrowser:browser_.get()
dependencyFactory:bvc_factory_
......@@ -145,7 +139,8 @@ void PerfTestWithBVC::SetUp() {
}
void PerfTestWithBVC::TearDown() {
[[bvc_ tabModel] closeAllTabs];
browser_.get()->GetWebStateList()->CloseAllWebStates(
WebStateList::CLOSE_NO_FLAGS);
[[bvc_ view] removeFromSuperview];
// Documented example of how to clear out the browser view controller
......@@ -154,9 +149,6 @@ void PerfTestWithBVC::TearDown() {
[bvc_ shutdown];
bvc_ = nil;
bvc_factory_ = nil;
tab_model_ = nil;
[otr_tab_model_ disconnect];
otr_tab_model_ = nil;
// The base class |TearDown| method calls the run loop so the
// NSAutoreleasePool can drain. This needs to be done before
......
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