webView:didCommitNavigation: handles correctly only latest navigaiton |
||||||
Issue descriptionWKWebView may have more than one in progress navigation. webView:didCommitNavigation: assumes that only the last navigation can be committed, which is not true. webView:didCommitNavigation: should be fixed to handle non-latest committed navigation as well.
,
Apr 17 2017
When can WKWebView have more than one in-progress navigation? Conceptually, that doesn't make much sense to me; is it just a matter of the WKNavigation callbacks being interleaved such that a new navigation is committed before we receive the didFail callback for the previous one? In your suggestion above, how do we choose the corresponding NavigationItem if we only store one pending item? Is this specific to quickly tapping the back/forward buttons?
,
Apr 18 2017
>> When can WKWebView have more than one in-progress navigation? When we ask WKWebView to navigate twice in a raw (f.e. double tab on back button). This is known situation, we just don't handle it everywhere: https://cs.chromium.org/chromium/src/ios/web/web_state/ui/crw_web_controller.mm?q=crw_web_c+package:%5Echromium$&l=4485 For double tap, both navigation may actually succeed, so it's not the case that older navigation fails and newer succeeds. We already store multiple WKNavigation objects, so my proposal is to associate them with NavigationContext and every navigation context will have a pointer to NavigationItem. >>> Is this specific to quickly tapping the back/forward buttons? That's the only use case I observed, but I believe there can be other cases as well.
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168 commit 5ef672aa5ffed4146ede2c1d22a04fb91dd5f168 Author: eugenebut <eugenebut@chromium.org> Date: Mon May 08 18:58:35 2017 Store NavigationContext in CRWWKNavigationStates. This CL (1 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL adds API to CRWWKNavigationStates to allow associating NavigationContextImpl with NKNavigation objects. This will allow to understand which NavigationContextImpl should be used in WKNavigationDelegate callbacks. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2835463002 Cr-Commit-Position: refs/heads/master@{#470068} [modify] https://crrev.com/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/5ef672aa5ffed4146ede2c1d22a04fb91dd5f168/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e92530274bb519ab0480a83f877ef62b8e681897 commit e92530274bb519ab0480a83f877ef62b8e681897 Author: eugenebut <eugenebut@chromium.org> Date: Mon May 08 21:45:31 2017 Return WKNavigation from loadRequest and loadPostRequest. This CL (2 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL makes load*Request API return WKNavigation object, which can be used to store NavigationContext in CRWWKNavigationStates. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2840473003 Cr-Commit-Position: refs/heads/master@{#470140} [modify] https://crrev.com/e92530274bb519ab0480a83f877ef62b8e681897/ios/web/web_state/js/crw_js_post_request_loader.h [modify] https://crrev.com/e92530274bb519ab0480a83f877ef62b8e681897/ios/web/web_state/js/crw_js_post_request_loader.mm [modify] https://crrev.com/e92530274bb519ab0480a83f877ef62b8e681897/ios/web/web_state/ui/crw_web_controller.mm
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f97229b40443d92c57c3a08ebef8aff466932da0 commit f97229b40443d92c57c3a08ebef8aff466932da0 Author: eugenebut <eugenebut@chromium.org> Date: Tue May 09 04:46:31 2017 Return NavigationContext from registerLoadRequest. This CL (3 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL makes registerLoadRequest API return NavigationContext object, which can be used to store NavigationContext in CRWWKNavigationStates. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2834413002 Cr-Commit-Position: refs/heads/master@{#470223} [modify] https://crrev.com/f97229b40443d92c57c3a08ebef8aff466932da0/ios/web/web_state/ui/crw_web_controller.mm
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1000a00b446127c2c0148fe11b2bf2b8223e85b4 commit 1000a00b446127c2c0148fe11b2bf2b8223e85b4 Author: eugenebut <eugenebut@chromium.org> Date: Tue May 09 13:52:33 2017 Added WebStateImpl::OnNavigationFinished(). This CL (4 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL adds WebStateimpl::OnNavigationFinished which simply calls WebStateObserver::DidFinishNavigation. This method will be used to pass NavigationContext (which will be created earlier) to DidFinishNavigation. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2833423002 Cr-Commit-Position: refs/heads/master@{#470314} [modify] https://crrev.com/1000a00b446127c2c0148fe11b2bf2b8223e85b4/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/1000a00b446127c2c0148fe11b2bf2b8223e85b4/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/1000a00b446127c2c0148fe11b2bf2b8223e85b4/ios/web/web_state/web_state_impl_unittest.mm
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6a32ecf073cb1616e51c927b2bfdad46a539c31 commit c6a32ecf073cb1616e51c927b2bfdad46a539c31 Author: eugenebut <eugenebut@chromium.org> Date: Tue May 09 17:39:15 2017 Added Setters to NavigationContextImpl. This CL (5 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL adds setters to NavigationContextImpl, which will be created only with web state and URL and will be configured via setters before committing. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2838583002 Cr-Commit-Position: refs/heads/master@{#470372} [modify] https://crrev.com/c6a32ecf073cb1616e51c927b2bfdad46a539c31/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/c6a32ecf073cb1616e51c927b2bfdad46a539c31/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/c6a32ecf073cb1616e51c927b2bfdad46a539c31/ios/web/web_state/navigation_context_impl_unittest.mm
,
May 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c79b906de7b97f04ea867398b884ec9c0341839 commit 2c79b906de7b97f04ea867398b884ec9c0341839 Author: eugenebut <eugenebut@chromium.org> Date: Tue May 09 23:49:51 2017 Allow storing null navigations in CRWWKNavigationStates. This CL (6 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). WKWebView passes null WKNavigation to WKNavigationDelegate callbacks for window opening navigation actions. This is a normal flow so CRWWKNavigationStates should be able to store null navigations. Because there can be at most one window opening navigation it is safe to use unique key for storing null navigation. BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2838593002 Cr-Commit-Position: refs/heads/master@{#470439} [modify] https://crrev.com/2c79b906de7b97f04ea867398b884ec9c0341839/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/2c79b906de7b97f04ea867398b884ec9c0341839/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/2c79b906de7b97f04ea867398b884ec9c0341839/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
May 10 2017
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/756b09442132dec9b01cbeda19d0a8d69e8dd551 commit 756b09442132dec9b01cbeda19d0a8d69e8dd551 Author: eugenebut <eugenebut@chromium.org> Date: Wed May 10 20:28:39 2017 Create NavigationContext right after the navigation was started. This CL (7 of 7) is a part of large refactoring to create NavigationContext when navigation was started, not when it is finished. This refactoring will allow to pass the same instance of NavigationContext to DidStartNavigation and DidFinishNavigation ( crbug.com/713836 ). Also this will allow to associate NavigationItem with NavigationContext object to commit non-latest navigation ( crbug.com/712269 ). This CL changes the time of NavigationContext creation. Subsequent CLs will fix: - crbug.com/713836 by passing NavigationContext to OnProvisionalNavigationStarted - crbug.com/712269 by looking up for pending NavigationContext and extracting NavigationItem from it BUG= 713836 , 712269 Review-Url: https://codereview.chromium.org/2842443002 Cr-Commit-Position: refs/heads/master@{#470687} [modify] https://crrev.com/756b09442132dec9b01cbeda19d0a8d69e8dd551/ios/web/web_state/ui/crw_web_controller.h [modify] https://crrev.com/756b09442132dec9b01cbeda19d0a8d69e8dd551/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/756b09442132dec9b01cbeda19d0a8d69e8dd551/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
May 12 2017
,
May 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f66b4a55804cd494ec380c0a08446656ce512b9 commit 1f66b4a55804cd494ec380c0a08446656ce512b9 Author: eugenebut <eugenebut@chromium.org> Date: Mon May 15 21:23:25 2017 Store NavigationItem's unique ID in navigation context. This allows to correctly commit non-latest pending navigation in webView:didCommitNavigation: BUG= 712269 Review-Url: https://codereview.chromium.org/2881933003 Cr-Commit-Position: refs/heads/master@{#471905} [modify] https://crrev.com/1f66b4a55804cd494ec380c0a08446656ce512b9/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/1f66b4a55804cd494ec380c0a08446656ce512b9/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/1f66b4a55804cd494ec380c0a08446656ce512b9/ios/web/web_state/ui/crw_web_controller.mm
,
May 15 2017
,
May 16 2017
,
Aug 22 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by eugene...@chromium.org
, Apr 17 2017