Commit 8df7c018 authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[Nav Experiment] Set back-forward bit in NavigationItem transition type.

Fix a bug where WKBasedNavigationManager is not setting the
ui::PAGE_TRANSITION_FORWARD_BACK bit in an existing NavigationItem in
history navigation.

The legacy navigation manager doesn't set this bit for same-document
back-forward navigation. This seems to be a bug, but will be fixed in
a separate CL to make possible bisecting easier.

Bug: 798832
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I2967256a32027384e16b03c24fdc0ee899a3d61d
Reviewed-on: https://chromium-review.googlesource.com/849352Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526844}
parent 063efefe
...@@ -2227,6 +2227,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentDocument) { ...@@ -2227,6 +2227,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentDocument) {
EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex()); EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex());
EXPECT_EQ(-1, navigation_manager()->GetPendingItemIndex()); EXPECT_EQ(-1, navigation_manager()->GetPendingItemIndex());
EXPECT_FALSE(navigation_manager()->GetItemAtIndex(0)->GetTransitionType() &
ui::PAGE_TRANSITION_FORWARD_BACK);
EXPECT_CALL(navigation_manager_delegate(), RecordPageStateInNavigationItem()); EXPECT_CALL(navigation_manager_delegate(), RecordPageStateInNavigationItem());
EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent()); EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent());
...@@ -2240,6 +2242,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentDocument) { ...@@ -2240,6 +2242,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentDocument) {
} }
navigation_manager()->GoToIndex(0); navigation_manager()->GoToIndex(0);
EXPECT_TRUE(navigation_manager()->GetItemAtIndex(0)->GetTransitionType() &
ui::PAGE_TRANSITION_FORWARD_BACK);
if (GetParam() == TEST_LEGACY_NAVIGATION_MANAGER) { if (GetParam() == TEST_LEGACY_NAVIGATION_MANAGER) {
// Since LoadCurrentItem() is noop in test, we can only verify that the // Since LoadCurrentItem() is noop in test, we can only verify that the
...@@ -2270,6 +2274,8 @@ TEST_P(NavigationManagerTest, GoToIndexSameDocument) { ...@@ -2270,6 +2274,8 @@ TEST_P(NavigationManagerTest, GoToIndexSameDocument) {
EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex()); EXPECT_EQ(1, navigation_manager()->GetLastCommittedItemIndex());
EXPECT_EQ(-1, navigation_manager()->GetPendingItemIndex()); EXPECT_EQ(-1, navigation_manager()->GetPendingItemIndex());
EXPECT_FALSE(navigation_manager()->GetItemAtIndex(0)->GetTransitionType() &
ui::PAGE_TRANSITION_FORWARD_BACK);
EXPECT_CALL(navigation_manager_delegate(), RecordPageStateInNavigationItem()); EXPECT_CALL(navigation_manager_delegate(), RecordPageStateInNavigationItem());
EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent()); EXPECT_CALL(navigation_manager_delegate(), ClearTransientContent());
...@@ -2282,6 +2288,12 @@ TEST_P(NavigationManagerTest, GoToIndexSameDocument) { ...@@ -2282,6 +2288,12 @@ TEST_P(NavigationManagerTest, GoToIndexSameDocument) {
} }
navigation_manager()->GoToIndex(0); navigation_manager()->GoToIndex(0);
// Preserve the existing behavior of legacy navigation manager for change
// management, even though it seems like a bug that the back-forward
// transition bit is not set for same-document history navigation.
EXPECT_EQ(GetParam() == TEST_WK_BASED_NAVIGATION_MANAGER,
(navigation_manager()->GetItemAtIndex(0)->GetTransitionType() &
ui::PAGE_TRANSITION_FORWARD_BACK) > 0);
if (GetParam() == TEST_LEGACY_NAVIGATION_MANAGER) { if (GetParam() == TEST_LEGACY_NAVIGATION_MANAGER) {
EXPECT_EQ(0, navigation_manager()->GetLastCommittedItemIndex()); EXPECT_EQ(0, navigation_manager()->GetLastCommittedItemIndex());
...@@ -2309,6 +2321,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentUserAgentType) { ...@@ -2309,6 +2321,8 @@ TEST_P(NavigationManagerTest, GoToIndexDifferentUserAgentType) {
EXPECT_CALL(navigation_manager_delegate(), WillChangeUserAgentType()); EXPECT_CALL(navigation_manager_delegate(), WillChangeUserAgentType());
navigation_manager()->GoToIndex(0); navigation_manager()->GoToIndex(0);
EXPECT_TRUE(navigation_manager()->GetItemAtIndex(0)->GetTransitionType() &
ui::PAGE_TRANSITION_FORWARD_BACK);
} }
TEST_P(NavigationManagerTest, LoadIfNecessary) { TEST_P(NavigationManagerTest, LoadIfNecessary) {
......
...@@ -456,6 +456,9 @@ NavigationItemImpl* WKBasedNavigationManagerImpl::GetTransientItemImpl() const { ...@@ -456,6 +456,9 @@ NavigationItemImpl* WKBasedNavigationManagerImpl::GetTransientItemImpl() const {
void WKBasedNavigationManagerImpl::FinishGoToIndex(int index) { void WKBasedNavigationManagerImpl::FinishGoToIndex(int index) {
DiscardNonCommittedItems(); DiscardNonCommittedItems();
NavigationItem* item = GetItemAtIndex(index);
item->SetTransitionType(ui::PageTransitionFromInt(
item->GetTransitionType() | ui::PAGE_TRANSITION_FORWARD_BACK));
WKBackForwardListItem* wk_item = GetWKItemAtIndex(index); WKBackForwardListItem* wk_item = GetWKItemAtIndex(index);
DCHECK(wk_item); DCHECK(wk_item);
[delegate_->GetWebViewNavigationProxy() goToBackForwardListItem:wk_item]; [delegate_->GetWebViewNavigationProxy() goToBackForwardListItem:wk_item];
......
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