Commit 9c818e00 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Use Omnibox callbacks to switch tabs

This CL uses the callbacks used when the user tab an omnibox result
to switch to an open tab. It allows to record the same metrics as
desktop.

Bug: 906598
Change-Id: I12d848915b1a8a266d887d1eec11f71d2daef1a8
Reviewed-on: https://chromium-review.googlesource.com/c/1341538
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarStepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611082}
parent f8903aaa
......@@ -4074,6 +4074,11 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint
- (void)loadURLWithParams:(const ChromeLoadParams&)chromeParams {
web::NavigationManager::WebLoadParams params = chromeParams.web_params;
if (chromeParams.disposition == WindowOpenDisposition::SWITCH_TO_TAB) {
[self switchToTabWithURL:params.url];
return;
}
[[OmniboxGeolocationController sharedInstance]
locationBarDidSubmitURL:params.url
transition:params.transition_type
......@@ -4603,7 +4608,7 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint
[nativeController focusFakebox];
}
- (void)unfocusOmniboxAndSwitchToTabWithURL:(const GURL&)URL {
- (void)switchToTabWithURL:(const GURL&)URL {
NSInteger newWebStateIndex = 0;
WebStateList* webStateList = self.tabModel.webStateList;
web::WebState* currentWebState = webStateList->GetActiveWebState();
......@@ -4661,8 +4666,6 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint
toNewView:swipeView
inPosition:position];
[self.dispatcher cancelOmniboxEdit];
webStateList->ActivateWebStateAt(newWebStateIndex);
}
......
......@@ -296,7 +296,9 @@ TEST_F(BrowserViewControllerTest, TestSwitchToTab) {
ASSERT_EQ(web_state_ptr, web_state_list->GetActiveWebState());
[bvc_.dispatcher unfocusOmniboxAndSwitchToTabWithURL:url];
ChromeLoadParams params(url);
params.disposition = WindowOpenDisposition::SWITCH_TO_TAB;
[bvc_ loadURLWithParams:params];
EXPECT_EQ(web_state_ptr_2, web_state_list->GetActiveWebState());
}
......
......@@ -104,9 +104,6 @@ class GURL;
// omnibox.
- (void)focusFakebox;
// Unfocus omnibox then switch to the first tab displaying |URL|.
- (void)unfocusOmniboxAndSwitchToTabWithURL:(const GURL&)URL;
@end
#endif // IOS_CHROME_BROWSER_UI_COMMANDS_BROWSER_COMMANDS_H_
......@@ -218,7 +218,8 @@ const int kLocationAuthorizationStatusCount = 4;
#pragma mark - LocationBarURLLoader
- (void)loadGURLFromLocationBar:(const GURL&)url
transition:(ui::PageTransition)transition {
transition:(ui::PageTransition)transition
disposition:(WindowOpenDisposition)disposition {
if (url.SchemeIs(url::kJavaScriptScheme)) {
// Evaluate the URL as JavaScript if its scheme is JavaScript.
NSString* jsToEval = [base::SysUTF8ToNSString(url.GetContent())
......@@ -236,6 +237,7 @@ const int kLocationAuthorizationStatusCount = 4;
params.transition_type = transition;
params.extra_headers = [self variationHeadersForURL:url];
ChromeLoadParams chromeParams(params);
chromeParams.disposition = disposition;
[self.URLLoader loadURLWithParams:chromeParams];
if (google_util::IsGoogleSearchUrl(url)) {
......
......@@ -126,8 +126,11 @@ TEST_F(LocationBarCoordinatorTest, LoadGoogleUrl) {
GURL url("https://www.google.com/");
ui::PageTransition transition = ui::PAGE_TRANSITION_TYPED;
WindowOpenDisposition disposition = WindowOpenDisposition::SWITCH_TO_TAB;
[coordinator_ start];
[coordinator_ loadGURLFromLocationBar:url transition:transition];
[coordinator_ loadGURLFromLocationBar:url
transition:transition
disposition:disposition];
EXPECT_EQ(url, url_loader_.url);
EXPECT_TRUE(url_loader_.referrer.url.is_empty());
......@@ -136,6 +139,7 @@ TEST_F(LocationBarCoordinatorTest, LoadGoogleUrl) {
EXPECT_FALSE(url_loader_.rendererInitiated);
ASSERT_EQ(1U, url_loader_.extraHeaders.count);
EXPECT_GT([url_loader_.extraHeaders[@"X-Client-Data"] length], 0U);
EXPECT_EQ(disposition, url_loader_.disposition);
}
// Calls -loadGURLFromLocationBar:transition: with https://www.nongoogle.com/
......@@ -148,8 +152,11 @@ TEST_F(LocationBarCoordinatorTest, LoadNonGoogleUrl) {
GURL url("https://www.nongoogle.com/");
ui::PageTransition transition = ui::PAGE_TRANSITION_TYPED;
WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB;
[coordinator_ start];
[coordinator_ loadGURLFromLocationBar:url transition:transition];
[coordinator_ loadGURLFromLocationBar:url
transition:transition
disposition:disposition];
EXPECT_EQ(url, url_loader_.url);
EXPECT_TRUE(url_loader_.referrer.url.is_empty());
......@@ -157,6 +164,7 @@ TEST_F(LocationBarCoordinatorTest, LoadNonGoogleUrl) {
EXPECT_TRUE(ui::PageTransitionCoreTypeIs(transition, url_loader_.transition));
EXPECT_FALSE(url_loader_.rendererInitiated);
ASSERT_EQ(0U, url_loader_.extraHeaders.count);
EXPECT_EQ(disposition, url_loader_.disposition);
}
} // namespace
......@@ -8,13 +8,15 @@
#import <Foundation/Foundation.h>
#include "ui/base/page_transition_types.h"
#include "ui/base/window_open_disposition.h"
class GURL;
// A means of loading URLs for the location bar.
@protocol LocationBarURLLoader
- (void)loadGURLFromLocationBar:(const GURL&)url
transition:(ui::PageTransition)transition;
transition:(ui::PageTransition)transition
disposition:(WindowOpenDisposition)disposition;
@end
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_LOCATION_BAR_URL_LOADER_H_
......@@ -10,6 +10,7 @@
#import "ios/chrome/browser/ui/omnibox/autocomplete_result_consumer.h"
#import "ios/chrome/browser/ui/omnibox/image_retriever.h"
#include "ui/base/window_open_disposition.h"
@protocol BrowserCommands;
@class OmniboxPopupPresenter;
......@@ -22,7 +23,9 @@ class IOSImageDataFetcherWrapper;
class OmniboxPopupMediatorDelegate {
public:
virtual bool IsStarredMatch(const AutocompleteMatch& match) const = 0;
virtual void OnMatchSelected(const AutocompleteMatch& match, size_t row) = 0;
virtual void OnMatchSelected(const AutocompleteMatch& match,
size_t row,
WindowOpenDisposition disposition) = 0;
virtual void OnMatchSelectedForAppending(const AutocompleteMatch& match) = 0;
virtual void OnMatchSelectedForDeletion(const AutocompleteMatch& match) = 0;
virtual void OnScroll() = 0;
......
......@@ -122,7 +122,7 @@
const AutocompleteMatch& match =
((const AutocompleteResult&)_currentResult).match_at(row);
_delegate->OnMatchSelected(match, row);
_delegate->OnMatchSelected(match, row, WindowOpenDisposition::CURRENT_TAB);
}
- (void)autocompleteResultConsumer:(id<AutocompleteResultConsumer>)sender
......@@ -131,7 +131,8 @@
((const AutocompleteResult&)_currentResult).match_at(row);
if (match.has_tab_match) {
[self.dispatcher unfocusOmniboxAndSwitchToTabWithURL:match.destination_url];
_delegate->OnMatchSelected(match, row,
WindowOpenDisposition::SWITCH_TO_TAB);
} else {
if (AutocompleteMatch::IsSearchType(match.type)) {
base::RecordAction(
......
......@@ -51,7 +51,11 @@ class OmniboxPopupViewIOS : public OmniboxPopupView,
// OmniboxPopupViewControllerDelegate implementation.
bool IsStarredMatch(const AutocompleteMatch& match) const override;
void OnMatchHighlighted(size_t row) override;
void OnMatchSelected(const AutocompleteMatch& match, size_t row) override;
// |disposition| should be CURRENT_TAB is the match should be loaded,
// SWITCH_TO_TAB if it should switch to this tab.
void OnMatchSelected(const AutocompleteMatch& match,
size_t row,
WindowOpenDisposition disposition) override;
void OnMatchSelectedForAppending(const AutocompleteMatch& match) override;
void OnMatchSelectedForDeletion(const AutocompleteMatch& match) override;
void OnScroll() override;
......
......@@ -99,8 +99,8 @@ void OmniboxPopupViewIOS::OnMatchHighlighted(size_t row) {
void OmniboxPopupViewIOS::OnMatchSelected(
const AutocompleteMatch& selectedMatch,
size_t row) {
WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB;
size_t row,
WindowOpenDisposition disposition) {
base::RecordAction(UserMetricsAction("MobileOmniboxUse"));
// OpenMatch() may close the popup, which will clear the result set and, by
......
......@@ -44,7 +44,9 @@ void WebOmniboxEditControllerImpl::OnAutocompleteAccept(
if (destination_url.is_valid()) {
transition = ui::PageTransitionFromInt(
transition | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
[URLLoader_ loadGURLFromLocationBar:destination_url transition:transition];
[URLLoader_ loadGURLFromLocationBar:destination_url
transition:transition
disposition:disposition];
}
}
......
......@@ -422,9 +422,9 @@ void TapKeyboardReturnKeyInOmniboxWithText(std::string text) {
[self.URLLoader loadURLWithParams:chromeParams];
[self cancelOmniboxEdit];
};
load_GURL_from_location_bar_swizzler_.reset(
new ScopedBlockSwizzler([LocationBarCoordinator class],
@selector(loadGURLFromLocationBar:transition:),
load_GURL_from_location_bar_swizzler_.reset(new ScopedBlockSwizzler(
[LocationBarCoordinator class],
@selector(loadGURLFromLocationBar:transition:disposition:),
loadGURLFromLocationBarBlock));
}
......
......@@ -18,6 +18,7 @@
@property(nonatomic, readonly) BOOL rendererInitiated;
@property(nonatomic, readonly) BOOL inIncognito;
@property(nonatomic, readonly, copy) NSDictionary* extraHeaders;
@property(nonatomic, readonly) WindowOpenDisposition disposition;
@end
......
......@@ -19,6 +19,7 @@
@property(nonatomic) BOOL rendererInitiated;
@property(nonatomic) BOOL inIncognito;
@property(nonatomic, copy) NSDictionary* extraHeaders;
@property(nonatomic) WindowOpenDisposition disposition;
@end
@implementation FakeURLLoader
......@@ -35,6 +36,7 @@
self.transition = params.transition_type;
self.rendererInitiated = params.is_renderer_initiated;
self.extraHeaders = params.extra_headers;
self.disposition = chromeParams.disposition;
}
- (void)webPageOrderedOpen:(OpenNewTabCommand*)command {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment