New issue
Advanced search Search tips

Issue 785253 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Create a LocationBarCoordinator

Project Member Reported by gambard@chromium.org, Nov 15 2017

Issue description

For now the LocationBarView and LocationBar are created and managed by the Toolbar. It should have its own coordinator.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 28 2017

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

commit 61453617abd0387394cd1aa85360e1b15f47bbe3
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Thu Dec 28 18:20:30 2017

Create files for location bar.

Moves location bar to its own folder and creates a set of files for the
location bar.

Bug: 788639,  785253 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ibb11776adc5eb2abcc0ad692351e9242e1e6c8da
Reviewed-on: https://chromium-review.googlesource.com/822260
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526308}
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/BUILD.gn
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/OWNERS
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/README.md
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_coordinator.h
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_coordinator_unittest.mm
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_mediator.h
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_mediator.mm
[rename] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_view.h
[rename] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_view.mm
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_view_controller.h
[add] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/location_bar/location_bar_view_controller.mm
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/omnibox/BUILD.gn
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/omnibox/location_bar_controller_impl.mm
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/omnibox/omnibox_view_ios.mm
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/toolbar/clean/BUILD.gn
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm
[modify] https://crrev.com/61453617abd0387394cd1aa85360e1b15f47bbe3/ios/chrome/test/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 17 2018

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

commit 16f4b01dc203028c7c04efe83ba81f17abd5f9eb
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Wed Jan 17 14:35:04 2018

Move a method from ToolbarCoordinator to LocationBarCoordinator.

This is the first of many CLs to move Location Bar logic from
the ToolbarCoordinator. It moves the URL loading for LBControllerImpl
to LBCoordinator by splitting LBDelegate protocol into two.

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id69db0b1669619de5e7977c4eb3b3b5f9a4b7a6c
Bug:  785253 
Reviewed-on: https://chromium-review.googlesource.com/866932
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529737}
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/location_bar/location_bar_coordinator.h
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[add] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/location_bar/location_bar_url_loader.h
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/omnibox/location_bar_controller_impl.h
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/omnibox/location_bar_controller_impl.mm
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/omnibox/location_bar_delegate.h
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/qr_scanner/BUILD.gn
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/toolbar/BUILD.gn
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/toolbar/clean/BUILD.gn
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/16f4b01dc203028c7c04efe83ba81f17abd5f9eb/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 24 2018

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

commit 7654576158eac23f4a60e7bbdf40861138d00f3e
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Wed Jan 24 18:30:53 2018

Move OmniboxFocuser protocol implementation to the location bar.

Since the location bar contains the omnibox (not the toolbar), it makes
sense for LB to implement OmniboxFocuser protocol.

Bug:  785253 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4b93b4ba59175e8c42b4b441b58a9f691a9b7543
Reviewed-on: https://chromium-review.googlesource.com/868020
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531614}
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/location_bar/location_bar_coordinator.h
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/qr_scanner/qr_scanner_view_controller_egtest.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.h
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/legacy_toolbar_coordinator.mm
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/public/primary_toolbar_coordinator.h
[modify] https://crrev.com/7654576158eac23f4a60e7bbdf40861138d00f3e/ios/chrome/browser/ui/toolbar/toolbar_adapter.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2018

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

commit 502ae8470e6f833031d07b05ac5af6ea2a7041bc
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Thu Jan 25 13:59:40 2018

Change relationship between BVC, Toolbar and Location Bar delegation.

Before this CL, BVC is Toolbar's delegate, which is a
LocationBarControllerImpl's delegate. We would like for the
LocationBarCoordinator to own LBControllerImpl fully. For this, the
LocationBarCoordinator needs to become its delegate. However, the
delegate is mostly forwarding commands to BVC. So after this CL:

1. BVC is the delegate for LBCoordinator.
2. BVC calls Toolbar methods for Toolbar to react to events in LB.
3. LocationBarCoordinator is delegate for LBControllerImpl.
4. Remove old TODO and related ivar in BVC.

Bug:  785253 , 244366
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7a735ed60a54c040437a91ff225f0c6c31e0e799
Reviewed-on: https://chromium-review.googlesource.com/881165
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531886}
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/location_bar/location_bar_coordinator.h
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/omnibox/location_bar_controller_impl.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/omnibox/location_bar_delegate.h
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.h
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/legacy_toolbar_coordinator.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/public/primary_toolbar_coordinator.h
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/public/toolbar.h
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/toolbar_adapter.mm
[modify] https://crrev.com/502ae8470e6f833031d07b05ac5af6ea2a7041bc/ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 29 2018

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

commit 0b7d447cc7b0f5ee0a61928f89291ba837f581d6
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Mon Jan 29 18:08:41 2018

Move LocationBarControllerImpl and PopupCoordinator to LocationBar

Moves the _locationBar ivar (it becomes _locationBarController) to
LocationBarCoordinator. Moves popup coordinator, too.

Bug:  785253 ,  806028 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4bea5a069fc2caeb76f27d204eeddd8d5b281430
Reviewed-on: https://chromium-review.googlesource.com/883504
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532489}
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/location_bar/BUILD.gn
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/location_bar/location_bar_coordinator.h
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/location_bar/location_bar_coordinator_unittest.mm
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/toolbar/clean/BUILD.gn
[modify] https://crrev.com/0b7d447cc7b0f5ee0a61928f89291ba837f581d6/ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm

Status: Fixed (was: Assigned)

Sign in to add a comment