Commit 30322cd7 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

[ios] Don't double restore URLs after clear browsing data.

Normally restore URL's wouldn't make it to -CreateRestoreSessionUrl
because VirtualURLs are saved during session serialization.  However,
after clear browsing data we short-circuit this path and directly
restore the session URL history. In this path, the session may effectively
be restored twice.  While the code path works, it's more prone to
failures and looks bad since the restore URL will be visible in the
omnibox.

Bug: 949540
Change-Id: Id39a328dcf94575719387dd56221a6cef85fe522
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569966Reviewed-by: default avatarDanyao Wang <danyao@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652168}
parent a3ff93e7
...@@ -663,6 +663,7 @@ void WKBasedNavigationManagerImpl::AddRestoreCompletionCallback( ...@@ -663,6 +663,7 @@ void WKBasedNavigationManagerImpl::AddRestoreCompletionCallback(
void WKBasedNavigationManagerImpl::LoadIfNecessary() { void WKBasedNavigationManagerImpl::LoadIfNecessary() {
if (!web_view_cache_.IsAttachedToWebView()) { if (!web_view_cache_.IsAttachedToWebView()) {
// Loading from detached mode is equivalent to restoring cached history. // Loading from detached mode is equivalent to restoring cached history.
// This can happen after clearing browsing data by removing the web view.
Restore(web_view_cache_.GetCurrentItemIndex(), Restore(web_view_cache_.GetCurrentItemIndex(),
web_view_cache_.ReleaseCachedItems()); web_view_cache_.ReleaseCachedItems());
DCHECK(web_view_cache_.IsAttachedToWebView()); DCHECK(web_view_cache_.IsAttachedToWebView());
...@@ -817,6 +818,13 @@ void WKBasedNavigationManagerImpl::WKWebViewCache::DetachFromWebView() { ...@@ -817,6 +818,13 @@ void WKBasedNavigationManagerImpl::WKWebViewCache::DetachFromWebView() {
for (size_t index = 0; index < GetBackForwardListItemCount(); index++) { for (size_t index = 0; index < GetBackForwardListItemCount(); index++) {
cached_items_[index].reset(new NavigationItemImpl( cached_items_[index].reset(new NavigationItemImpl(
*GetNavigationItemImplAtIndex(index, true /* create_if_missing */))); *GetNavigationItemImplAtIndex(index, true /* create_if_missing */)));
// Don't put restore URL's into |cached_items|, extract them first.
GURL url = cached_items_[index]->GetURL();
if (wk_navigation_util::IsRestoreSessionUrl(url)) {
GURL extracted_url;
if (wk_navigation_util::ExtractTargetURL(url, &extracted_url))
cached_items_[index]->SetURL(extracted_url);
}
} }
} }
attached_to_web_view_ = false; attached_to_web_view_ = false;
......
...@@ -858,6 +858,11 @@ class WKBasedNavigationManagerDetachedModeTest ...@@ -858,6 +858,11 @@ class WKBasedNavigationManagerDetachedModeTest
ASSERT_EQ(url2_, manager_->GetItemAtIndex(2)->GetURL()); ASSERT_EQ(url2_, manager_->GetItemAtIndex(2)->GetURL());
} }
NSString* CreateRedirectUrlForWKList(GURL url) {
GURL redirect_url = wk_navigation_util::CreateRedirectUrl(url);
return base::SysUTF8ToNSString(redirect_url.spec());
}
GURL url0_; GURL url0_;
GURL url1_; GURL url1_;
GURL url2_; GURL url2_;
...@@ -1002,4 +1007,16 @@ TEST_F(WKBasedNavigationManagerDetachedModeTest, LoadURLWithParams) { ...@@ -1002,4 +1007,16 @@ TEST_F(WKBasedNavigationManagerDetachedModeTest, LoadURLWithParams) {
histogram_tester_.ExpectBucketCount(kRestoreNavigationItemCount, 3, 1); histogram_tester_.ExpectBucketCount(kRestoreNavigationItemCount, 3, 1);
} }
// Tests that detaching placeholder urls are cleaned before being cached.
TEST_F(WKBasedNavigationManagerDetachedModeTest, CachedPlaceholders) {
[mock_wk_list_ setCurrentURL:CreateRedirectUrlForWKList(url1_)
backListURLs:@[ CreateRedirectUrlForWKList(url0_) ]
forwardListURLs:@[ CreateRedirectUrlForWKList(url2_) ]];
manager_->DetachFromWebView();
EXPECT_EQ(url0_, manager_->GetNavigationItemImplAtIndex(0)->GetURL());
EXPECT_EQ(url1_, manager_->GetNavigationItemImplAtIndex(1)->GetURL());
EXPECT_EQ(url2_, manager_->GetNavigationItemImplAtIndex(2)->GetURL());
}
} // namespace web } // namespace web
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