Commit 54c09d3f authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Restore items state when restoring session

When a session is restored, the restored NavigationItems have all the
information of the restored items, including scroll offset, user
agent...
But the items were only used for their URL and all the other
information were lost. This CL fixes that by storing those items
until the restore is complete. At this point their information can be
restored in the items associated with the WKBackForwardItem.

For now those information aren't used with kEmbedderBlockRestoreUrl
disabled.

Bug: 771200
Change-Id: Ic04f3eb0a893899acfcfdd46b33c950b2a8e71e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2061151
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747630}
parent b3d09c15
...@@ -131,6 +131,9 @@ class NavigationItemImpl : public web::NavigationItem { ...@@ -131,6 +131,9 @@ class NavigationItemImpl : public web::NavigationItem {
void SetUntrusted(); void SetUntrusted();
bool IsUntrusted(); bool IsUntrusted();
// Restores the state of the |other| navigation item in this item.
void RestoreStateFromItem(NavigationItem* other);
#ifndef NDEBUG #ifndef NDEBUG
// Returns a human-readable description of the state for debugging purposes. // Returns a human-readable description of the state for debugging purposes.
NSString* GetDescription() const; NSString* GetDescription() const;
......
...@@ -338,6 +338,17 @@ void NavigationItemImpl::ResetForCommit() { ...@@ -338,6 +338,17 @@ void NavigationItemImpl::ResetForCommit() {
SetNavigationInitiationType(web::NavigationInitiationType::NONE); SetNavigationInitiationType(web::NavigationInitiationType::NONE);
} }
void NavigationItemImpl::RestoreStateFromItem(NavigationItem* other) {
// Only restore the UserAgent type and the page display state. The other
// headers might not make sense after creating a new navigation to the page.
bool inherited_user_agent =
other->GetUserAgentType() == other->GetUserAgentForInheritance();
if (other->GetUserAgentType() != UserAgentType::NONE) {
SetUserAgentType(other->GetUserAgentType(), inherited_user_agent);
}
SetPageDisplayState(other->GetPageDisplayState());
}
ErrorRetryStateMachine& NavigationItemImpl::error_retry_state_machine() { ErrorRetryStateMachine& NavigationItemImpl::error_retry_state_machine() {
DCHECK(!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage)); DCHECK(!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage));
return error_retry_state_machine_; return error_retry_state_machine_;
......
...@@ -103,6 +103,14 @@ class MockNavigationManagerDelegate : public NavigationManagerDelegate { ...@@ -103,6 +103,14 @@ class MockNavigationManagerDelegate : public NavigationManagerDelegate {
WebState* web_state_ = nullptr; WebState* web_state_ = nullptr;
}; };
// Data holder for the informations to be restored in the items.
struct ItemInfoToBeRestored {
GURL url;
UserAgentType user_agent;
bool user_agent_inherited;
PageDisplayState display_state;
};
} // namespace } // namespace
// Test fixture for NavigationManagerImpl testing. // Test fixture for NavigationManagerImpl testing.
...@@ -1824,12 +1832,22 @@ TEST_F(NavigationManagerTest, TestBackwardForwardItems) { ...@@ -1824,12 +1832,22 @@ TEST_F(NavigationManagerTest, TestBackwardForwardItems) {
// Tests that Restore() creates the correct navigation state. // Tests that Restore() creates the correct navigation state.
TEST_F(NavigationManagerTest, Restore) { TEST_F(NavigationManagerTest, Restore) {
GURL urls[3] = {GURL("http://www.url.com/0"), GURL("http://www.url.com/1"), ItemInfoToBeRestored restore_information[3];
GURL("http://www.url.com/2")}; restore_information[0] = {GURL("http://www.url.com/0"), UserAgentType::MOBILE,
true, PageDisplayState()};
restore_information[1] = {GURL("http://www.url.com/1"),
UserAgentType::DESKTOP, true, PageDisplayState()};
restore_information[2] = {GURL("http://www.url.com/2"),
UserAgentType::DESKTOP, false, PageDisplayState()};
std::vector<std::unique_ptr<NavigationItem>> items; std::vector<std::unique_ptr<NavigationItem>> items;
for (size_t index = 0; index < base::size(urls); ++index) { for (size_t index = 0; index < base::size(restore_information); ++index) {
items.push_back(NavigationItem::Create()); items.push_back(NavigationItem::Create());
items.back()->SetURL(urls[index]); items.back()->SetURL(restore_information[index].url);
items.back()->SetUserAgentType(
restore_information[index].user_agent,
restore_information[index].user_agent_inherited);
items.back()->SetPageDisplayState(restore_information[index].display_state);
} }
// Call Restore() and check that the NavigationItems are in the correct order // Call Restore() and check that the NavigationItems are in the correct order
...@@ -1838,6 +1856,7 @@ TEST_F(NavigationManagerTest, Restore) { ...@@ -1838,6 +1856,7 @@ TEST_F(NavigationManagerTest, Restore) {
navigation_manager()->Restore(1, std::move(items)); navigation_manager()->Restore(1, std::move(items));
__block bool restore_done = false; __block bool restore_done = false;
navigation_manager()->AddRestoreCompletionCallback(base::BindOnce(^{ navigation_manager()->AddRestoreCompletionCallback(base::BindOnce(^{
navigation_manager()->CommitPendingItem();
restore_done = true; restore_done = true;
})); }));
...@@ -1857,7 +1876,7 @@ TEST_F(NavigationManagerTest, Restore) { ...@@ -1857,7 +1876,7 @@ TEST_F(NavigationManagerTest, Restore) {
navigation_manager()->OnNavigationStarted(pending_url); navigation_manager()->OnNavigationStarted(pending_url);
// Simulate the end effect of loading the restore session URL in web view. // Simulate the end effect of loading the restore session URL in web view.
pending_item->SetURL(urls[1]); pending_item->SetURL(restore_information[1].url);
[mock_wk_list_ setCurrentURL:@"http://www.url.com/1" [mock_wk_list_ setCurrentURL:@"http://www.url.com/1"
backListURLs:@[ @"http://www.url.com/0" ] backListURLs:@[ @"http://www.url.com/0" ]
forwardListURLs:@[ @"http://www.url.com/2" ]]; forwardListURLs:@[ @"http://www.url.com/2" ]];
...@@ -1870,10 +1889,29 @@ TEST_F(NavigationManagerTest, Restore) { ...@@ -1870,10 +1889,29 @@ TEST_F(NavigationManagerTest, Restore) {
EXPECT_FALSE(navigation_manager()->IsRestoreSessionInProgress()); EXPECT_FALSE(navigation_manager()->IsRestoreSessionInProgress());
ASSERT_EQ(3, navigation_manager()->GetItemCount()); ASSERT_EQ(3, navigation_manager()->GetItemCount());
EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex()); EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex());
EXPECT_EQ(urls[1], navigation_manager()->GetLastCommittedItem()->GetURL()); EXPECT_EQ(restore_information[1].url,
navigation_manager()->GetLastCommittedItem()->GetURL());
for (size_t i = 0; i < base::size(urls); ++i) { for (size_t i = 0; i < base::size(restore_information); ++i) {
EXPECT_EQ(urls[i], navigation_manager()->GetItemAtIndex(i)->GetURL()); NavigationItem* navigation_item = navigation_manager()->GetItemAtIndex(i);
EXPECT_EQ(restore_information[i].url, navigation_item->GetURL());
EXPECT_EQ(restore_information[i].user_agent,
navigation_item->GetUserAgentType());
if (restore_information[i].user_agent_inherited) {
EXPECT_EQ(restore_information[i].user_agent,
navigation_item->GetUserAgentForInheritance());
} else {
if (base::FeatureList::IsEnabled(
features::kUseDefaultUserAgentInWebClient)) {
EXPECT_EQ(UserAgentType::AUTOMATIC,
navigation_item->GetUserAgentForInheritance());
} else {
EXPECT_EQ(UserAgentType::MOBILE,
navigation_item->GetUserAgentForInheritance());
}
}
EXPECT_EQ(restore_information[i].display_state,
navigation_item->GetPageDisplayState());
} }
histogram_tester_.ExpectTotalCount(kRestoreNavigationItemCount, 1); histogram_tester_.ExpectTotalCount(kRestoreNavigationItemCount, 1);
......
...@@ -204,6 +204,12 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl { ...@@ -204,6 +204,12 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
DISALLOW_COPY_AND_ASSIGN(WKWebViewCache); DISALLOW_COPY_AND_ASSIGN(WKWebViewCache);
}; };
// Type of the list passed to restore items.
enum class RestoreItemListType {
kBackList,
kForwardList,
};
// NavigationManagerImpl: // NavigationManagerImpl:
NavigationItemImpl* GetNavigationItemImplAtIndex(size_t index) const override; NavigationItemImpl* GetNavigationItemImplAtIndex(size_t index) const override;
NavigationItemImpl* GetLastCommittedItemInCurrentOrRestoredSession() NavigationItemImpl* GetLastCommittedItemInCurrentOrRestoredSession()
...@@ -222,6 +228,17 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl { ...@@ -222,6 +228,17 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
NavigationInitiationType initiation_type) override; NavigationInitiationType initiation_type) override;
bool IsPlaceholderUrl(const GURL& url) const override; bool IsPlaceholderUrl(const GURL& url) const override;
// Restores the state of the |items_restored| in the navigation items
// associated with the WKBackForwardList. |back_list| is used to specify if
// the items passed are the list containing the back list or the forward list.
void RestoreItemsState(
RestoreItemListType list_type,
std::vector<std::unique_ptr<NavigationItem>> items_restored);
// Restores the state of the |restored_visible_item_| in the last committed
// item.
void RestoreVisibleItemState();
// Restores the specified navigation session in the current web view. This // Restores the specified navigation session in the current web view. This
// differs from Restore() in that it doesn't reset the current navigation // differs from Restore() in that it doesn't reset the current navigation
// history to empty before restoring. It simply appends the restored session // history to empty before restoring. It simply appends the restored session
......
...@@ -84,7 +84,7 @@ void WKBasedNavigationManagerImpl::OnNavigationItemCommitted() { ...@@ -84,7 +84,7 @@ void WKBasedNavigationManagerImpl::OnNavigationItemCommitted() {
delegate_->OnNavigationItemCommitted(item); delegate_->OnNavigationItemCommitted(item);
if (!wk_navigation_util::IsRestoreSessionUrl(item->GetURL())) { if (!wk_navigation_util::IsRestoreSessionUrl(item->GetURL())) {
restored_visible_item_.reset(); RestoreVisibleItemState();
} }
} }
...@@ -417,7 +417,7 @@ bool WKBasedNavigationManagerImpl::ShouldBlockUrlDuringRestore( ...@@ -417,7 +417,7 @@ bool WKBasedNavigationManagerImpl::ShouldBlockUrlDuringRestore(
// Abort restore. // Abort restore.
DiscardNonCommittedItems(); DiscardNonCommittedItems();
last_committed_item_index_ = web_view_cache_.GetCurrentItemIndex(); last_committed_item_index_ = web_view_cache_.GetCurrentItemIndex();
restored_visible_item_.reset(); RestoreVisibleItemState();
FinalizeSessionRestore(); FinalizeSessionRestore();
return true; return true;
} }
...@@ -733,6 +733,24 @@ void WKBasedNavigationManagerImpl::UnsafeRestore( ...@@ -733,6 +733,24 @@ void WKBasedNavigationManagerImpl::UnsafeRestore(
if (last_committed_item_index > -1) if (last_committed_item_index > -1)
restored_visible_item_ = std::move(items[last_committed_item_index]); restored_visible_item_ = std::move(items[last_committed_item_index]);
std::vector<std::unique_ptr<NavigationItem>> back_items;
for (int index = 0; index < last_committed_item_index; index++) {
back_items.push_back(std::move(items[index]));
}
std::vector<std::unique_ptr<NavigationItem>> forward_items;
for (size_t index = last_committed_item_index + 1; index < items.size();
index++) {
forward_items.push_back(std::move(items[index]));
}
AddRestoreCompletionCallback(base::BindOnce(
&WKBasedNavigationManagerImpl::RestoreItemsState, base::Unretained(this),
RestoreItemListType::kBackList, std::move(back_items)));
AddRestoreCompletionCallback(base::BindOnce(
&WKBasedNavigationManagerImpl::RestoreItemsState, base::Unretained(this),
RestoreItemListType::kForwardList, std::move(forward_items)));
LoadURLWithParams(params); LoadURLWithParams(params);
// On restore prime the first navigation item with the title. The remaining // On restore prime the first navigation item with the title. The remaining
...@@ -743,6 +761,49 @@ void WKBasedNavigationManagerImpl::UnsafeRestore( ...@@ -743,6 +761,49 @@ void WKBasedNavigationManagerImpl::UnsafeRestore(
} }
} }
void WKBasedNavigationManagerImpl::RestoreItemsState(
RestoreItemListType list_type,
std::vector<std::unique_ptr<NavigationItem>> items_restored) {
bool back_list = list_type == RestoreItemListType::kBackList;
size_t current_item_index = web_view_cache_.GetCurrentItemIndex();
size_t cache_offset = back_list ? 0 : current_item_index + 1;
size_t cache_limit = back_list
? current_item_index
: web_view_cache_.GetBackForwardListItemCount();
for (size_t index = 0; index < items_restored.size(); index++) {
size_t cache_index = index + cache_offset;
if (cache_index >= cache_limit)
break;
NavigationItemImpl* cached_item =
web_view_cache_.GetNavigationItemImplAtIndex(
cache_index, true /* create_if_missing */);
NavigationItem* restore_item = items_restored[index].get();
bool is_same_url = cached_item->GetURL() == restore_item->GetURL();
if (wk_navigation_util::IsRestoreSessionUrl(cached_item->GetURL())) {
GURL target_url;
if (wk_navigation_util::ExtractTargetURL(cached_item->GetURL(),
&target_url))
is_same_url = target_url == restore_item->GetURL();
}
if (is_same_url) {
cached_item->RestoreStateFromItem(restore_item);
}
}
}
void WKBasedNavigationManagerImpl::RestoreVisibleItemState() {
NavigationItemImpl* last_committed_item =
GetLastCommittedItemInCurrentOrRestoredSession();
if (restored_visible_item_ && last_committed_item) {
last_committed_item->RestoreStateFromItem(restored_visible_item_.get());
}
restored_visible_item_.reset();
}
void WKBasedNavigationManagerImpl::LoadURLWithParams( void WKBasedNavigationManagerImpl::LoadURLWithParams(
const NavigationManager::WebLoadParams& params) { const NavigationManager::WebLoadParams& params) {
if (IsRestoreSessionInProgress() && if (IsRestoreSessionInProgress() &&
......
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