New issue
Advanced search Search tips

Issue 713836 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Task

Blocking:
issue 674991



Sign in to add a comment

Implement WebStateObserver::DidStartNavigation(NavigationContext*)

Project Member Reported by eugene...@chromium.org, Apr 20 2017

Issue description

This mirrors contents's WebContentsObserver::DidStartNavigation(NavigationHandle*)

and should replace WebContentsObserver::DidStartProvisionalNavigation(GURL)
 
Project Member

Comment 1 by bugdroid1@chromium.org, 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

Project Member

Comment 2 by bugdroid1@chromium.org, 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

Project Member

Comment 3 by bugdroid1@chromium.org, 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

Project Member

Comment 4 by bugdroid1@chromium.org, 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

Project Member

Comment 5 by bugdroid1@chromium.org, 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

Project Member

Comment 6 by bugdroid1@chromium.org, 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

Labels: -Pri-3 Pri-2
Project Member

Comment 8 by bugdroid1@chromium.org, 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

Project Member

Comment 9 by bugdroid1@chromium.org, May 10 2017

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

commit 670d9f911edaeba050e8077861129c6d3f48496a
Author: eugenebut <eugenebut@chromium.org>
Date: Wed May 10 21:55:15 2017

Call WebStateObserver::ProvisionalNavigationStarted for state navigations

ProvisionalNavigationStarted goes in pair with DidFinishNavigation,
which is called for push and replace state operations.

BUG= 713836 

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

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

Project Member

Comment 10 by bugdroid1@chromium.org, May 17 2017

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

commit 7ebdb138f42c121d85e25821a24f864e8a380d05
Author: eugenebut <eugenebut@chromium.org>
Date: Wed May 17 19:18:06 2017

Added -[CRWWKNavigationStates pendingNavigations] API.

This method returns all navigations that are currently pending
(REQUESTED, STARTED, REDIRECTED), including null navigations, which are
represented with NSNull.

This API will be used in dependent CL to allow looking up for a pending
navigation by URL.

BUG= 713836 

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

[modify] https://crrev.com/7ebdb138f42c121d85e25821a24f864e8a380d05/ios/web/web_state/ui/crw_wk_navigation_states.h
[modify] https://crrev.com/7ebdb138f42c121d85e25821a24f864e8a380d05/ios/web/web_state/ui/crw_wk_navigation_states.mm
[modify] https://crrev.com/7ebdb138f42c121d85e25821a24f864e8a380d05/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm

Project Member

Comment 11 by bugdroid1@chromium.org, May 17 2017

Project Member

Comment 12 by bugdroid1@chromium.org, May 18 2017

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

commit 1ba5140bd27b3ad9542e029ded6068a79dc56a3c
Author: eugenebut <eugenebut@chromium.org>
Date: Thu May 18 18:57:16 2017

Look up for pending navigation contexts instead of creating new ones.

Use -[CRWWKNavigationStates pendingNavigations] to get all pending
navigations and match pending context by URL.

BUG= 713836 

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

[modify] https://crrev.com/1ba5140bd27b3ad9542e029ded6068a79dc56a3c/ios/web/web_state/ui/crw_web_controller.mm

Status: Fixed (was: Started)

Sign in to add a comment