New issue
Advanced search Search tips

Issue 661316 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocked on:
issue 792515


Participants' hotlists:
Slim-Nav-Cleanup


Sign in to add a comment

Move navigation code from CRWWebController to web::NavigationManager

Project Member Reported by eugene...@chromium.org, Nov 1 2016

Issue description

Things like JavaScript execution can be delegated to CRWWebController via NavigationManagerDelegate.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 16 2016

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

commit f00c143bc322f36f5ff919620fd57b2e021a3262
Author: eugenebut <eugenebut@chromium.org>
Date: Wed Nov 16 02:54:56 2016

[ios] Removed back-forward navigation API from CRWWebController.

NavigationManager API should be used for navigation instead.

BUG=661316

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

[modify] https://crrev.com/f00c143bc322f36f5ff919620fd57b2e021a3262/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/f00c143bc322f36f5ff919620fd57b2e021a3262/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 16 2016

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

commit 862c1088fa95986676931fcf7231fcbd6d182fe4
Author: eugenebut <eugenebut@chromium.org>
Date: Wed Nov 16 03:45:08 2016

Revert of [ios] Removed back-forward navigation API from CRWWebController. (patchset #2 id:20001 of https://codereview.chromium.org/2499593003/ )

Reason for revert:
Breaks upstream unit test.

Original issue's description:
> [ios] Removed back-forward navigation API from CRWWebController.
>
> NavigationManager API should be used for navigation instead.
>
> BUG=661316
>
> Committed: https://crrev.com/f00c143bc322f36f5ff919620fd57b2e021a3262
> Cr-Commit-Position: refs/heads/master@{#432369}

TBR=kkhorimoto@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661316

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

[modify] https://crrev.com/862c1088fa95986676931fcf7231fcbd6d182fe4/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/862c1088fa95986676931fcf7231fcbd6d182fe4/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 16 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/18deb5359bf2856399e8bb8c1bbb8a7835dfba32

commit 18deb5359bf2856399e8bb8c1bbb8a7835dfba32
Author: eugenebut <eugenebut@google.com>
Date: Wed Nov 16 06:31:45 2016

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16 2016

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

commit 3a222d75c572b2df8a4e6c4bc0d0e5b851147ff2
Author: eugenebut <eugenebut@chromium.org>
Date: Wed Nov 16 14:52:58 2016

Reland of [ios] Removed back-forward navigation API from CRWWebController. (patchset #1 id:1 of https://codereview.chromium.org/2509493003/ )

Reason for revert:
Downstream tests were fixed. Relanding without changes.

Original issue's description:
> Revert of [ios] Removed back-forward navigation API from CRWWebController. (patchset #2 id:20001 of https://codereview.chromium.org/2499593003/ )
>
> Reason for revert:
> Breaks upstream unit test.
>
> Original issue's description:
> > [ios] Removed back-forward navigation API from CRWWebController.
> >
> > NavigationManager API should be used for navigation instead.
> >
> > BUG=661316
> >
> > Committed: https://crrev.com/f00c143bc322f36f5ff919620fd57b2e021a3262
> > Cr-Commit-Position: refs/heads/master@{#432369}
>
> TBR=kkhorimoto@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=661316
>
> Committed: https://crrev.com/862c1088fa95986676931fcf7231fcbd6d182fe4
> Cr-Commit-Position: refs/heads/master@{#432372}

TBR=kkhorimoto@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661316

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

[modify] https://crrev.com/3a222d75c572b2df8a4e6c4bc0d0e5b851147ff2/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/3a222d75c572b2df8a4e6c4bc0d0e5b851147ff2/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 16 2016

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

commit 54cd9dad1e908f83de618794b8ade832fa5db58f
Author: eugenebut <eugenebut@chromium.org>
Date: Wed Nov 16 19:37:04 2016

[ios] Removed goBack/goForward from CRWSessionController.

These methods are unused and CRWSessionController should be folded into
NavigationManager.

BUG=661316

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

[modify] https://crrev.com/54cd9dad1e908f83de618794b8ade832fa5db58f/ios/web/navigation/crw_session_controller.h
[modify] https://crrev.com/54cd9dad1e908f83de618794b8ade832fa5db58f/ios/web/navigation/crw_session_controller.mm
[modify] https://crrev.com/54cd9dad1e908f83de618794b8ade832fa5db58f/ios/web/navigation/crw_session_controller_unittest.mm

Labels: -Type-Feature Type-Task
Labels: Proj-WKBackForwardList
Owner: danyao@chromium.org
Project Member

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

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

commit 5f500189525f074e490de61daedd091604b9cebc
Author: Danyao Wang <danyao@google.com>
Date: Tue Aug 15 17:47:55 2017

[Nav Experiment] Move logic in |-loadWithParams| to NavigationManager.

|-loadWithParams| in CRWWebController contains primarily navigation code
that belongs better in NavigationManagerImpl. Refactoring this code
allows LegacyNavigationManagerImpl and WKBasedNavigationManagerImpl to
share the implementation of LoadURLWithParams().

Notable changes:
1. Exposed |-recordStateInHistory|, |-clearTransientContentView|
   functionalities of CRWWebController via  NavigationManagerDelegate.
2. Broke the cycle between WebStateImpl::ClearTransientContentView
   and CRWWebController |-clearTransientContentView| so only the former
   calls the latter. Updateded dependencies.

Bug: 734150, 661316
Change-Id: Id85f6fa6c433fa31697a1a5ecbb50cb855d898ac
Reviewed-on: https://chromium-review.googlesource.com/614110
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494436}
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/interstitials/web_interstitial_impl.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/navigation_manager_delegate.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/test/fakes/test_navigation_manager_delegate.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/test/fakes/test_navigation_manager_delegate.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/5f500189525f074e490de61daedd091604b9cebc/ios/web/web_state/web_state_impl.mm

Project Member

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

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

commit 0f31f8aee72ccf73289391cee5aa85d1764dedb6
Author: Danyao Wang <danyao@google.com>
Date: Thu Aug 17 22:24:46 2017

[Nav Experiment] Move |-goToItemAtIndex| to NavigationManagerImpl.

Common logic that does not depend on CRWSessionController is moved to
NavigationManagerImpl. The rest is pushed into GoToIndexImpl() in the
sub classes so either session controller or WKBackForwardList is used.

Bug: 734150, 661316
Change-Id: I9959e89e3b3314f784d25ad6c59e62a504d9dded
Reviewed-on: https://chromium-review.googlesource.com/616235
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495352}
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/navigation_manager_delegate.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/test/fakes/test_navigation_manager_delegate.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/test/fakes/test_navigation_manager_delegate.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/0f31f8aee72ccf73289391cee5aa85d1764dedb6/ios/web/web_state/web_state_impl.mm

Cc: danyao@chromium.org
Owner: ----
Blockedon: 792515
Components: -Mobile>WebView>Glue

Sign in to add a comment