Commit b984a9ae authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Fix crash in GetLastCommittedItemInCurrentOrRestoredSession

This CL fixes a crash that happens when we are checking if the last
committed item has the same URL as the current/back/forward item and
then updates its UserAgent.
This fix wasn't fully working. The correct fix is crrev.com/c/2193994.
Removing the code allows to cleanup the code for the correct fix, and
remove the crash.
As it wasn't really fixing the issue in the first place, the regression
isn't an issue.

Fixed: 1085348
Change-Id: I7c45ab5709e70f31edfc5eb3989f2c235ddbbac1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2215846
Auto-Submit: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771828}
parent e8d70a08
...@@ -869,34 +869,8 @@ WKBasedNavigationManagerImpl::GetLastCommittedItemInCurrentOrRestoredSession() ...@@ -869,34 +869,8 @@ WKBasedNavigationManagerImpl::GetLastCommittedItemInCurrentOrRestoredSession()
DCHECK_EQ(0, GetItemCount()); DCHECK_EQ(0, GetItemCount());
return nullptr; return nullptr;
} }
bool existing_last_committed_item =
web_view_cache_.GetNavigationItemImplAtIndex(
index, false /* create_if_missing */) != nullptr;
NavigationItemImpl* last_committed_item = NavigationItemImpl* last_committed_item =
GetNavigationItemImplAtIndex(static_cast<size_t>(index)); GetNavigationItemImplAtIndex(static_cast<size_t>(index));
id<CRWWebViewNavigationProxy> proxy = delegate_->GetWebViewNavigationProxy();
if (!existing_last_committed_item &&
![proxy.URL isEqual:proxy.backForwardList.currentItem.URL]) {
// In some cases (especially google websites),
// webViewBackForwardStateDidChange callback is called first, leading to
// creating a NavigationItem to match the WKBackForwardList and considering
// that it is committed. In that case, the URL of the WebView isn't matching
// the one of the |wk_item|. It is possible to use this fact to detect this
// case and have this new item inheriting the UserAgent of the previous
// page. See https://crbug.com/1049094 for more details.
NavigationItemImpl* current_committed_item = nullptr;
if ([proxy.backForwardList.backItem.URL isEqual:proxy.URL]) {
current_committed_item =
GetNavigationItemFromWKItem(proxy.backForwardList.backItem);
} else if ([proxy.backForwardList.forwardItem.URL isEqual:proxy.URL]) {
current_committed_item =
GetNavigationItemFromWKItem(proxy.backForwardList.forwardItem);
}
if (current_committed_item) {
last_committed_item->SetUserAgentType(
current_committed_item->GetUserAgentForInheritance());
}
}
if (last_committed_item && GetWebState() && if (last_committed_item && GetWebState() &&
!CanTrustLastCommittedItem(last_committed_item)) { !CanTrustLastCommittedItem(last_committed_item)) {
// Don't check trust level here, as at this point it's expected // Don't check trust level here, as at this point it's expected
......
...@@ -200,25 +200,6 @@ TEST_F(WKBasedNavigationManagerTest, SyncInGetLastCommittedItem) { ...@@ -200,25 +200,6 @@ TEST_F(WKBasedNavigationManagerTest, SyncInGetLastCommittedItem) {
EXPECT_FALSE(item->GetTimestamp().is_null()); EXPECT_FALSE(item->GetTimestamp().is_null());
} }
// Tests that GetLastCommittedItem() creates a default NavigationItem inheriting
// the UserAgent of the previous items when calling GetLastCommittedItem with
// the URL of the WebView not being the same as the URL of the current item.
// This can happpen on some navigation see https://crbug.com/1049094 .
TEST_F(WKBasedNavigationManagerTest, SyncInGetLastCommittedItemOffSyncWebView) {
OCMStub([mock_web_view_ URL])
.andReturn([[NSURL alloc] initWithString:@"http://www.0.com"]);
[mock_wk_list_ setCurrentURL:@"http://www.1.com"
backListURLs:@[ @"http://www.0.com" ]
forwardListURLs:nil];
manager_->GetItemAtIndex(0)->SetUserAgentType(UserAgentType::DESKTOP);
NavigationItem* item = manager_->GetLastCommittedItem();
ASSERT_NE(item, nullptr);
EXPECT_EQ("http://www.1.com/", item->GetURL().spec());
EXPECT_EQ(UserAgentType::DESKTOP, item->GetUserAgentType(nil));
EXPECT_EQ(UserAgentType::DESKTOP, item->GetUserAgentForInheritance());
}
// Tests that GetLastCommittedItem() creates a default NavigationItem when the // Tests that GetLastCommittedItem() creates a default NavigationItem when the
// last committed item in WKWebView is an app-specific URL. // last committed item in WKWebView is an app-specific URL.
TEST_F(WKBasedNavigationManagerTest, TEST_F(WKBasedNavigationManagerTest,
......
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