New issue
Advanced search Search tips

Issue 910894 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Remove WebStateObserver::NavigationItemsPruned after shipping slim-navigation-manager

Project Member Reported by eugene...@chromium.org, Dec 1

Issue description

App Version (from "Chrome Settings > About Chrome"): 
iOS Version: 
Device: 

Steps to reproduce: 

Observed behavior: 

Expected behavior: 

Frequency: 
<number of times you were able to reproduce> 

Additional comments: 

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 3

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

commit 4215c4dd4bd6a110a155154ef0b517d53e71bccc
Author: Eugene But <eugenebut@chromium.org>
Date: Mon Dec 03 18:43:25 2018

Add WebStateObserverTest.NewPageLoadDestroysForwardItems.

Tests that new page load calls NavigationItemsPruned callback if there
were forward navigation items.

Bug: 910894
Change-Id: I98bfd1a9960b99d0d7d8511b2b4068f56344b8d1
Reviewed-on: https://chromium-review.googlesource.com/c/1357841
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613171}
[modify] https://crrev.com/4215c4dd4bd6a110a155154ef0b517d53e71bccc/ios/web/web_state/web_state_observer_inttest.mm

Owner: eugene...@chromium.org
Status: Assigned (was: Untriaged)
Looks like you are already working on this.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 4

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

commit 252a6578e5df0602179012de0ae244d217242510
Author: Eugene But <eugenebut@chromium.org>
Date: Tue Dec 04 13:39:17 2018

Implement -[PopupMenuMediator webStateDidChangeBackForwardState:].

This callback replaces webState:didPruneNavigationItemsWithCount: when
slim-navigation-manager is enabled.

webState:didPruneNavigationItemsWithCount: was implemented by
PopupMenuMediator, which means that webStateDidChangeBackForwardState:
should be implemented as well.

Bug: 910894
Change-Id: I8af264737e3505cae8c1c4bdb78827f88c81a154
Reviewed-on: https://chromium-review.googlesource.com/c/1359864
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613530}
[modify] https://crrev.com/252a6578e5df0602179012de0ae244d217242510/ios/chrome/browser/ui/popup_menu/popup_menu_mediator.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 5

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

commit d04af01a6bf5dbb69737f6683d39f93c1ae8be55
Author: Eugene But <eugenebut@chromium.org>
Date: Wed Dec 05 02:42:34 2018

Implement -[CWVWebView webStateDidChangeBackForwardState:].

Whan slim-navigation-manager is enabled, this callback replaces
webState:navigationItemsPruned:

Bug: 910894
Change-Id: Ib337e9072127fcbc505897db983efbc439871828
Reviewed-on: https://chromium-review.googlesource.com/c/1360982
Reviewed-by: Hiroshi Ichikawa <ichikawa@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613830}
[modify] https://crrev.com/d04af01a6bf5dbb69737f6683d39f93c1ae8be55/ios/web_view/internal/cwv_web_view.mm

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7

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

commit c82006414aa7d03f2b8034b4e0de2db3094a7233
Author: Eugene But <eugenebut@google.com>
Date: Fri Dec 07 19:25:53 2018

IOSChromeLocalSessionEventRouter does not subclass from GlobalWebStateObserver.

IOSChromeLocalSessionEventRouter will observe multiple WebState objects
without using deprecated GlobalWebStateObserver.

Also removed browser state check in OnWebStateChange. This check was needed
because each WSO callback was called for both BrowserState objects. So the
router had to check if the call belongs to correct browser state. After this
CL there are no extra calls, hence no need for check.

Bug: 910894, 782269,  720786 
Change-Id: Ie25ae62efea9edd4a981800d1a43b6ef28b35b14
Reviewed-on: https://chromium-review.googlesource.com/c/1359863
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614779}
[modify] https://crrev.com/c82006414aa7d03f2b8034b4e0de2db3094a7233/ios/chrome/browser/sync/BUILD.gn
[rename] https://crrev.com/c82006414aa7d03f2b8034b4e0de2db3094a7233/ios/chrome/browser/sync/session_sync_service_factory.mm
[modify] https://crrev.com/c82006414aa7d03f2b8034b4e0de2db3094a7233/ios/chrome/browser/sync/sessions/BUILD.gn
[modify] https://crrev.com/c82006414aa7d03f2b8034b4e0de2db3094a7233/ios/chrome/browser/sync/sessions/ios_chrome_local_session_event_router.h
[modify] https://crrev.com/c82006414aa7d03f2b8034b4e0de2db3094a7233/ios/chrome/browser/sync/sessions/ios_chrome_local_session_event_router.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 8

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

commit dca91f47f7dacc724352ef9b2055b6d7740eefb0
Author: Eugene But <eugenebut@google.com>
Date: Sat Dec 08 00:34:35 2018

Marked NavigationItemsPruned as deprecated.

DidChangeBackForwardState is a superset of this callback and should be
used instead of NavigationItemsPruned in the future.
NavigationItemsPruned is not called when slim-navigation-manager is
enabled and DidChangeBackForwardState is not called when
slim-navigation-manager is disabled. So for now the clients should
implement both callbacks.

Bug: 910894
Change-Id: I39a3972a909e956b92e0c68a66ce346c49833be7
Reviewed-on: https://chromium-review.googlesource.com/c/1368948
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614886}
[modify] https://crrev.com/dca91f47f7dacc724352ef9b2055b6d7740eefb0/ios/web/public/web_state/web_state_observer.h

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 10

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

commit ce030490dbec9d6015ee1d9c952ec2c44901ff97
Author: Eugene But <eugenebut@google.com>
Date: Mon Dec 10 18:51:54 2018

Implement IOSChromeLocalSessionEventRouter::DidChangeBackForwardState.

NavigationItemsPruned is not called when slim-navigation-manager is
enabled. DidChangeBackForwardState is a superset of
NavigationItemsPruned and is called for slim-navigation-manager.

Overridden DidChangeBackForwardState has the same implementation as
NavigationItemsPruned.

Bug: 910894
Change-Id: Ib0aac6df22440745345eebb513d06ff52b3a309c
Reviewed-on: https://chromium-review.googlesource.com/c/1368839
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615188}
[modify] https://crrev.com/ce030490dbec9d6015ee1d9c952ec2c44901ff97/ios/chrome/browser/sync/sessions/ios_chrome_local_session_event_router.h
[modify] https://crrev.com/ce030490dbec9d6015ee1d9c952ec2c44901ff97/ios/chrome/browser/sync/sessions/ios_chrome_local_session_event_router.mm

Summary: Remove WebStateObserver::NavigationItemsPruned after shipping slim-navigation-manager (was: WebStateObserver::NavigationItemsPruned is not called with slim-navigation-manager enabled)
Labels: -Type-Bug -Pri-2 Pri-3 Type-Task
Owner: ----
Status: Available (was: Started)

Sign in to add a comment