Skip to content

Commit 34f5631

Browse files
author
Stepan Khapugin
committed
Adds EG test coverage for Shortcuts.
This CL introduces a few changes to test utils and to Shortcuts feature to allow for writing EG tests: - Initializes shortcuts VC and Coordinator later, to allow flag change in runtime. - Makes -focusOmniboxAndType: accept an empty string argument It also implements EG tests for the Shortcuts feature. Bug: 911100 Change-Id: I9473a3570fbf1594fc94210af27a1b3c8714e4a4 Reviewed-on: https://chromium-review.googlesource.com/c/1344092 Commit-Queue: Stepan Khapugin <[email protected]> Reviewed-by: Eric Noyau <[email protected]> Reviewed-by: Gauthier Ambard <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#613543}(cherry picked from commit d8c044f) Reviewed-on: https://chromium-review.googlesource.com/c/1365592 Reviewed-by: Stepan Khapugin <[email protected]> Cr-Commit-Position: refs/branch-heads/3626@{#108} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
1 parent 3b01859 commit 34f5631

File tree

7 files changed

+326
-23
lines changed

7 files changed

+326
-23
lines changed

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.mm

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,20 +80,6 @@ - (void)start {
8080
forProtocol:@protocol(OmniboxSuggestionCommands)];
8181

8282
_popupView->SetMediator(self.mediator);
83-
84-
if (base::FeatureList::IsEnabled(
85-
omnibox::kOmniboxPopupShortcutIconsInZeroState) &&
86-
!self.browserState->IsOffTheRecord()) {
87-
self.shortcutsCoordinator = [[ShortcutsCoordinator alloc]
88-
initWithBaseViewController:self.popupViewController
89-
browserState:self.browserState];
90-
self.shortcutsCoordinator.dispatcher =
91-
(id<ApplicationCommands, BrowserCommands, UrlLoader,
92-
OmniboxFocuser>)(self.dispatcher);
93-
[self.shortcutsCoordinator start];
94-
self.popupViewController.shortcutsViewController =
95-
self.shortcutsCoordinator.viewController;
96-
}
9783
}
9884

