[iOS][OR] Open custom AutocompleteMatch in OmniboxAutocompleteController
Move select match for opening of custom AutocompleteMatch to OmniboxAutocompleteController. Following CLs will move the remaining logic of OnSelectedMatchForOpening. Bug: 390409252 Change-Id: I7b7bda35e7afa4bbc303273d83b91b52783b4440 Skip-Clang-Tidy-Checks: google-default-arguments the method is marked virtual for mocking Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6257471 Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org> Commit-Queue: Christian Xu <christianxu@chromium.org> Auto-Submit: Christian Xu <christianxu@chromium.org> Reviewed-by: Ameur Hosni <ameurhosni@google.com> Cr-Commit-Position: refs/heads/main@{#1419913}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
f396bb5a3c
commit
ce81166f4d
components/omnibox/browser
ios/chrome/browser/omnibox
@ -197,14 +197,14 @@ class OmniboxEditModel {
|
||||
// Opens given selection. Most kinds of selection invoke an action or
|
||||
// otherwise call `OpenMatch`, but some may `AcceptInput` which is not
|
||||
// guaranteed to open a match or commit the omnibox.
|
||||
void OpenSelection(
|
||||
virtual void OpenSelection(
|
||||
OmniboxPopupSelection selection,
|
||||
base::TimeTicks timestamp = base::TimeTicks(),
|
||||
WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB);
|
||||
|
||||
// A simplified version of OpenSelection that opens the model's current
|
||||
// selection.
|
||||
void OpenSelection(
|
||||
virtual void OpenSelection(
|
||||
base::TimeTicks timestamp = base::TimeTicks(),
|
||||
WindowOpenDisposition disposition = WindowOpenDisposition::CURRENT_TAB);
|
||||
|
||||
|
@ -14,6 +14,8 @@
|
||||
#import "components/omnibox/browser/autocomplete_controller.h"
|
||||
#import "components/omnibox/browser/autocomplete_match.h"
|
||||
#import "components/omnibox/browser/omnibox_controller.h"
|
||||
#import "components/omnibox/browser/omnibox_edit_model.h"
|
||||
#import "components/omnibox/browser/omnibox_popup_selection.h"
|
||||
#import "components/open_from_clipboard/clipboard_recent_content.h"
|
||||
#import "ios/chrome/browser/omnibox/model/omnibox_popup_controller.h"
|
||||
#import "ios/chrome/browser/omnibox/ui_bundled/omnibox_view_ios.h"
|
||||
@ -35,6 +37,8 @@ using base::UserMetricsAction;
|
||||
raw_ptr<AutocompleteController> _autocompleteController;
|
||||
/// Controller of the omnibox view.
|
||||
raw_ptr<OmniboxViewIOS> _omniboxViewIOS;
|
||||
/// Omnibox edit model. Should only be used for autocomplete interactions.
|
||||
raw_ptr<OmniboxEditModel> _omniboxEditModel;
|
||||
|
||||
/// Pref tracking if the bottom omnibox is enabled.
|
||||
PrefBackedBoolean* _bottomOmniboxEnabled;
|
||||
@ -49,6 +53,7 @@ using base::UserMetricsAction;
|
||||
_omniboxController = omniboxController;
|
||||
_autocompleteController = omniboxController->autocomplete_controller();
|
||||
_omniboxViewIOS = omniboxViewIOS;
|
||||
_omniboxEditModel = omniboxController->edit_model();
|
||||
|
||||
_preferredOmniboxPosition = metrics::OmniboxEventProto::UNKNOWN_POSITION;
|
||||
_bottomOmniboxEnabled = [[PrefBackedBoolean alloc]
|
||||
@ -66,6 +71,7 @@ using base::UserMetricsAction;
|
||||
[_bottomOmniboxEnabled setObserver:nil];
|
||||
_bottomOmniboxEnabled = nil;
|
||||
_autocompleteController = nullptr;
|
||||
_omniboxEditModel = nullptr;
|
||||
_omniboxController = nullptr;
|
||||
_omniboxViewIOS = nullptr;
|
||||
}
|
||||
@ -138,6 +144,7 @@ using base::UserMetricsAction;
|
||||
- (void)selectMatchForOpening:(const AutocompleteMatch&)match
|
||||
inRow:(NSUInteger)row
|
||||
openIn:(WindowOpenDisposition)disposition {
|
||||
const auto matchSelectionTimestamp = base::TimeTicks();
|
||||
base::RecordAction(UserMetricsAction("MobileOmniboxUse"));
|
||||
|
||||
// OpenMatch() may close the popup, which will clear the result set and, by
|
||||
@ -151,6 +158,23 @@ using base::UserMetricsAction;
|
||||
"MobileOmnibox.PressedClipboardSuggestionAge",
|
||||
ClipboardRecentContent::GetInstance()->GetClipboardContentAge());
|
||||
}
|
||||
|
||||
if (!_autocompleteController || !_omniboxEditModel) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Sometimes the match provided does not correspond to the autocomplete
|
||||
// result match specified by `index`. Most Visited Tiles, for example,
|
||||
// provide ad hoc matches that are not in the result at all.
|
||||
if (row >= _autocompleteController->result().size() ||
|
||||
_autocompleteController->result().match_at(row).destination_url !=
|
||||
match.destination_url) {
|
||||
[self openCustomMatch:match
|
||||
disposition:disposition
|
||||
selectionTimestamp:matchSelectionTimestamp];
|
||||
return;
|
||||
}
|
||||
|
||||
if (_omniboxViewIOS) {
|
||||
_omniboxViewIOS->OnSelectedMatchForOpening(matchCopy, disposition, GURL(),
|
||||
std::u16string(), row);
|
||||
@ -190,4 +214,18 @@ using base::UserMetricsAction;
|
||||
}
|
||||
}
|
||||
|
||||
#pragma mark - Private
|
||||
|
||||
/// Opens a match created outside of autocomplete controller.
|
||||
- (void)openCustomMatch:(AutocompleteMatch)match
|
||||
disposition:(WindowOpenDisposition)disposition
|
||||
selectionTimestamp:(base::TimeTicks)timestamp {
|
||||
if (!_autocompleteController || !_omniboxEditModel) {
|
||||
return;
|
||||
}
|
||||
OmniboxPopupSelection selection(
|
||||
_autocompleteController->InjectAdHocMatch(match));
|
||||
_omniboxEditModel->OpenSelection(selection, timestamp, disposition);
|
||||
}
|
||||
|
||||
@end
|
||||
|
@ -12,6 +12,7 @@
|
||||
#import "components/omnibox/browser/fake_autocomplete_provider_client.h"
|
||||
#import "components/omnibox/browser/omnibox_client.h"
|
||||
#import "components/omnibox/browser/omnibox_controller.h"
|
||||
#import "components/omnibox/browser/omnibox_edit_model.h"
|
||||
#import "components/omnibox/browser/test_omnibox_client.h"
|
||||
#import "components/prefs/testing_pref_service.h"
|
||||
#import "ios/chrome/browser/omnibox/model/omnibox_popup_controller.h"
|
||||
@ -61,6 +62,25 @@ class MockAutocompleteController : public AutocompleteController {
|
||||
metrics::OmniboxEventProto::OmniboxPosition omnibox_position;
|
||||
};
|
||||
|
||||
/// A mock class for OmniboxEditModel.
|
||||
class MockOmniboxEditModel : public OmniboxEditModel {
|
||||
public:
|
||||
MockOmniboxEditModel(OmniboxController* controller)
|
||||
: OmniboxEditModel(controller, nullptr),
|
||||
last_opened_selection(OmniboxPopupSelection(UINT_MAX)) {}
|
||||
MockOmniboxEditModel(const MockOmniboxEditModel&) = delete;
|
||||
MockOmniboxEditModel& operator=(const MockOmniboxEditModel&) = delete;
|
||||
~MockOmniboxEditModel() override = default;
|
||||
|
||||
void OpenSelection(OmniboxPopupSelection selection,
|
||||
base::TimeTicks timestamp,
|
||||
WindowOpenDisposition disposition) override {
|
||||
last_opened_selection = selection;
|
||||
}
|
||||
|
||||
OmniboxPopupSelection last_opened_selection;
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
||||
class OmniboxAutocompleteControllerTest : public PlatformTest {
|
||||
@ -79,6 +99,11 @@ class OmniboxAutocompleteControllerTest : public PlatformTest {
|
||||
omnibox_controller_->SetAutocompleteControllerForTesting(
|
||||
std::move(autocomplete));
|
||||
|
||||
auto edit_model =
|
||||
std::make_unique<MockOmniboxEditModel>(omnibox_controller_.get());
|
||||
omnibox_edit_model_ = edit_model.get();
|
||||
omnibox_controller_->SetEditModelForTesting(std::move(edit_model));
|
||||
|
||||
controller_ = [[OmniboxAutocompleteController alloc]
|
||||
initWithOmniboxController:omnibox_controller_.get()
|
||||
omniboxViewIOS:nullptr];
|
||||
@ -90,6 +115,7 @@ class OmniboxAutocompleteControllerTest : public PlatformTest {
|
||||
~OmniboxAutocompleteControllerTest() override {
|
||||
[controller_ disconnect];
|
||||
autocomplete_controller_ = nullptr;
|
||||
omnibox_edit_model_ = nullptr;
|
||||
omnibox_controller_ = nullptr;
|
||||
popup_ = nil;
|
||||
TestingApplicationContext::GetGlobal()->SetLocalState(nullptr);
|
||||
@ -103,6 +129,12 @@ class OmniboxAutocompleteControllerTest : public PlatformTest {
|
||||
/*destination_url=*/"http://this-site-matches.com")};
|
||||
}
|
||||
|
||||
/// Returns the match opened by OmniboxEditModel::OpenSelection.
|
||||
const AutocompleteMatch& LastOpenedMatch() {
|
||||
return autocomplete_controller_->result().match_at(
|
||||
omnibox_edit_model_->last_opened_selection.line);
|
||||
}
|
||||
|
||||
protected:
|
||||
// Message loop for the main test thread.
|
||||
base::test::TaskEnvironment environment_;
|
||||
@ -111,10 +143,21 @@ class OmniboxAutocompleteControllerTest : public PlatformTest {
|
||||
|
||||
OmniboxAutocompleteController* controller_;
|
||||
raw_ptr<MockAutocompleteController> autocomplete_controller_;
|
||||
raw_ptr<MockOmniboxEditModel> omnibox_edit_model_;
|
||||
std::unique_ptr<OmniboxController> omnibox_controller_;
|
||||
id popup_;
|
||||
};
|
||||
|
||||
// Custom matcher for AutocompleteMatch
|
||||
MATCHER_P(IsSameAsMatch, expected, "") {
|
||||
return arg.destination_url == expected.destination_url &&
|
||||
arg.fill_into_edit == expected.fill_into_edit &&
|
||||
arg.additional_text == expected.additional_text &&
|
||||
arg.inline_autocompletion == expected.inline_autocompletion &&
|
||||
arg.contents == expected.contents &&
|
||||
arg.description == expected.description;
|
||||
}
|
||||
|
||||
// Tests that adding fake matches adds them to the results.
|
||||
TEST_F(OmniboxAutocompleteControllerTest, AddFakeMatches) {
|
||||
ACMatches sample_matches = SampleMatches();
|
||||
@ -122,6 +165,8 @@ TEST_F(OmniboxAutocompleteControllerTest, AddFakeMatches) {
|
||||
EXPECT_EQ(autocomplete_controller_->result().size(), sample_matches.size());
|
||||
}
|
||||
|
||||
#pragma mark - Request suggestion
|
||||
|
||||
// Tests requesting result when there are none still calls
|
||||
// updateWithSortedResults.
|
||||
TEST_F(OmniboxAutocompleteControllerTest, RequestResultEmpty) {
|
||||
@ -192,6 +237,8 @@ TEST_F(OmniboxAutocompleteControllerTest, RequestResultPartVisible) {
|
||||
EXPECT_OCMOCK_VERIFY(popup_);
|
||||
}
|
||||
|
||||
#pragma mark - Logging
|
||||
|
||||
// Tests that omnibox position update is forwarded to autocompleteController.
|
||||
TEST_F(OmniboxAutocompleteControllerTest, OmniboxPositionUpdates) {
|
||||
local_state_->SetBoolean(prefs::kBottomOmnibox, true);
|
||||
@ -202,3 +249,31 @@ TEST_F(OmniboxAutocompleteControllerTest, OmniboxPositionUpdates) {
|
||||
EXPECT_EQ(autocomplete_controller_->omnibox_position,
|
||||
metrics::OmniboxEventProto::TOP_POSITION);
|
||||
}
|
||||
|
||||
#pragma mark - Open match
|
||||
|
||||
// Tests opening a match that doesn't exist in autocomplete controller.
|
||||
TEST_F(OmniboxAutocompleteControllerTest, OpenCreatedMatch) {
|
||||
autocomplete_controller_->SetAutocompleteMatches(SampleMatches());
|
||||
AutocompleteMatch match = CreateSearchMatch(u"some match");
|
||||
|
||||
// Open match that doesn't come from the autocomplete controller. Row is
|
||||
// higher than autocomplete_controller_->result().size().
|
||||
[controller_ selectMatchForOpening:match
|
||||
inRow:10
|
||||
openIn:WindowOpenDisposition::CURRENT_TAB];
|
||||
|
||||
// Expect the match to be opened.
|
||||
EXPECT_THAT(LastOpenedMatch(), IsSameAsMatch(match));
|
||||
|
||||
// Reset the last opened selection.
|
||||
omnibox_edit_model_->last_opened_selection = OmniboxPopupSelection(UINT_MAX);
|
||||
|
||||
// Open match that doesn't come from the autocomplete controller. Row is
|
||||
// smaller than autocomplete_controller_->result().size().
|
||||
[controller_ selectMatchForOpening:match
|
||||
inRow:1
|
||||
openIn:WindowOpenDisposition::CURRENT_TAB];
|
||||
// Expect the match to be opened.
|
||||
EXPECT_THAT(LastOpenedMatch(), IsSameAsMatch(match));
|
||||
}
|
||||
|
@ -728,19 +728,6 @@ void OmniboxViewIOS::OnSelectedMatchForOpening(
|
||||
size_t index) {
|
||||
const auto match_selection_timestamp = base::TimeTicks();
|
||||
|
||||
// Sometimes the match provided does not correspond to the autocomplete
|
||||
// result match specified by `index`. Most Visited Tiles, for example,
|
||||
// provide ad hoc matches that are not in the result at all.
|
||||
auto* autocomplete_controller = controller()->autocomplete_controller();
|
||||
if (index >= autocomplete_controller->result().size() ||
|
||||
autocomplete_controller->result().match_at(index).destination_url !=
|
||||
match.destination_url) {
|
||||
OmniboxPopupSelection selection(
|
||||
autocomplete_controller->InjectAdHocMatch(match));
|
||||
model()->OpenSelection(selection, match_selection_timestamp, disposition);
|
||||
return;
|
||||
}
|
||||
|
||||
// Fill in clipboard matches if they don't have a destination URL.
|
||||
if (match.destination_url.is_empty()) {
|
||||
if (match.type == AutocompleteMatchType::CLIPBOARD_URL) {
|
||||
|
Reference in New Issue
Block a user