New issue
Advanced search Search tips

Issue 893121 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Task



Sign in to add a comment

Implement Switch to Tab in iOS

Project Member Reported by gambard@google.com, Oct 8

Issue description

If an omnibox suggestion is already opened in another tab, the suggestion should have a way to switch to the opened tab instead of opening the URL in the current Tab, same as what desktop is doing.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 9

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

commit ce7fd58d98dfaa4f4bfb0f26b546bb63c95e859e
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Oct 09 08:37:05 2018

[iOS] Add kOmniboxTabSwitchSuggestions to iOS

This CL adds the kOmniboxTabSwitchSuggestions flag to the flags enabled
on iOS. It also makes sure that the Material Design Controller value is
not used to check the availability of the feature as it doesn't exist on
iOS.

Bug:  893121 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I43c316d54ac48cec668c405db659a43d499328c8
Reviewed-on: https://chromium-review.googlesource.com/c/1268238
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597841}
[modify] https://crrev.com/ce7fd58d98dfaa4f4bfb0f26b546bb63c95e859e/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/ce7fd58d98dfaa4f4bfb0f26b546bb63c95e859e/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/ce7fd58d98dfaa4f4bfb0f26b546bb63c95e859e/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/ce7fd58d98dfaa4f4bfb0f26b546bb63c95e859e/ios/chrome/browser/ios_chrome_flag_descriptions.h

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 11

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

commit f15fd6643f912c81675267101a68c09a06dd7fd5
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Oct 11 13:38:58 2018

[iOS] Handle IsTabOpenWithURL for iOS

This CL makes sure that iOS is correctly handling
AutocompleteProviderClientImpl::IsTabOpenWithURL for iOS, passing the
argument in the iOS AutocompleteSuggestion.

Bug:  893121 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ia40621abf9a524c8531be8a268b71036fd1af6b7
Reviewed-on: https://chromium-review.googlesource.com/c/1268239
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598744}
[modify] https://crrev.com/f15fd6643f912c81675267101a68c09a06dd7fd5/ios/chrome/browser/autocomplete/BUILD.gn
[rename] https://crrev.com/f15fd6643f912c81675267101a68c09a06dd7fd5/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.mm
[modify] https://crrev.com/f15fd6643f912c81675267101a68c09a06dd7fd5/ios/chrome/browser/ui/omnibox/autocomplete_match_formatter.mm
[modify] https://crrev.com/f15fd6643f912c81675267101a68c09a06dd7fd5/ios/chrome/browser/ui/omnibox/autocomplete_suggestion.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 12

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

commit 2cc06f195b610cbe4aef0645fd96ddc29b2ab72a
Author: Gauthier Ambard <gambard@chromium.org>
Date: Fri Oct 12 09:59:56 2018

[iOS] Add SwitchToTab button to the omnibox popup

This CL adds the SwitchToTab image and handling to the omnibox popup
row. It allows to switch easily to an opened tab based on omnibox
suggestions.

Bug:  893121 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ib440267802c1210ea416066c679a2ff4e963fbb6
Reviewed-on: https://chromium-review.googlesource.com/c/1268344
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599155}
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/browser_view_controller_unittest.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/commands/browser_commands.h
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/autocomplete_result_consumer.h
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/BUILD.gn
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_mediator.h
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_mediator.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_row.h
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_row.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_view_controller.mm
[modify] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/omnibox_popup_view_controller_unittest.mm
[add] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/resources/BUILD.gn
[add] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/resources/omnibox_popup_tab_match.imageset/Contents.json
[add] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/resources/omnibox_popup_tab_match.imageset/omnibox_popup_tab_match.png
[add] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/resources/omnibox_popup_tab_match.imageset/omnibox_popup_tab_match@2x.png
[add] https://crrev.com/2cc06f195b610cbe4aef0645fd96ddc29b2ab72a/ios/chrome/browser/ui/omnibox/popup/resources/omnibox_popup_tab_match.imageset/omnibox_popup_tab_match@3x.png

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7

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

commit 228dfafe9bdbc5f5281b961c4d889745b5deeb71
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Nov 07 08:21:05 2018

[iOS] Move SwipeView to its own file

This is a preliminary step to start using it for the "Switch to this
tab" feature.

Bug:  893121 
Change-Id: I58f9095b689dbd0a59fffcb20bca7924dd239d7f
Reviewed-on: https://chromium-review.googlesource.com/c/1319675
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605994}
[modify] https://crrev.com/228dfafe9bdbc5f5281b961c4d889745b5deeb71/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/228dfafe9bdbc5f5281b961c4d889745b5deeb71/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm
[add] https://crrev.com/228dfafe9bdbc5f5281b961c4d889745b5deeb71/ios/chrome/browser/ui/side_swipe/swipe_view.h
[add] https://crrev.com/228dfafe9bdbc5f5281b961c4d889745b5deeb71/ios/chrome/browser/ui/side_swipe/swipe_view.mm

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified on 72.0.3608.0 Canary,  iPhoneX iOS 11.4.1, iPhoneXR iOS 12.0.1

Enabled #omnibox-tab-switch-suggetions flag
https://drive.google.com/file/d/1Bn-NR3jjAiYYN7xucpkcUBYEIu8Tfhxb/view
Hi gambard@, I looked through your change and noticed that the new IsTabOpenWithURL() method doesn't appear to look at the current profile or incognito level (like the desktop one does.) Am I reading it wrong, or is that not relevant on iOS ?

cheers

Cc: k...@chromium.org
Hi krb@, in the current iOS implementation, the autocomplete provider is only checking the tabs opened in the current mode (i.e. it will return true iif the URL is opened in an other tabs in the same mode).
How does it work on desktop?
I'm guessing it's 'webState' that hold the mode? Desktop compares both the in/cognito and the profile (since the same process can hold multiple).
browser_state is holding the mode. WebState is a representation of a tab (probably the same thing as WebContents?).
Getting the tab model from the BrowserState, gives the TabModel from the current mode, which gives all the tabs for the current mode.

I think it is the same code as on desktop as BrowserList::GetInstance()->GetLastActive(); is the same thing as TabModelList::GetLastActiveTabModelForChromeBrowserState();

We don't have different profiles I think on iOS.
Cc: eugene...@chromium.org
+eugenebut@ as you know both contents/ and ios/web/.
content's BrowserContext == ios' BrowserState
content's WebContents == ios's WebState

AFAIK BrowserState or BrowserContext represent the profile, and on iOS we only support Incognito and Non-Incognito.
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 21

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

commit a541f7419b108868ab8231e4b0077e6736ceab5b
Author: Mark Cogan <marq@google.com>
Date: Fri Dec 21 16:05:01 2018

[iOS] Factor some model logic out of switchToTabWithParams:

This CL factors some model-layer logic out of BVC.

- The logic for finding a webstate with a given URL is moved, as suggested, into WebStateList, along with unit tests.

- The logic for identifying the "NTP with no history" state is moved into ntp_util.

Some other cleanup is done in switchToTabWithParams: to reduce the use of local variables and make the code more separable.

Bug:  893121 
Change-Id: I6f8def5380df31f8babeb0355102c6efb38c9a69
Reviewed-on: https://chromium-review.googlesource.com/c/1386830
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618542}
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.mm
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/ui/ntp/ntp_util.h
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/ui/ntp/ntp_util.mm
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/web_state_list/web_state_list.h
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/web_state_list/web_state_list.mm
[modify] https://crrev.com/a541f7419b108868ab8231e4b0077e6736ceab5b/ios/chrome/browser/web_state_list/web_state_list_unittest.mm

Sign in to add a comment