New issue
Advanced search Search tips

Issue 676129 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug

Blocked on:
issue 730192



Sign in to add a comment

CRWSessionController does not always create a pending entry

Project Member Reported by eugene...@chromium.org, Dec 20 2016

Issue description

addPendingEntry:referrer:transition:rendererInitiated: does not create a pending entry if URL is the same. This is probably fine, but it's not OK, that sessioController.pendingEntry is nil after |addPendingEntry:| call. 

The right thing is probably setting pendingEntryIndex to currentNavigationIndex.
 
The item should return ui::PAGE_TRANSITION_RELOAD on GetTransitionType().
I am not sure the proposed solution will do it.
I guess ui::PAGE_TRANSITION_RELOAD should be set by -[CRWWebController reloadInternal] method.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2ccf5b200be3027dd7b67346574689fc483500f

commit f2ccf5b200be3027dd7b67346574689fc483500f
Author: liaoyuke <liaoyuke@chromium.org>
Date: Wed Apr 05 23:30:52 2017

Reland of Set user agent type of transient item the same as pending item

The original CL assumes that a pending item should always exists when
adding a transient item, however this sometimes breaks because as
described in crbug.com/676129, NavigationManagerImpl or
CRWSessionController don't create a pending item if its URL is the same
as the last committed item.

This CL fixes the problem by relaxing the condition to the following:
if pending item exists, use pending item's user agent type, otherwise
use last committed non-native item's user agent type.

BUG=676129

Review-Url: https://codereview.chromium.org/2805453002
Cr-Commit-Position: refs/heads/master@{#462271}

[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/chrome/browser/ssl/ios_ssl_error_handler_unittest.mm
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/interstitials/web_interstitial_impl.mm
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/public/test/web_test_with_web_state.h
[modify] https://crrev.com/f2ccf5b200be3027dd7b67346574689fc483500f/ios/web/public/test/web_test_with_web_state.mm

Cc: -eugene...@chromium.org
Owner: eugene...@chromium.org
Status: Assigned (was: Available)
Will try to fix this in Q3
Issue 546400 has been merged into this issue.
Cc: eugene...@chromium.org
Owner: mrefaat@chromium.org
Blockedon: 730192
Cc: danyao@chromium.org
Labels: Proj-Not-WKBackForwardList
Status: Started (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc2e0a45e23c739bb3c04d119d43abbb01398289

commit bc2e0a45e23c739bb3c04d119d43abbb01398289
Author: mrefaat <mrefaat@chromium.org>
Date: Fri Jul 14 16:38:00 2017

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}

[modify] https://crrev.com/bc2e0a45e23c739bb3c04d119d43abbb01398289/ios/web/web_state/navigation_callbacks_inttest.mm
[modify] https://crrev.com/bc2e0a45e23c739bb3c04d119d43abbb01398289/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d7d7325907b8c0920e7a8705f5df9f17432ae1c8

commit d7d7325907b8c0920e7a8705f5df9f17432ae1c8
Author: mrefaat <mrefaat@chromium.org>
Date: Fri Aug 04 22:00:48 2017

Revert of Always set the navigation pending item for page reload. (patchset #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}

[modify] https://crrev.com/d7d7325907b8c0920e7a8705f5df9f17432ae1c8/ios/web/web_state/navigation_callbacks_inttest.mm
[modify] https://crrev.com/d7d7325907b8c0920e7a8705f5df9f17432ae1c8/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 11 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2bc90b1fa16f1b9ac306e33ceb3c8e8813f6b469

commit 2bc90b1fa16f1b9ac306e33ceb3c8e8813f6b469
Author: Eugene But <eugenebut@chromium.org>
Date: Fri Aug 11 18:10:37 2017

Revert of Always set the navigation pending item for page reload. (patchset #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, mrefaat@chromium.org
BUG=676129

(cherry picked from commit d7d7325907b8c0920e7a8705f5df9f17432ae1c8)

Review-Url: https://codereview.chromium.org/2998463002
Cr-Original-Commit-Position: refs/heads/master@{#492138}
Change-Id: I53f42b2ed052a05fcfdf429a9098e77bfd4d70c2
Reviewed-on: https://chromium-review.googlesource.com/612486
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#495}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/2bc90b1fa16f1b9ac306e33ceb3c8e8813f6b469/ios/web/web_state/navigation_callbacks_inttest.mm
[modify] https://crrev.com/2bc90b1fa16f1b9ac306e33ceb3c8e8813f6b469/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd3bffc75041f47d403dae70832447c766bfd691

commit dd3bffc75041f47d403dae70832447c766bfd691
Author: Eugene But <eugenebut@google.com>
Date: Wed Oct 17 17:44:00 2018

Small fixes for navigation callbacks.

This CL fixes NavigationContext::IsRendererInitiated for reloads and
WebStatePolicyDecider::RequestInfo::transition_type for
browser-initiated navigation.

Bug: 676129, 725239
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I608b208d204bc055fcfd6c34dd7bd2a04b9c232a
Reviewed-on: https://chromium-review.googlesource.com/c/1282448
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600460}
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/web/web_state/web_state_observer_inttest.mm
[modify] https://crrev.com/dd3bffc75041f47d403dae70832447c766bfd691/ios/web_view/test/navigation_delegate_inttest.mm

Sign in to add a comment