Commit 8d06cb3d authored by Stepan Khapugin's avatar Stepan Khapugin Committed by Commit Bot

[iOS] Use default icon when there's no default match.

Adds a code path for no default match case for the left image to be set
to the default search image. Also adds a EG test for tapping on the
omnibox with something in clipboard that verifies that the clipboard
match (which isn't a default match) on NTP appears but doesn't populate
text in the omnibox (not being default), and therefore tests that the
app doesn't crash.

Bug: 1024885, 1027561
Change-Id: I7242a537d919a6d3a6a4048ddf59a3249cb862f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939970
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Auto-Submit: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: default avatarRobbie Gibson <rkgibson@google.com>
Cr-Commit-Position: refs/heads/master@{#719556}
parent 1c5e531a
...@@ -187,6 +187,7 @@ source_set("omnibox_internal") { ...@@ -187,6 +187,7 @@ source_set("omnibox_internal") {
} }
source_set("eg_tests") { source_set("eg_tests") {
defines = [ "CHROME_EARL_GREY_1" ]
testonly = true testonly = true
sources = [ sources = [
"omnibox_app_interface.h", "omnibox_app_interface.h",
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#import "ios/chrome/test/earl_grey/chrome_matchers.h" #import "ios/chrome/test/earl_grey/chrome_matchers.h"
#import "ios/chrome/test/earl_grey/chrome_matchers_app_interface.h" #import "ios/chrome/test/earl_grey/chrome_matchers_app_interface.h"
#import "ios/chrome/test/earl_grey/chrome_test_case.h" #import "ios/chrome/test/earl_grey/chrome_test_case.h"
#import "ios/testing/earl_grey/earl_grey_test.h"
#import "ios/testing/hardware_keyboard_util.h" #import "ios/testing/hardware_keyboard_util.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
...@@ -554,6 +555,44 @@ id<GREYMatcher> SearchCopiedTextButton() { ...@@ -554,6 +555,44 @@ id<GREYMatcher> SearchCopiedTextButton() {
assertWithMatcher:grey_nil()]; assertWithMatcher:grey_nil()];
} }
- (void)testNoDefaultMatch {
NSString* copiedText = @"test no default match1";
// Put some text in pasteboard.
UIPasteboard.generalPasteboard.string = copiedText;
// Copying can take a while, wait for it to happen.
GREYCondition* copyCondition =
[GREYCondition conditionWithName:@"test text copied condition"
block:^BOOL {
return [UIPasteboard.generalPasteboard.string
isEqualToString:copiedText];
}];
// Wait for copy to happen or timeout after 5 seconds.
GREYAssertTrue([copyCondition waitWithTimeout:5],
@"Copying test text failed");
// Focus the omnibox.
[self focusFakebox];
// Make sure that:
// 1. Chrome didn't crash (See crbug.com/1024885 for historic context)
// 2. There's nothing in the omnibox
// 3. There's a "text you copied" match
[[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
assertWithMatcher:chrome_test_util::OmniboxText("")];
// Returns the popup row containing the |url| as suggestion.
id<GREYMatcher> textYouCopiedMatch =
grey_allOf(grey_kindOfClassName(@"OmniboxPopupRow"),
grey_descendant(grey_accessibilityLabel(
[NSString stringWithFormat:@"\"%@\"", copiedText])),
nil);
[[EarlGrey selectElementWithMatcher:textYouCopiedMatch]
assertWithMatcher:grey_notNil()];
}
#pragma mark - Helpers #pragma mark - Helpers
// Taps the fake omnibox and waits for the real omnibox to be visible. // Taps the fake omnibox and waits for the real omnibox to be visible.
......
...@@ -25,6 +25,9 @@ ...@@ -25,6 +25,9 @@
answerType answerType
faviconURL:(GURL)faviconURL; faviconURL:(GURL)faviconURL;
// Reset the left image to be the default search one.
- (void)setDefaultLeftImage;
@end @end
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_LEFT_IMAGE_CONSUMER_H_ #endif // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_LEFT_IMAGE_CONSUMER_H_
...@@ -120,6 +120,20 @@ const CGFloat kOmniboxIconSize = 16; ...@@ -120,6 +120,20 @@ const CGFloat kOmniboxIconSize = 16;
} }
} }
- (void)setDefaultLeftImage {
UIImage* image = GetOmniboxSuggestionIconForAutocompleteMatchType(
AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, /* is_starred */ false);
[self.consumer updateAutocompleteIcon:image];
__weak OmniboxMediator* weakSelf = self;
if (base::FeatureList::IsEnabled(kOmniboxUseDefaultSearchEngineFavicon)) {
// Show Default Search Engine favicon.
[self loadDefaultSearchEngineFaviconWithCompletion:^(UIImage* image) {
[weakSelf.consumer updateAutocompleteIcon:image];
}];
}
}
// Loads a favicon for a given page URL. // Loads a favicon for a given page URL.
// |pageURL| is url for the page that needs a favicon // |pageURL| is url for the page that needs a favicon
// |completion| handler might be called multiple // |completion| handler might be called multiple
......
...@@ -107,7 +107,8 @@ class OmniboxViewIOS : public OmniboxView, ...@@ -107,7 +107,8 @@ class OmniboxViewIOS : public OmniboxView,
// OmniboxPopupViewSuggestionsDelegate methods // OmniboxPopupViewSuggestionsDelegate methods
void OnTopmostSuggestionImageChanged( void OnSelectedMatchImageChanged(
bool has_match,
AutocompleteMatchType::Type match_type, AutocompleteMatchType::Type match_type,
base::Optional<SuggestionAnswer::AnswerType> answer_type, base::Optional<SuggestionAnswer::AnswerType> answer_type,
GURL favicon_url) override; GURL favicon_url) override;
......
...@@ -702,13 +702,18 @@ void OmniboxViewIOS::EmphasizeURLComponents() { ...@@ -702,13 +702,18 @@ void OmniboxViewIOS::EmphasizeURLComponents() {
#pragma mark - OmniboxPopupViewSuggestionsDelegate #pragma mark - OmniboxPopupViewSuggestionsDelegate
void OmniboxViewIOS::OnTopmostSuggestionImageChanged( void OmniboxViewIOS::OnSelectedMatchImageChanged(
bool has_match,
AutocompleteMatchType::Type match_type, AutocompleteMatchType::Type match_type,
base::Optional<SuggestionAnswer::AnswerType> answer_type, base::Optional<SuggestionAnswer::AnswerType> answer_type,
GURL favicon_url) { GURL favicon_url) {
[left_image_consumer_ setLeftImageForAutocompleteType:match_type if (has_match) {
answerType:answer_type [left_image_consumer_ setLeftImageForAutocompleteType:match_type
faviconURL:favicon_url]; answerType:answer_type
faviconURL:favicon_url];
} else {
[left_image_consumer_ setDefaultLeftImage];
}
} }
void OmniboxViewIOS::OnResultsChanged(const AutocompleteResult& result) { void OmniboxViewIOS::OnResultsChanged(const AutocompleteResult& result) {
......
...@@ -51,12 +51,15 @@ OmniboxPopupViewIOS::~OmniboxPopupViewIOS() { ...@@ -51,12 +51,15 @@ OmniboxPopupViewIOS::~OmniboxPopupViewIOS() {
void OmniboxPopupViewIOS::UpdateEditViewIcon() { void OmniboxPopupViewIOS::UpdateEditViewIcon() {
const AutocompleteResult& result = model_->result(); const AutocompleteResult& result = model_->result();
// TODO (crbug.com/1024885): Use a better fallback icon than the icon for the // Use default icon as a fallback
// first match. if (model_->selected_line() == OmniboxPopupModel::kNoMatch) {
const AutocompleteMatch& match = delegate_->OnSelectedMatchImageChanged(/*has_match=*/false,
model_->selected_line() == OmniboxPopupModel::kNoMatch AutocompleteMatchType::NUM_TYPES,
? *result.begin() base::nullopt, GURL());
: result.match_at(model_->selected_line()); return;
}
const AutocompleteMatch& match = result.match_at(model_->selected_line());
base::Optional<SuggestionAnswer::AnswerType> optAnswerType = base::nullopt; base::Optional<SuggestionAnswer::AnswerType> optAnswerType = base::nullopt;
if (match.answer && match.answer->type() > 0 && if (match.answer && match.answer->type() > 0 &&
...@@ -65,8 +68,8 @@ void OmniboxPopupViewIOS::UpdateEditViewIcon() { ...@@ -65,8 +68,8 @@ void OmniboxPopupViewIOS::UpdateEditViewIcon() {
optAnswerType = optAnswerType =
static_cast<SuggestionAnswer::AnswerType>(match.answer->type()); static_cast<SuggestionAnswer::AnswerType>(match.answer->type());
} }
delegate_->OnTopmostSuggestionImageChanged(match.type, optAnswerType, delegate_->OnSelectedMatchImageChanged(/*has_match=*/true, match.type,
match.destination_url); optAnswerType, match.destination_url);
} }
void OmniboxPopupViewIOS::UpdatePopupAppearance() { void OmniboxPopupViewIOS::UpdatePopupAppearance() {
......
...@@ -15,10 +15,14 @@ enum class WindowOpenDisposition; ...@@ -15,10 +15,14 @@ enum class WindowOpenDisposition;
class OmniboxPopupViewSuggestionsDelegate { class OmniboxPopupViewSuggestionsDelegate {
public: public:
// Called whenever the topmost suggestion image has changed. // Called whenever the selected image has changed.
// Current UI should only use |matchType|; new UI may use |answerType| and // has_match indicates if there is a selected match. When there's no selected
// |faviconURL| if available. // match (for example, on NTP with no zero suggest, there's no default match),
virtual void OnTopmostSuggestionImageChanged( // values in match_type, answer_type, and favicon_url are invalid and a
// default image should be used instead. Current UI should only use
// |matchType|; new UI may use |answerType| and |faviconURL| if available.
virtual void OnSelectedMatchImageChanged(
bool has_match,
AutocompleteMatchType::Type match_type, AutocompleteMatchType::Type match_type,
base::Optional<SuggestionAnswer::AnswerType> answer_type, base::Optional<SuggestionAnswer::AnswerType> answer_type,
GURL favicon_url) = 0; GURL favicon_url) = 0;
......
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