HistoryStateOperationsTests fail with WKBasedNavigationManager |
|||
Issue descriptionReplaceStateNoHashChangeEvent, ReplaceStatePostRequest, and StateReplacement all fail because they're expecting the NavigationItemImpl corresponding to the last committed item to change, but WKBasedNavigationManagerImpl::GetNavigationItemImplAtIndex returns an existing NavigationItemImpl as-is when one already exists for a particular wk_item. So any changes in the wk_item (like its url) aren't reflected in the returned NavigationItemImpl.
,
Nov 9 2017
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23a351aeb879eb30e02ca06afb721f9bfbb4df25 commit 23a351aeb879eb30e02ca06afb721f9bfbb4df25 Author: Danyao Wang <danyao@chromium.org> Date: Fri Nov 10 19:49:59 2017 [Nav Experiment] Re-enable same-document handling for new nav manager. Same-document handling (URL, loading KVO and pushState/replaceState override) was originally disabled when using WKBasedNavigationManager because I thought their primary purpose was to update the navigation history, which is no longer needed. I overlooked that this code path also triggers WebStateObserver callbacks, and current contract requires that they be triggered for same-document navigations. Although we have plans to update the contract (e.g. crbug.com/767092), it would be done after switching to new navigation manager. Summary of the change: - Split overrides that are not needed and incompatible with new nav manager from navigation.js to legacy_navigation.js. - Added navigation.js back to web_bundle so it is injected even when new nav manager is used. - Removed condition gating URL and loading KVO - Moved CRWSessionController methods used by CRWWebController's state navigation handlers to NavigationManager (i.e. |updateCurrentItem| and |pushNewItemWithURL|. This change fixes the following tests that were failing with new nav manager: NavigationCallbacksTest HistoryStateOperationsTest Bug: 776373 , 776486 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Ife803390b42ca30098deeea9ce35487bc33b1ed7 Reviewed-on: https://chromium-review.googlesource.com/762176 Commit-Queue: Danyao Wang <danyao@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#515643} [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/BUILD.gn [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/crw_session_controller.h [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/crw_session_controller.mm [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/crw_session_controller_unittest.mm [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/legacy_navigation_manager_impl.h [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/legacy_navigation_manager_impl.mm [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/navigation_manager_impl.h [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/navigation_manager_impl.mm [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/navigation_manager_impl_unittest.mm [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/wk_based_navigation_manager_impl.h [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/navigation/wk_based_navigation_manager_impl.mm [add] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/web_state/js/resources/legacy_navigation.js [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/web_state/js/resources/navigation.js [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/web_state/js/resources/web_bundle.js [modify] https://crrev.com/23a351aeb879eb30e02ca06afb721f9bfbb4df25/ios/web/web_state/ui/crw_web_controller.mm
,
Nov 10 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by danyao@chromium.org
, Nov 9 2017