Commit 4a93b46d authored by Sebastien Lalancette's avatar Sebastien Lalancette Committed by Commit Bot

[iOS][SH-iOS] Show Alert When Link-to-Text Link Generation Fails

Added an AlertCoordinator to the BrowserContainerCoordinator, which
will be used to handle link-generation failures coming from the
LinkToTextMediator. Added a UserAction for when the user presses on the
Share Page button of that alert.

Bug: 1136043
Change-Id: I7e4e6c84000a7c760e46b8b7d5251c2de7e9c79e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495262
Auto-Submit: Sebastien Lalancette <seblalancette@chromium.org>
Commit-Queue: Jesse Doherty <jwd@chromium.org>
Reviewed-by: default avatarJesse Doherty <jwd@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarsebsg <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821796}
parent e0b81e3b
...@@ -17,17 +17,21 @@ source_set("browser_container") { ...@@ -17,17 +17,21 @@ source_set("browser_container") {
deps = [ deps = [
":ui", ":ui",
"//base", "//base",
"//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser/link_to_text", "//ios/chrome/browser/link_to_text",
"//ios/chrome/browser/main", "//ios/chrome/browser/main",
"//ios/chrome/browser/overlays", "//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area", "//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/screen_time:buildflags", "//ios/chrome/browser/screen_time:buildflags",
"//ios/chrome/browser/ui/alert_coordinator",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/link_to_text", "//ios/chrome/browser/ui/link_to_text",
"//ios/chrome/browser/ui/overlays", "//ios/chrome/browser/ui/overlays",
"//ios/chrome/browser/web_state_list", "//ios/chrome/browser/web_state_list",
"//ios/web/public", "//ios/web/public",
"//ui/base",
"//ui/strings:ui_strings_grit",
"//url", "//url",
] ]
...@@ -63,6 +67,7 @@ source_set("unit_tests") { ...@@ -63,6 +67,7 @@ source_set("unit_tests") {
testonly = true testonly = true
sources = [ sources = [
"browser_container_coordinator_unittest.mm",
"browser_container_mediator_unittest.mm", "browser_container_mediator_unittest.mm",
"browser_container_view_controller_unittest.mm", "browser_container_view_controller_unittest.mm",
] ]
...@@ -73,13 +78,21 @@ source_set("unit_tests") { ...@@ -73,13 +78,21 @@ source_set("unit_tests") {
":browser_container", ":browser_container",
":ui", ":ui",
"//base/test:test_support", "//base/test:test_support",
"//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser/link_to_text",
"//ios/chrome/browser/main:test_support", "//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/overlays", "//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area", "//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/overlays/test", "//ios/chrome/browser/overlays/test",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/link_to_text",
"//ios/chrome/browser/web_state_list", "//ios/chrome/browser/web_state_list",
"//ios/chrome/test:test_support",
"//ios/web/public/test", "//ios/web/public/test",
"//ios/web/public/test/fakes", "//ios/web/public/test/fakes",
"//testing/gtest", "//testing/gtest",
"//third_party/ocmock",
"//ui/base",
"//ui/strings:ui_strings_grit",
] ]
} }
...@@ -7,10 +7,13 @@ ...@@ -7,10 +7,13 @@
#import <Availability.h> #import <Availability.h>
#include "base/check.h" #include "base/check.h"
#import "base/metrics/user_metrics.h"
#import "base/metrics/user_metrics_action.h"
#import "ios/chrome/browser/link_to_text/link_to_text_payload.h" #import "ios/chrome/browser/link_to_text/link_to_text_payload.h"
#import "ios/chrome/browser/main/browser.h" #import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/overlays/public/overlay_presenter.h" #import "ios/chrome/browser/overlays/public/overlay_presenter.h"
#include "ios/chrome/browser/screen_time/screen_time_buildflags.h" #include "ios/chrome/browser/screen_time/screen_time_buildflags.h"
#import "ios/chrome/browser/ui/alert_coordinator/alert_coordinator.h"
#import "ios/chrome/browser/ui/browser_container/browser_container_mediator.h" #import "ios/chrome/browser/ui/browser_container/browser_container_mediator.h"
#import "ios/chrome/browser/ui/browser_container/browser_container_view_controller.h" #import "ios/chrome/browser/ui/browser_container/browser_container_view_controller.h"
#import "ios/chrome/browser/ui/commands/activity_service_commands.h" #import "ios/chrome/browser/ui/commands/activity_service_commands.h"
...@@ -19,6 +22,9 @@ ...@@ -19,6 +22,9 @@
#import "ios/chrome/browser/ui/link_to_text/link_to_text_consumer.h" #import "ios/chrome/browser/ui/link_to_text/link_to_text_consumer.h"
#import "ios/chrome/browser/ui/link_to_text/link_to_text_mediator.h" #import "ios/chrome/browser/ui/link_to_text/link_to_text_mediator.h"
#import "ios/chrome/browser/ui/overlays/overlay_container_coordinator.h" #import "ios/chrome/browser/ui/overlays/overlay_container_coordinator.h"
#import "ios/chrome/grit/ios_strings.h"
#import "ui/base/l10n/l10n_util.h"
#import "ui/strings/grit/ui_strings.h"
#import "url/gurl.h" #import "url/gurl.h"
#if BUILDFLAG(IOS_SCREEN_TIME_ENABLED) #if BUILDFLAG(IOS_SCREEN_TIME_ENABLED)
...@@ -47,6 +53,8 @@ ...@@ -47,6 +53,8 @@
@property(nonatomic, strong) ChromeCoordinator* screenTimeCoordinator; @property(nonatomic, strong) ChromeCoordinator* screenTimeCoordinator;
// The handler for activity services commands. // The handler for activity services commands.
@property(nonatomic, weak) id<ActivityServiceCommands> activityServiceHandler; @property(nonatomic, weak) id<ActivityServiceCommands> activityServiceHandler;
// Coordinator used to present alerts to the user.
@property(nonatomic, strong) AlertCoordinator* alertCoordinator;
@end @end
@implementation BrowserContainerCoordinator @implementation BrowserContainerCoordinator
...@@ -113,7 +121,31 @@ ...@@ -113,7 +121,31 @@
} }
- (void)linkGenerationFailed { - (void)linkGenerationFailed {
// TODO(crbug.com/1136043): Show an alert. self.alertCoordinator = [[AlertCoordinator alloc]
initWithBaseViewController:self.viewController
browser:self.browser
title:l10n_util::GetNSString(
IDS_IOS_LINK_TO_TEXT_ERROR_TITLE)
message:l10n_util::GetNSString(
IDS_IOS_LINK_TO_TEXT_ERROR_DESCRIPTION)];
[self.alertCoordinator
addItemWithTitle:l10n_util::GetNSString(IDS_APP_OK)
action:^{
base::RecordAction(base::UserMetricsAction(
"SharedHighlights.LinkGenerated.Error.OK"));
}
style:UIAlertActionStyleCancel];
__weak __typeof(self) weakSelf = self;
[self.alertCoordinator
addItemWithTitle:l10n_util::GetNSString(IDS_IOS_SHARE_PAGE_BUTTON_LABEL)
action:^{
base::RecordAction(base::UserMetricsAction(
"SharedHighlights.LinkGenerated.Error.SharePage"));
[weakSelf.activityServiceHandler sharePage];
}
style:UIAlertActionStyleDefault];
[self.alertCoordinator start];
} }
#pragma mark - Private methods #pragma mark - Private methods
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "ios/chrome/browser/ui/browser_container/browser_container_coordinator.h"
#import <UIKit/UIKit.h>
#import "base/mac/foundation_util.h"
#import "base/test/task_environment.h"
#import "ios/chrome/browser/link_to_text/link_to_text_payload.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ui/browser_container/browser_container_view_controller.h"
#import "ios/chrome/browser/ui/commands/activity_service_commands.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/commands/share_highlight_command.h"
#import "ios/chrome/browser/ui/link_to_text/link_to_text_consumer.h"
#import "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/scoped_key_window.h"
#import "ios/web/public/test/fakes/test_web_state.h"
#import "testing/platform_test.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#import "third_party/ocmock/gtest_support.h"
#import "ui/base/l10n/l10n_util.h"
#import "ui/strings/grit/ui_strings.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
// Test fixture for BrowserContainerCoordinator.
class BrowserContainerCoordinatorTest : public PlatformTest {
public:
BrowserContainerCoordinatorTest() {
mocked_handler_ = OCMStrictProtocolMock(@protocol(ActivityServiceCommands));
[browser_.GetCommandDispatcher()
startDispatchingToTarget:mocked_handler_
forProtocol:@protocol(ActivityServiceCommands)];
}
BrowserContainerCoordinator* CreateAndStartCoordinator() {
BrowserContainerCoordinator* coordinator =
[[BrowserContainerCoordinator alloc]
initWithBaseViewController:nil
browser:&browser_];
[coordinator start];
[scoped_key_window_.Get() setRootViewController:coordinator.viewController];
return coordinator;
}
protected:
base::test::TaskEnvironment task_environment_;
TestBrowser browser_;
id mocked_handler_;
ScopedKeyWindow scoped_key_window_;
};
// Tests that the coordinator properly handles link-to-text payload updates.
TEST_F(BrowserContainerCoordinatorTest, LinkToTextConsumerGeneratePayload) {
BrowserContainerCoordinator* coordinator = CreateAndStartCoordinator();
LinkToTextPayload* test_payload =
[[LinkToTextPayload alloc] initWithURL:GURL("https://google.com")
title:@"Some title"
selectedText:@"Selected on page"
sourceView:[[UIView alloc] init]
sourceRect:CGRectMake(0, 1, 2, 3)];
[[mocked_handler_ expect]
shareHighlight:[OCMArg checkWithBlock:^BOOL(
ShareHighlightCommand* command) {
EXPECT_EQ(test_payload.URL, command.URL);
EXPECT_TRUE([test_payload.title isEqualToString:command.title]);
EXPECT_TRUE(
[test_payload.selectedText isEqualToString:command.selectedText]);
EXPECT_EQ(test_payload.sourceView, command.sourceView);
EXPECT_TRUE(
CGRectEqualToRect(test_payload.sourceRect, command.sourceRect));
return YES;
}]];
id<LinkToTextConsumer> consumer =
static_cast<id<LinkToTextConsumer>>(coordinator);
[consumer generatedPayload:test_payload];
[mocked_handler_ verify];
}
// Tests that the coordinator displays an alert when getting an update about how
// a link-to-text link generation failed.
TEST_F(BrowserContainerCoordinatorTest,
LinkToTextConsumerLinkGenerationFailed) {
BrowserContainerCoordinator* coordinator = CreateAndStartCoordinator();
id<LinkToTextConsumer> consumer =
static_cast<id<LinkToTextConsumer>>(coordinator);
[consumer linkGenerationFailed];
EXPECT_TRUE([coordinator.viewController.presentedViewController
isKindOfClass:[UIAlertController class]]);
UIAlertController* alert_controller =
base::mac::ObjCCastStrict<UIAlertController>(
coordinator.viewController.presentedViewController);
ASSERT_EQ(2LU, alert_controller.actions.count);
// First action should be the OK button.
UIAlertAction* ok_action = alert_controller.actions[0];
EXPECT_TRUE(
[l10n_util::GetNSString(IDS_APP_OK) isEqualToString:ok_action.title]);
EXPECT_EQ(UIAlertActionStyleCancel, ok_action.style);
// Second action should the Share button.
UIAlertAction* share_action = alert_controller.actions[1];
EXPECT_TRUE([l10n_util::GetNSString(IDS_IOS_SHARE_PAGE_BUTTON_LABEL)
isEqualToString:share_action.title]);
EXPECT_EQ(UIAlertActionStyleDefault, share_action.style);
}
...@@ -21108,6 +21108,24 @@ should be able to be added at any place in this file. ...@@ -21108,6 +21108,24 @@ should be able to be added at any place in this file.
</description> </description>
</action> </action>
<action name="SharedHighlights.LinkGenerated.Error.OK">
<owner>seblalancette@chromium.org</owner>
<owner>chrome-shared-highlighting@google.com</owner>
<description>
User pressed the 'OK' button on the Link Generation Error Alert dialog (iOS
only).
</description>
</action>
<action name="SharedHighlights.LinkGenerated.Error.SharePage">
<owner>seblalancette@chromium.org</owner>
<owner>chrome-shared-highlighting@google.com</owner>
<description>
User pressed the 'Share Page...' button on the Link Generation Error Alert
dialog (iOS only).
</description>
</action>
<action name="SharingHubAndroid.BottomRowScrolled"> <action name="SharingHubAndroid.BottomRowScrolled">
<obsolete> <obsolete>
Renamed to SharingHubAndroid.ThirdPartyAppsScrolled 09/2020. Renamed to SharingHubAndroid.ThirdPartyAppsScrolled 09/2020.
......
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