9985
- (void)stop {
@@ -108,6 +94,21 @@ - (BOOL)isOpen {
10894
}
10995

11096
- (void)openPopup {
97+
// Initialize the shortcuts feature when necessary.
98+
if (base::FeatureList::IsEnabled(
99+
omnibox::kOmniboxPopupShortcutIconsInZeroState) &&
100+
!self.browserState->IsOffTheRecord() && !self.shortcutsCoordinator) {
101+
self.shortcutsCoordinator = [[ShortcutsCoordinator alloc]
102+
initWithBaseViewController:self.popupViewController
103+
browserState:self.browserState];
104+
self.shortcutsCoordinator.dispatcher =
105+
(id<ApplicationCommands, BrowserCommands, UrlLoader,
106+
OmniboxFocuser>)(self.dispatcher);
107+
[self.shortcutsCoordinator start];
108+
self.popupViewController.shortcutsViewController =
109+
self.shortcutsCoordinator.viewController;
110+
}
111+
111112
// Show shortcuts when the feature is enabled. Don't show them on NTP as they
112113
// are already part of the NTP.
113114
if (!IsVisibleURLNewTabPage(self.webStateList->GetActiveWebState()) &&

ios/chrome/browser/ui/omnibox/popup/shortcuts/BUILD.gn

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,34 @@ source_set("unit_tests") {
7171
"//ui/base",
7272
]
7373
}
74+
75+
source_set("eg_tests") {
76+
testonly = true
77+
sources = [
78+
"shortcuts_egtest.mm",
79+
]
80+
deps = [
81+
":shortcuts",
82+
"//base",
83+
"//base/test:test_support",
84+
"//components/keyed_service/ios",
85+
"//components/reading_list/core",
86+
"//components/strings",
87+
"//ios/chrome/browser/browser_state",
88+
"//ios/chrome/browser/reading_list",
89+
"//ios/chrome/browser/ui",
90+
"//ios/chrome/browser/ui:feature_flags",
91+
"//ios/chrome/test/app:test_support",
92+
"//ios/chrome/test/base:base",
93+
"//ios/chrome/test/earl_grey:test_support",
94+
"//ios/testing/earl_grey:earl_grey_support",
95+
"//ios/third_party/earl_grey:earl_grey+link",
96+
"//testing/gmock",
97+
"//ui/strings",
98+
]
99+
libs = [
100+
"UIKit.framework",
101+
"XCTest.framework",
102+
]
103+
configs += [ "//build/config/compiler:enable_arc" ]
104+
}
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
1+
// Copyright 2018 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#import <EarlGrey/EarlGrey.h>
6+
#import <XCTest/XCTest.h>
7+
8+
#include "base/strings/sys_string_conversions.h"
9+
#include "base/test/scoped_feature_list.h"
10+
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
11+
#include "ios/chrome/browser/ui/ui_feature_flags.h"
12+
#import "ios/chrome/test/app/chrome_test_util.h"
13+
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
14+
#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
15+
#import "ios/chrome/test/earl_grey/chrome_matchers.h"
16+
#import "ios/chrome/test/earl_grey/chrome_test_case.h"
17+
#include "net/test/embedded_test_server/http_request.h"
18+
#include "net/test/embedded_test_server/http_response.h"
19+
20+
#if !defined(__has_feature) || !__has_feature(objc_arc)
21+
#error "This file requires ARC support."
22+
#endif
23+
24+
namespace {
25+
26+
// Web page that will show up as one of the most visited tiles.
27+
const char kTilePageLoadedString[] = "This is a web page that you visit often";
28+
const char kTilePageURL[] = "/tile-page.html";
29+
const char kTilePageTitle[] = "Often visited page";
30+
31+
// Web page for navigation.
32+
const char kPageLoadedString[] = "Page loaded!";
33+
const char kPageURL[] = "/test-page.html";
34+
const char kPageTitle[] = "Page title!";
35+
36+
// Provides responses for redirect and changed window location URLs.
37+
std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
38+
const net::test_server::HttpRequest& request) {
39+
std::unique_ptr<net::test_server::BasicHttpResponse> http_response =
40+
std::make_unique<net::test_server::BasicHttpResponse>();
41+
http_response->set_code(net::HTTP_OK);
42+
43+
if (request.relative_url == kPageURL) {
44+
http_response->set_content("<html><head><title>" + std::string(kPageTitle) +
45+
"</title></head><body>" +
46+
std::string(kPageLoadedString) +
47+
"</body></html>");
48+
return std::move(http_response);
49+
}
50+
51+
if (request.relative_url == kTilePageURL) {
52+
http_response->set_content(
53+
"<html><head><title>" + std::string(kTilePageTitle) +
54+
"</title></head><body>" + std::string(kTilePageLoadedString) +
55+
"</body></html>");
56+
return std::move(http_response);
57+
}
58+
59+
return nil;
60+
}
61+
62+
} // namespace
63+
64+
// Test case for the Omnibox Shortcuts UI.
65+
@interface ShortcutsTestCase : ChromeTestCase
66+
@end
67+
68+
@implementation ShortcutsTestCase {
69+
base::test::ScopedFeatureList _featureList;
70+
}
71+
72+
- (void)setUp {
73+
[super setUp];
74+
// Enable the shortcuts flag.
75+
_featureList.InitAndEnableFeature(kOmniboxPopupShortcutIconsInZeroState);
76+
77+
// Start a server to be able to navigate to a web page.
78+
self.testServer->RegisterRequestHandler(
79+
base::BindRepeating(&StandardResponse));
80+
GREYAssertTrue(self.testServer->Start(), @"Test server failed to start.");
81+
82+
[ChromeEarlGrey clearBrowsingHistory];
83+
[self prepareMostVisitedTiles];
84+
// Clear pasteboard
85+
[[UIPasteboard generalPasteboard] setItems:@[]];
86+
}
87+
88+
#pragma mark - tests
89+
90+
// Tests that the shortcuts show up on a web page.
91+
- (void)testShowsUp {
92+
[self navigateToAPage];
93+
[ChromeEarlGreyUI focusOmnibox];
94+
[[EarlGrey selectElementWithMatcher:[self mostVisitedTileMatcher]]
95+
assertWithMatcher:grey_sufficientlyVisible()];
96+
}
97+
98+
// Tests that shortcuts don't show up when there are other omnibox suggestions
99+
// available, for example "what you typed" suggestion.
100+
- (void)testNotShownWhenSuggestionsAvailable {
101+
[self navigateToAPage];
102+
[ChromeEarlGreyUI focusOmniboxAndType:@"foo"];
103+
[[EarlGrey selectElementWithMatcher:[self mostVisitedTileMatcher]]
104+
assertWithMatcher:grey_nil()];
105+
}
106+
107+
- (void)testShowsUpWhenOmniboxIsEmpty {
108+
[self navigateToAPage];
109+
// Focus omnibox and hit backspace to erase the text.
110+
[ChromeEarlGreyUI focusOmniboxAndType:@"\b"];
111+
[[EarlGrey selectElementWithMatcher:[self mostVisitedTileMatcher]]
112+
assertWithMatcher:grey_sufficientlyVisible()];
113+
}
114+
115+
// Test that tapping a most visited tile navigates to that page.
116+
- (void)testTapMostVisitedTile {
117+
[self navigateToAPage];
118+
[ChromeEarlGreyUI focusOmnibox];
119+
[[EarlGrey selectElementWithMatcher:[self mostVisitedTileMatcher]]
120+
performAction:grey_tap()];
121+
[ChromeEarlGrey waitForWebViewContainingText:kTilePageLoadedString];
122+
}
123+
124+
- (void)testBookmarksShortcut {
125+
[self navigateToAPage];
126+
[ChromeEarlGreyUI focusOmnibox];
127+
128+
// Tap on bookmarks.
129+
[[EarlGrey selectElementWithMatcher:
130+
chrome_test_util::StaticTextWithAccessibilityLabel(
131+
@"Bookmarks")] performAction:grey_tap()];
132+
133+
// Verify that the bookmarks dialog opened with Done button and "Bookmarks"
134+
// title.
135+
[[EarlGrey
136+
selectElementWithMatcher:grey_allOf(grey_accessibilityTrait(
137+
UIAccessibilityTraitHeader),
138+
grey_accessibilityLabel(@"Bookmarks"),
139+
nil)]
140+
assertWithMatcher:grey_sufficientlyVisible()];
141+
142+
[[EarlGrey selectElementWithMatcher:chrome_test_util::
143+
BookmarksNavigationBarDoneButton()]
144+
performAction:grey_tap()];
145+
146+
// Verify that after tapping Done the omnibox is defocused.
147+
[[EarlGrey selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
148+
assertWithMatcher:grey_sufficientlyVisible()];
149+
}
150+
151+
- (void)testReadingListShortcut {
152+
[self navigateToAPage];
153+
[ChromeEarlGreyUI focusOmnibox];
154+
155+
// Tap on reading list.
156+
[[EarlGrey selectElementWithMatcher:
157+
chrome_test_util::StaticTextWithAccessibilityLabel(
158+
@"Reading List")] performAction:grey_tap()];
159+
160+
// Verify that the reading list dialog opened with Done button and "Reading
161+
// List" title.
162+
[[EarlGrey
163+
selectElementWithMatcher:grey_allOf(
164+
grey_accessibilityTrait(
165+
UIAccessibilityTraitHeader),
166+
grey_accessibilityLabel(@"Reading List"),
167+
nil)]
168+
assertWithMatcher:grey_sufficientlyVisible()];
169+
170+
[[EarlGrey
171+
selectElementWithMatcher:[GREYMatchers matcherForButtonTitle:@"Done"]]
172+
performAction:grey_tap()];
173+
174+
// Verify that after tapping Done the omnibox is defocused.
175+
[[EarlGrey selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
176+
assertWithMatcher:grey_sufficientlyVisible()];
177+
}
178+
179+
- (void)testRecentTabsShortcut {
180+
[self navigateToAPage];
181+
[ChromeEarlGreyUI focusOmnibox];
182+
183+
// Tap on recent tabs.
184+
[[EarlGrey selectElementWithMatcher:
185+
chrome_test_util::StaticTextWithAccessibilityLabel(
186+
@"Recent Tabs")] performAction:grey_tap()];
187+
188+
// Verify that the Recent Tabs dialog opened with Done button and "Recent
189+
// Tabs" title.
190+
[[EarlGrey
191+
selectElementWithMatcher:grey_allOf(
192+
grey_accessibilityTrait(
193+
UIAccessibilityTraitHeader),
194+
grey_accessibilityLabel(@"Recent Tabs"),
195+
nil)]
196+
assertWithMatcher:grey_sufficientlyVisible()];
197+
198+
[[EarlGrey
199+
selectElementWithMatcher:[GREYMatchers matcherForButtonTitle:@"Done"]]
200+
performAction:grey_tap()];
201+
202+
// Verify that after tapping Done the omnibox is defocused.
203+
[[EarlGrey selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
204+
assertWithMatcher:grey_sufficientlyVisible()];
205+
}
206+
207+
- (void)testHistoryShortcut {
208+
[self navigateToAPage];
209+
[ChromeEarlGreyUI focusOmnibox];
210+
211+
// Tap on history.
212+
[[EarlGrey selectElementWithMatcher:
213+
chrome_test_util::StaticTextWithAccessibilityLabel(@"History")]
214+
performAction:grey_tap()];
215+
216+
// Verify that the History dialog opened with Done button and "History"
217+
// title.
218+
[[EarlGrey
219+
selectElementWithMatcher:grey_allOf(grey_accessibilityTrait(
220+
UIAccessibilityTraitHeader),
221+
grey_accessibilityLabel(@"History"),
222+
nil)]
223+
assertWithMatcher:grey_sufficientlyVisible()];
224+
225+
[[EarlGrey
226+
selectElementWithMatcher:[GREYMatchers matcherForButtonTitle:@"Done"]]
227+
performAction:grey_tap()];
228+
229+
// Verify that after tapping Done the omnibox is defocused.
230+
[[EarlGrey selectElementWithMatcher:chrome_test_util::DefocusedLocationView()]
231+
assertWithMatcher:grey_sufficientlyVisible()];
232+
}
233+
234+
#pragma mark - helpers
235+
236+
- (void)navigateToAPage {
237+
const GURL pageURL = self.testServer->GetURL(kPageURL);
238+
[ChromeEarlGrey loadURL:pageURL];
239+
[ChromeEarlGrey waitForWebViewContainingText:kPageLoadedString];
240+
}
241+
242+
- (void)prepareMostVisitedTiles {
243+
const GURL pageURL = self.testServer->GetURL(kTilePageURL);
244+
[ChromeEarlGrey loadURL:pageURL];
245+
[ChromeEarlGrey waitForWebViewContainingText:kTilePageLoadedString];
246+
247+
// After loading URL, need to do another action before opening a new tab
248+
// with the icon present.
249+
[ChromeEarlGrey goBack];
250+
251+
[[self class] closeAllTabs];
252+
[ChromeEarlGrey openNewTab];
253+
}
254+
255+
- (id<GREYMatcher>)mostVisitedTileMatcher {
256+
NSString* pageTitle = base::SysUTF8ToNSString(kTilePageTitle);
257+
return chrome_test_util::StaticTextWithAccessibilityLabel(pageTitle);
258+
}
259+
260+
@end

ios/chrome/browser/ui/omnibox/popup/shortcuts/shortcuts_mediator.mm

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,26 +105,26 @@ - (void)openMostVisitedItem:(ShortcutsMostVisitedItem*)item {
105105
web::NavigationManager::WebLoadParams params(item.URL);
106106
params.transition_type = ui::PAGE_TRANSITION_AUTO_BOOKMARK;
107107
ChromeLoadParams chromeParams(params);
108-
[self.dispatcher loadURLWithParams:chromeParams];
109108
[self.dispatcher cancelOmniboxEdit];
109+
[self.dispatcher loadURLWithParams:chromeParams];
110110
}
111111

112112
- (void)openBookmarks {
113-
[self.dispatcher showBookmarksManager];
114113
[self.dispatcher cancelOmniboxEdit];
114+
[self.dispatcher showBookmarksManager];
115115
}
116116

117117
- (void)openReadingList {
118-
[self.dispatcher showReadingList];
119118
[self.dispatcher cancelOmniboxEdit];
119+
[self.dispatcher showReadingList];
120120
}
121121
- (void)openRecentTabs {
122-
[self.dispatcher showRecentTabs];
123122
[self.dispatcher cancelOmniboxEdit];
123+
[self.dispatcher showRecentTabs];
124124
}
125125
- (void)openHistory {
126-
[self.dispatcher showHistory];
127126
[self.dispatcher cancelOmniboxEdit];
127+
[self.dispatcher showHistory];
128128
}
129129

130130
#pragma mark - MostVisitedSitesObserving

ios/chrome/test/earl_grey/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ chrome_ios_eg_test("ios_chrome_ui_egtests") {
9797
"//ios/chrome/browser/ui/infobars:eg_tests",
9898
"//ios/chrome/browser/ui/ntp:eg_tests",
9999
"//ios/chrome/browser/ui/omnibox/popup:eg_tests",
100+
"//ios/chrome/browser/ui/omnibox/popup/shortcuts:eg_tests",
100101
"//ios/chrome/browser/ui/page_info:eg_tests",
101102
"//ios/chrome/browser/ui/payments:eg_tests",
102103
"//ios/chrome/browser/ui/popup_menu:eg_tests",

ios/chrome/test/earl_grey/chrome_earl_grey_ui.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,12 @@
5050

5151
// Focuses the omnibox by tapping and types |text| into it. The '\n' symbol can
5252
// be passed in order to commit the string.
53+
// If |text| is empty or nil, the omnibox is just focused.
5354
+ (void)focusOmniboxAndType:(NSString*)text;
5455

56+
// Focuses the omnibox by tapping it.
57+
+ (void)focusOmnibox;
58+
5559
// Open a new tab via the tools menu.
5660
+ (void)openNewTab;
5761

0 commit comments

Comments
 (0)