Skip to content

Commit

Permalink
Revert of Always set the navigation pending item for page reload. (pa…
Browse files Browse the repository at this point in the history
…tchset #6 id:220001 of https://codereview.chromium.org/2931833004/ )

Reason for revert:
introduced crbug/750775

Original issue's description:
> Always set the navigation pending item for page reload.
>
> Set the pendingItemIndex to the last committed item in case of page reload
> and on the case of application open/tab out of memory reopened.
>
> This doesn't solve the PendingItem being null problem entirely there are still
> couple cases when that can happen and they are not page reloads.
>
> CL 1/2
>
> BUG=676129
>
> Review-Url: https://codereview.chromium.org/2931833004
> Cr-Commit-Position: refs/heads/master@{#486777}
> Committed: https://chromium.googlesource.com/chromium/src/+/bc2e0a45e23c739bb3c04d119d43abbb01398289

TBR=eugenebut@chromium.org,kkhorimoto@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=676129

Review-Url: https://codereview.chromium.org/2998463002
Cr-Commit-Position: refs/heads/master@{#492138}
  • Loading branch information
mrefaat88 authored and Commit Bot committed Aug 4, 2017
1 parent 72b0d83 commit d7d7325
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 26 deletions.
17 changes: 12 additions & 5 deletions ios/web/web_state/navigation_callbacks_inttest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,15 @@
EXPECT_FALSE((*context)->GetError());
ASSERT_FALSE((*context)->GetResponseHeaders());
ASSERT_TRUE(web_state->IsLoading());
NavigationManager* navigation_manager = web_state->GetNavigationManager();
NavigationItem* item = navigation_manager->GetPendingItem();
EXPECT_EQ(url, item->GetURL());
// TODO(crbug.com/676129): Reload does not create a pending item. Remove this
// workaround once the bug is fixed.
if (!ui::PageTransitionTypeIncludingQualifiersIs(
ui::PageTransition::PAGE_TRANSITION_RELOAD,
(*context)->GetPageTransition())) {
NavigationManager* navigation_manager = web_state->GetNavigationManager();
NavigationItem* item = navigation_manager->GetPendingItem();
EXPECT_EQ(url, item->GetURL());
}
}

// Verifies correctness of |NavigationContext| (|arg0|) for navigations via POST
Expand Down Expand Up @@ -211,8 +217,9 @@
EXPECT_FALSE((*context)->IsSameDocument());
EXPECT_FALSE((*context)->GetError());
EXPECT_FALSE((*context)->GetResponseHeaders());
EXPECT_EQ(web_state->GetNavigationManager()->GetPendingItem(),
web_state->GetNavigationManager()->GetLastCommittedItem());
// TODO(crbug.com/676129): Reload does not create a pending item. Check
// pending item once the bug is fixed.
EXPECT_FALSE(web_state->GetNavigationManager()->GetPendingItem());
}

// Verifies correctness of |NavigationContext| (|arg0|) for reload navigation
Expand Down
26 changes: 5 additions & 21 deletions ios/web/web_state/ui/crw_web_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1517,18 +1517,11 @@ - (double)loadingProgress {
// Typically on PAGE_TRANSITION_CLIENT_REDIRECT.
[[self sessionController] updatePendingItem:requestURL];
} else {
// If this is a reload then there no need to create a new pending item,
// instead update the pending item to the last committed item.
if (PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD)) {
self.sessionController.pendingItemIndex =
self.sessionController.lastCommittedItemIndex;
} else {
// A new session history entry needs to be created.
self.navigationManagerImpl->AddPendingItem(
requestURL, referrer, transition,
web::NavigationInitiationType::RENDERER_INITIATED,
web::NavigationManager::UserAgentOverrideOption::INHERIT);
}
// A new session history entry needs to be created.
self.navigationManagerImpl->AddPendingItem(
requestURL, referrer, transition,
web::NavigationInitiationType::RENDERER_INITIATED,
web::NavigationManager::UserAgentOverrideOption::INHERIT);
}
std::unique_ptr<web::NavigationContextImpl> context =
web::NavigationContextImpl::CreateNavigationContext(
Expand Down Expand Up @@ -1986,11 +1979,6 @@ - (void)triggerPendingLoad {
BOOL isChromeScheme =
web::GetWebClient()->IsAppSpecificURL(currentNavigationURL);

// Since this is implicit reload, no new pending item should be created, set
// the pending item index to the last committed item.
self.sessionController.pendingItemIndex =
self.sessionController.lastCommittedItemIndex;

// Don't immediately load the web page if in overlay mode. Always load if
// native.
if (isChromeScheme || !_overlayPreviewMode) {
Expand Down Expand Up @@ -2032,10 +2020,6 @@ - (void)reload {
_lastUserInteraction.reset();
base::RecordAction(UserMetricsAction("Reload"));
GURL url = self.currentNavItem->GetURL();
// Reloading shouldn't create create a new pending item, instead set the
// pending item index to the last committed item.
self.sessionController.pendingItemIndex =
self.sessionController.lastCommittedItemIndex;
if ([self shouldLoadURLInNativeView:url]) {
std::unique_ptr<web::NavigationContextImpl> navigationContext = [self
registerLoadRequestForURL:url
Expand Down

0 comments on commit d7d7325

Please sign in to comment.