New issue
Advanced search Search tips

Issue 661983 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task



Sign in to add a comment

Once more of the navigation state has moved into WC, replace "this" with WCO

Project Member Reported by stkhapugin@chromium.org, Nov 3 2016

Issue description

See todo in tab_model.h. The original todo was:

// TODO(stuartmorgan): once more of the navigation state has moved into WC,
// replace this with WCO.
- (void)navigationCommittedInTab:(Tab*)tab;

I couldn't decrypt what WCO is, and I guess the WC is WebController? 
 
Cc: eugene...@chromium.org
Labels: -Type-Bug Type-Feature
Owner: ----
Status: Available (was: Unconfirmed)
WebControllerObserver, which should not be used. We should use WebStateObserver instead.
Cc: stkhapugin@chromium.org
 Issue 661981  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 3 2017

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

commit 573bd7b3152aba99ff74ad02ae1aa846ad32a448
Author: sdefresne <sdefresne@chromium.org>
Date: Fri Mar 03 00:12:49 2017

Remove NavigationManager::GetPreviousItem().

The only location interested in the previous navigation item was
a callback that received the index of the item via its parameter
so use the parameter to access the navigation item and remove the
obsolete method.

Add some helper methods to CRWSessionController to avoid creating
and then destroying std::vector<> everytime the list of navigation
items was queried.

BUG= 661983 

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

[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/chrome/browser/tabs/tab.mm
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/chrome/browser/tabs/tab_model.h
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/chrome/browser/tabs/tab_model.mm
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/web/navigation/crw_session_controller.h
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/web/navigation/crw_session_controller.mm
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/web/navigation/navigation_manager_impl.mm
[modify] https://crrev.com/573bd7b3152aba99ff74ad02ae1aa846ad32a448/ios/web/navigation/navigation_manager_impl_unittest.mm

Components: UI>Browser>Core
Components: -UI>Browser>Core UI>Browser>Navigation
Labels: -Type-Feature Type-Task
Labels: Proj-Not-WKBackForwardList
Project Member

Comment 8 by sheriffbot@chromium.org, Jul 12

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)
Cc: -eugene...@chromium.org rohitrao@chromium.org
Components: -UI>Browser>Navigation Internals
Owner: eugene...@chromium.org
Status: Started (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 7

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

commit b117c3202ed77ed13e450819e608b647da6fc4b1
Author: Eugene But <eugenebut@google.com>
Date: Mon Jan 07 21:00:28 2019

Remove -[TabModel navigationCommittedInTab:previousItem:].

The body of the method was simplified and merged into the caller. Most
of the old code was related to calculation of "is in-page navigation"
boolean flag, which is actually present in LoadCommittedDetails.

New code simply uses LoadCommittedDetails.is_in_page flag.

The motivation for the change is to eventually replace didCommitNavigationWithDetails
with didFinishNavigation. didFinishNavigation does not provide
"previous navigation item" pointer, which was used in the old code.

Bug:  661983 
Change-Id: Ida8ffa4c1127a2750be642e29bb86e90d42dfad9
Reviewed-on: https://chromium-review.googlesource.com/c/1396451
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620466}
[modify] https://crrev.com/b117c3202ed77ed13e450819e608b647da6fc4b1/ios/chrome/browser/tabs/tab_model.h
[modify] https://crrev.com/b117c3202ed77ed13e450819e608b647da6fc4b1/ios/chrome/browser/tabs/tab_model.mm

Status: Fixed (was: Started)

Sign in to add a comment