Commit ef1c11a2 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

Add "Add to Bookmark" to the share menu

This CL adds a new activity to the activity menu (share menu) to have
allow bookmarking the current page.

Bug: 829324
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If03dedc18b3f514bf54b75a8508dad3bd4c3bf84
Reviewed-on: https://chromium-review.googlesource.com/1012067Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551274}
parent e8a8d37b
...@@ -11,6 +11,8 @@ source_set("activity_services") { ...@@ -11,6 +11,8 @@ source_set("activity_services") {
"activity_type_util.mm", "activity_type_util.mm",
"appex_constants.h", "appex_constants.h",
"appex_constants.mm", "appex_constants.mm",
"bookmark_activity.h",
"bookmark_activity.mm",
"canonical_url_retriever.h", "canonical_url_retriever.h",
"canonical_url_retriever.mm", "canonical_url_retriever.mm",
"chrome_activity_item_source.h", "chrome_activity_item_source.h",
...@@ -31,9 +33,11 @@ source_set("activity_services") { ...@@ -31,9 +33,11 @@ source_set("activity_services") {
"resources:activity_services_print", "resources:activity_services_print",
"resources:activity_services_reading_list", "resources:activity_services_reading_list",
"//base", "//base",
"//components/bookmarks/browser",
"//components/ui_metrics", "//components/ui_metrics",
"//ios/chrome/app/strings", "//ios/chrome/app/strings",
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/bookmarks",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/passwords", "//ios/chrome/browser/passwords",
"//ios/chrome/browser/snapshots", "//ios/chrome/browser/snapshots",
...@@ -89,12 +93,16 @@ source_set("unit_tests") { ...@@ -89,12 +93,16 @@ source_set("unit_tests") {
":activity_services", ":activity_services",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//components/bookmarks/browser",
"//components/bookmarks/test",
"//components/ui_metrics", "//components/ui_metrics",
"//ios/chrome/app/strings", "//ios/chrome/app/strings",
"//ios/chrome/browser/bookmarks",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/passwords", "//ios/chrome/browser/passwords",
"//ios/chrome/browser/snapshots", "//ios/chrome/browser/snapshots",
"//ios/chrome/browser/tabs", "//ios/chrome/browser/tabs",
"//ios/chrome/browser/ui:feature_flags",
"//ios/chrome/browser/ui/activity_services/requirements", "//ios/chrome/browser/ui/activity_services/requirements",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/testing:ios_test_support", "//ios/testing:ios_test_support",
......
...@@ -8,9 +8,11 @@ ...@@ -8,9 +8,11 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h"
#import "ios/chrome/browser/passwords/password_form_filler.h" #import "ios/chrome/browser/passwords/password_form_filler.h"
#import "ios/chrome/browser/ui/activity_services/activity_type_util.h" #import "ios/chrome/browser/ui/activity_services/activity_type_util.h"
#import "ios/chrome/browser/ui/activity_services/appex_constants.h" #import "ios/chrome/browser/ui/activity_services/appex_constants.h"
#import "ios/chrome/browser/ui/activity_services/bookmark_activity.h"
#import "ios/chrome/browser/ui/activity_services/chrome_activity_item_source.h" #import "ios/chrome/browser/ui/activity_services/chrome_activity_item_source.h"
#import "ios/chrome/browser/ui/activity_services/print_activity.h" #import "ios/chrome/browser/ui/activity_services/print_activity.h"
#import "ios/chrome/browser/ui/activity_services/reading_list_activity.h" #import "ios/chrome/browser/ui/activity_services/reading_list_activity.h"
...@@ -57,7 +59,9 @@ NSString* const kActivityServicesSnackbarCategory = ...@@ -57,7 +59,9 @@ NSString* const kActivityServicesSnackbarCategory =
- (NSArray*)activityItemsForData:(ShareToData*)data; - (NSArray*)activityItemsForData:(ShareToData*)data;
// Returns an array of UIActivity objects that can handle the given |data|. // Returns an array of UIActivity objects that can handle the given |data|.
- (NSArray*)applicationActivitiesForData:(ShareToData*)data - (NSArray*)applicationActivitiesForData:(ShareToData*)data
dispatcher:(id<BrowserCommands>)dispatcher; dispatcher:(id<BrowserCommands>)dispatcher
bookmarkModel:
(bookmarks::BookmarkModel*)bookmarkModel;
// Processes |extensionItems| returned from App Extension invocation returning // Processes |extensionItems| returned from App Extension invocation returning
// the |activityType|. Calls shareDelegate_ with the processed returned items // the |activityType|. Calls shareDelegate_ with the processed returned items
// and |result| of activity. Returns whether caller should reset UI. // and |result| of activity. Returns whether caller should reset UI.
...@@ -132,15 +136,18 @@ NSString* const kActivityServicesSnackbarCategory = ...@@ -132,15 +136,18 @@ NSString* const kActivityServicesSnackbarCategory =
dispatcher_ = dispatcher; dispatcher_ = dispatcher;
bookmarks::BookmarkModel* bookmarkModel =
ios::BookmarkModelFactory::GetForBrowserState(browserState);
DCHECK(!activityViewController_); DCHECK(!activityViewController_);
activityViewController_ = [[UIActivityViewController alloc] activityViewController_ = [[UIActivityViewController alloc]
initWithActivityItems:[self activityItemsForData:data] initWithActivityItems:[self activityItemsForData:data]
applicationActivities:[self applicationActivitiesForData:data applicationActivities:[self applicationActivitiesForData:data
dispatcher:dispatcher]]; dispatcher:dispatcher
bookmarkModel:bookmarkModel]];
// Reading List and Print activities refer to iOS' version of these. // Reading List and Print activities refer to iOS' version of these.
// Chrome-specific implementations of these two activities are provided below // Chrome-specific implementations of these two activities are provided below
// in applicationActivitiesForData:dispatcher: // in applicationActivitiesForData:dispatcher:bookmarkModel:
NSArray* excludedActivityTypes = @[ NSArray* excludedActivityTypes = @[
UIActivityTypeAddToReadingList, UIActivityTypePrint, UIActivityTypeAddToReadingList, UIActivityTypePrint,
UIActivityTypeSaveToCameraRoll UIActivityTypeSaveToCameraRoll
...@@ -237,19 +244,31 @@ NSString* const kActivityServicesSnackbarCategory = ...@@ -237,19 +244,31 @@ NSString* const kActivityServicesSnackbarCategory =
} }
- (NSArray*)applicationActivitiesForData:(ShareToData*)data - (NSArray*)applicationActivitiesForData:(ShareToData*)data
dispatcher:(id<BrowserCommands>)dispatcher { dispatcher:(id<BrowserCommands>)dispatcher
bookmarkModel:
(bookmarks::BookmarkModel*)bookmarkModel {
NSMutableArray* applicationActivities = [NSMutableArray array]; NSMutableArray* applicationActivities = [NSMutableArray array];
if (data.isPagePrintable) {
PrintActivity* printActivity = [[PrintActivity alloc] init];
printActivity.dispatcher = dispatcher;
[applicationActivities addObject:printActivity];
}
if (data.shareURL.SchemeIsHTTPOrHTTPS()) { if (data.shareURL.SchemeIsHTTPOrHTTPS()) {
ReadingListActivity* readingListActivity = ReadingListActivity* readingListActivity =
[[ReadingListActivity alloc] initWithURL:data.shareURL [[ReadingListActivity alloc] initWithURL:data.shareURL
title:data.title title:data.title
dispatcher:dispatcher]; dispatcher:dispatcher];
[applicationActivities addObject:readingListActivity]; [applicationActivities addObject:readingListActivity];
if (IsUIRefreshPhase1Enabled()) {
// Initialize the bookmarkActivity with the visible URL as it is the URL
// used to determine if the page is already bookmarked or not.
BookmarkActivity* bookmarkActivity =
[[BookmarkActivity alloc] initWithURL:data.visibleURL
bookmarkModel:bookmarkModel
dispatcher:dispatcher];
[applicationActivities addObject:bookmarkActivity];
}
}
if (data.isPagePrintable) {
PrintActivity* printActivity = [[PrintActivity alloc] init];
printActivity.dispatcher = dispatcher;
[applicationActivities addObject:printActivity];
} }
return applicationActivities; return applicationActivities;
} }
......
// Copyright 2014 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.
#ifndef IOS_CHROME_BROWSER_UI_ACTIVITY_SERVICES_BOOKMARK_ACTIVITY_H_
#define IOS_CHROME_BROWSER_UI_ACTIVITY_SERVICES_BOOKMARK_ACTIVITY_H_
#import <UIKit/UIKit.h>
namespace bookmarks {
class BookmarkModel;
}
@protocol BrowserCommands;
class GURL;
// Activity that adds the page to bookmarks.
@interface BookmarkActivity : UIActivity
// Initialize the bookmark activity with the |URL| to check to know if the page
// is already bookmarked in the |bookmarkModel|. The |dispatcher| is used to add
// the page to the bookmarks.
- (instancetype)initWithURL:(const GURL&)URL
bookmarkModel:(bookmarks::BookmarkModel*)bookmarkModel
dispatcher:(id<BrowserCommands>)dispatcher;
- (instancetype)init NS_UNAVAILABLE;
// Identifier for the bookmark activity.
+ (NSString*)activityIdentifier;
@end
#endif // IOS_CHROME_BROWSER_UI_ACTIVITY_SERVICES_BOOKMARK_ACTIVITY_H_
// Copyright 2014 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/activity_services/bookmark_activity.h"
#include "base/logging.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "ios/chrome/browser/ui/commands/browser_commands.h"
#include "ios/chrome/grit/ios_strings.h"
#include "ui/base/l10n/l10n_util_mac.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
namespace {
NSString* const kBookmarkActivityType = @"com.google.chrome.bookmarkActivity";
} // namespace
@interface BookmarkActivity ()
// The bookmarks model to know if the page is bookmarked.
@property(nonatomic, assign) bookmarks::BookmarkModel* bookmarkModel;
// The URL for the activity.
@property(nonatomic, assign) GURL URL;
// The dispatcher that handles when the activity is performed.
@property(nonatomic, weak) id<BrowserCommands> dispatcher;
@end
@implementation BookmarkActivity
@synthesize bookmarkModel = _bookmarkModel;
@synthesize dispatcher = _dispatcher;
@synthesize URL = _URL;
- (instancetype)initWithURL:(const GURL&)URL
bookmarkModel:(bookmarks::BookmarkModel*)bookmarkModel
dispatcher:(id<BrowserCommands>)dispatcher {
self = [super init];
if (self) {
_URL = URL;
_bookmarkModel = bookmarkModel;
_dispatcher = dispatcher;
}
return self;
}
+ (NSString*)activityIdentifier {
return kBookmarkActivityType;
}
#pragma mark - UIActivity
- (NSString*)activityType {
return [BookmarkActivity activityIdentifier];
}
- (NSString*)activityTitle {
if (self.bookmarkModel && self.bookmarkModel->IsBookmarked(self.URL))
return l10n_util::GetNSString(IDS_IOS_TOOLS_MENU_EDIT_BOOKMARK);
return l10n_util::GetNSString(IDS_IOS_TOOLS_MENU_ADD_TO_BOOKMARKS);
}
- (UIImage*)activityImage {
return [UIImage imageNamed:@"popup_menu_bookmarks"];
}
- (BOOL)canPerformWithActivityItems:(NSArray*)activityItems {
return YES;
}
- (void)prepareWithActivityItems:(NSArray*)activityItems {
}
+ (UIActivityCategory)activityCategory {
return UIActivityCategoryAction;
}
- (void)performActivity {
[self.dispatcher bookmarkPage];
[self activityDidFinish:YES];
}
@end
...@@ -14,16 +14,17 @@ ...@@ -14,16 +14,17 @@
// Designated initializer. // Designated initializer.
- (id)initWithShareURL:(const GURL&)shareURL - (id)initWithShareURL:(const GURL&)shareURL
passwordManagerURL:(const GURL&)passwordManagerURL visibleURL:(const GURL&)visibleURL
title:(NSString*)title title:(NSString*)title
isOriginalTitle:(BOOL)isOriginalTitle isOriginalTitle:(BOOL)isOriginalTitle
isPagePrintable:(BOOL)isPagePrintable isPagePrintable:(BOOL)isPagePrintable
thumbnailGenerator:(ThumbnailGeneratorBlock)thumbnailGenerator; thumbnailGenerator:(ThumbnailGeneratorBlock)thumbnailGenerator;
// The URL to be shared with share extensions. // The URL to be shared with share extensions. This URL is the canonical URL of
// the page.
@property(nonatomic, readonly) const GURL& shareURL; @property(nonatomic, readonly) const GURL& shareURL;
// The URL to be shared with password managers. // The visible URL of the page.
@property(nonatomic, readonly) const GURL& passwordManagerURL; @property(nonatomic, readonly) const GURL& visibleURL;
// NSURL versions of 'shareURL' and 'passwordManagerURL'. Use only for passing // NSURL versions of 'shareURL' and 'passwordManagerURL'. Use only for passing
// to libraries that take NSURL. // to libraries that take NSURL.
......
...@@ -18,8 +18,8 @@ ...@@ -18,8 +18,8 @@
// URL to be shared with share extensions. // URL to be shared with share extensions.
GURL shareURL_; GURL shareURL_;
// URL to be shared with password managers. // Visible URL of the page.
GURL passwordManagerURL_; GURL visibleURL_;
// Title to be shared (not nil). // Title to be shared (not nil).
NSString* title_; NSString* title_;
...@@ -46,18 +46,18 @@ ...@@ -46,18 +46,18 @@
@synthesize isPagePrintable = isPagePrintable_; @synthesize isPagePrintable = isPagePrintable_;
- (id)initWithShareURL:(const GURL&)shareURL - (id)initWithShareURL:(const GURL&)shareURL
passwordManagerURL:(const GURL&)passwordManagerURL visibleURL:(const GURL&)visibleURL
title:(NSString*)title title:(NSString*)title
isOriginalTitle:(BOOL)isOriginalTitle isOriginalTitle:(BOOL)isOriginalTitle
isPagePrintable:(BOOL)isPagePrintable isPagePrintable:(BOOL)isPagePrintable
thumbnailGenerator:(ThumbnailGeneratorBlock)thumbnailGenerator { thumbnailGenerator:(ThumbnailGeneratorBlock)thumbnailGenerator {
DCHECK(shareURL.is_valid()); DCHECK(shareURL.is_valid());
DCHECK(passwordManagerURL.is_valid()); DCHECK(visibleURL.is_valid());
DCHECK(title); DCHECK(title);
self = [super init]; self = [super init];
if (self) { if (self) {
shareURL_ = shareURL; shareURL_ = shareURL;
passwordManagerURL_ = passwordManagerURL; visibleURL_ = visibleURL;
title_ = [title copy]; title_ = [title copy];
isOriginalTitle_ = isOriginalTitle; isOriginalTitle_ = isOriginalTitle;
isPagePrintable_ = isPagePrintable; isPagePrintable_ = isPagePrintable;
...@@ -70,8 +70,8 @@ ...@@ -70,8 +70,8 @@
return shareURL_; return shareURL_;
} }
- (const GURL&)passwordManagerURL { - (const GURL&)visibleURL {
return passwordManagerURL_; return visibleURL_;
} }
- (NSURL*)shareNSURL { - (NSURL*)shareNSURL {
...@@ -79,7 +79,7 @@ ...@@ -79,7 +79,7 @@
} }
- (NSURL*)passwordManagerNSURL { - (NSURL*)passwordManagerNSURL {
return net::NSURLWithGURL(passwordManagerURL_); return net::NSURLWithGURL(visibleURL_);
} }
@end @end
...@@ -55,7 +55,7 @@ ShareToData* ShareToDataForTab(Tab* tab, const GURL& shareURL) { ...@@ -55,7 +55,7 @@ ShareToData* ShareToDataForTab(Tab* tab, const GURL& shareURL) {
!shareURL.is_empty() ? shareURL : tab.webState->GetVisibleURL(); !shareURL.is_empty() ? shareURL : tab.webState->GetVisibleURL();
return [[ShareToData alloc] initWithShareURL:finalURLToShare return [[ShareToData alloc] initWithShareURL:finalURLToShare
passwordManagerURL:tab.webState->GetVisibleURL() visibleURL:tab.webState->GetVisibleURL()
title:tab.title title:tab.title
isOriginalTitle:is_original_title isOriginalTitle:is_original_title
isPagePrintable:is_page_printable isPagePrintable:is_page_printable
......
...@@ -128,7 +128,7 @@ TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandlingNpShareUrl) { ...@@ -128,7 +128,7 @@ TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandlingNpShareUrl) {
ASSERT_TRUE(actual_data); ASSERT_TRUE(actual_data);
EXPECT_EQ(kExpectedShareUrl, actual_data.shareURL); EXPECT_EQ(kExpectedShareUrl, actual_data.shareURL);
EXPECT_EQ(kExpectedUrl, actual_data.passwordManagerURL); EXPECT_EQ(kExpectedUrl, actual_data.visibleURL);
EXPECT_NSEQ(base::SysUTF8ToNSString(kExpectedTitle), actual_data.title); EXPECT_NSEQ(base::SysUTF8ToNSString(kExpectedTitle), actual_data.title);
EXPECT_TRUE(actual_data.isOriginalTitle); EXPECT_TRUE(actual_data.isOriginalTitle);
EXPECT_FALSE(actual_data.isPagePrintable); EXPECT_FALSE(actual_data.isPagePrintable);
...@@ -147,7 +147,7 @@ TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandlingNoShareUrl) { ...@@ -147,7 +147,7 @@ TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandlingNoShareUrl) {
ASSERT_TRUE(actual_data); ASSERT_TRUE(actual_data);
EXPECT_EQ(kExpectedUrl, actual_data.shareURL); EXPECT_EQ(kExpectedUrl, actual_data.shareURL);
EXPECT_EQ(kExpectedUrl, actual_data.passwordManagerURL); EXPECT_EQ(kExpectedUrl, actual_data.visibleURL);
EXPECT_NSEQ(base::SysUTF8ToNSString(kExpectedTitle), actual_data.title); EXPECT_NSEQ(base::SysUTF8ToNSString(kExpectedTitle), actual_data.title);
EXPECT_TRUE(actual_data.isOriginalTitle); EXPECT_TRUE(actual_data.isOriginalTitle);
EXPECT_FALSE(actual_data.isPagePrintable); EXPECT_FALSE(actual_data.isPagePrintable);
......
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