New issue
Advanced search Search tips

Issue 761537 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Nov 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 760849



Sign in to add a comment

[iOS Clean] Allow for adding tab helpers to Browser.

Project Member Reported by kkhorimoto@chromium.org, Sep 1 2017

Issue description

Currently, no "tab helpers" created in the BrowserWebStateListDelegate can use BrowserCoordinators, since it will create a circular dependency due to its inclusion in the browser-list target.  We need a way to set the tab helpers from an external target to enable tab helpers that need to generate UI, like the WebStateDelegate.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 6 2017

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

commit 165999ecc2903e8c51323334f186877316e1f483
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Wed Sep 06 21:53:16 2017

Break cycle between BrowserList and WebStateListDelegate.

BrowserWebStateListDelegate is responsible for the creation of the
tab helpers attached to all the WebStates for a given Browser. The
tab helpers may have indirect dependencies on BrowserList.

To prevent such a cycle to appear, introduce a factory to inject
the WebStateListDelegate passed to the BrowserList, and split the
target that define the BrowserList implementation and the factory.

Convert BrowserList to a KeyedService to use the infrastructure
to ensure the WebStateListDelegate injection happens at the correct
time, and fix all uses of BrowserList::FromBrowserState() to use
the factory instead.

Bug:  761537 
Change-Id: I5a16571abb695bf5b76fc7e58a689ad7b93abd36
Reviewed-on: https://chromium-review.googlesource.com/649612
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eric Noyau <noyau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500104}
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/docs/ios/objects.md
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/browser_state/BUILD.gn
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/browser_state/browser_state_keyed_service_factories.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/BUILD.gn
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser.h
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list.h
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list.mm
[add] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_factory.h
[add] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_factory.mm
[add] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_impl.h
[add] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_impl.mm
[add] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_impl_unittest.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_session_service_factory.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_list_session_service_impl.mm
[delete] https://crrev.com/b73321e501bdaafc4660612dc87dd3df9fe2d301/ios/chrome/browser/ui/browser_list/browser_list_unittest.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_web_state_list_delegate.h
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/browser_list/browser_web_state_list_delegate.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/coordinators/BUILD.gn
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/coordinators/browser_coordinator_test.h
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/ui/coordinators/browser_coordinator_test.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/chrome/browser/web_state_list/web_state_list_delegate.h
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/clean/chrome/app/steps/root_coordinator_initializer.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/clean/chrome/browser/ui/overlays/overlay_service_factory.mm
[modify] https://crrev.com/165999ecc2903e8c51323334f186877316e1f483/ios/clean/chrome/browser/ui/overlays/overlay_service_impl.mm

Components: -Mobile>WebView>Glue Internals
Status: WontFix (was: Started)

Sign in to add a comment