[iOS] Separate flags for copied text/images in omnibox and elsewhere

This CL separates the one "copied text behavior" flag into 3. One
controls the copied text suggestion in the omnibox, one controls the
copied image suggestion in the omnibox, and the last controls all
copied content behavior elesewhere (omnibox long-press, search
accelerator menu)

Bug: 913958
Change-Id: I36ae3266ce03c657697777b9c43b0ba206bc677b
Reviewed-on: https://chromium-review.googlesource.com/c/1402754
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621534}

components/omnibox/browser/clipboard_url_provider.cc

@@ -131,7 +131,8 @@ base::Optional<AutocompleteMatch> ClipboardURLProvider::CreateURLMatch(
 base::Optional<AutocompleteMatch> ClipboardURLProvider::CreateTextMatch(
     const AutocompleteInput& input) {
   // Only try text match if feature is enabled
-  if (!base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (!base::FeatureList::IsEnabled(
+          omnibox::kEnableClipboardProviderTextSuggestions)) {
     return base::nullopt;
   }
 
@@ -171,7 +172,8 @@ base::Optional<AutocompleteMatch> ClipboardURLProvider::CreateTextMatch(
 base::Optional<AutocompleteMatch> ClipboardURLProvider::CreateImageMatch(
     const AutocompleteInput& input) {
   // Only try image match if feature is enabled
-  if (!base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (!base::FeatureList::IsEnabled(
+          omnibox::kEnableClipboardProviderImageSuggestions)) {
     return base::nullopt;
   }
 

components/omnibox/browser/clipboard_url_provider_unittest.cc

@@ -90,7 +90,8 @@ TEST_F(ClipboardURLProviderTest, HasMultipleMatches) {
 
 TEST_F(ClipboardURLProviderTest, MatchesText) {
   base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(omnibox::kCopiedTextBehavior);
+  base::Feature textFeature = omnibox::kEnableClipboardProviderTextSuggestions;
+  feature_list.InitAndEnableFeature(textFeature);
   auto template_url_service = std::make_unique<TemplateURLService>(
       /*initializers=*/nullptr, /*count=*/0);
   client_->set_template_url_service(std::move(template_url_service));

components/omnibox/browser/omnibox_field_trial.cc

@@ -158,6 +158,16 @@ const base::Feature kEnableClipboardProvider {
 #endif
 };
 
+// Feature to enable clipboard provider to suggest copied text.
+const base::Feature kEnableClipboardProviderTextSuggestions{
+    "OmniboxEnableClipboardProviderTextSuggestions",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
+// Feature to enable clipboard provider to suggest searching for copied images.
+const base::Feature kEnableClipboardProviderImageSuggestions{
+    "OmniboxEnableClipboardProviderImageSuggestions",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Feature to enable the search provider to send a request to the suggest
 // server on focus.  This allows the suggest server to warm up, by, for
 // example, loading per-user models into memory.  Having a per-user model
@@ -224,10 +234,6 @@ const base::Feature kDocumentProvider{"OmniboxDocumentProvider",
 const base::Feature kOmniboxPopupShortcutIconsInZeroState{
     "OmniboxPopupShortcutIconsInZeroState", base::FEATURE_DISABLED_BY_DEFAULT};
 
-// Feature to differentiate between a copied url and copied text
-const base::Feature kCopiedTextBehavior{"CopiedTextBehavior",
-                                        base::FEATURE_DISABLED_BY_DEFAULT};
-
 }  // namespace omnibox
 
 namespace {

components/omnibox/browser/omnibox_field_trial.h

@@ -39,6 +39,8 @@ extern const base::Feature kOmniboxTabSwitchSuggestions;
 extern const base::Feature kExperimentalKeywordMode;
 extern const base::Feature kOmniboxPedalSuggestions;
 extern const base::Feature kEnableClipboardProvider;
+extern const base::Feature kEnableClipboardProviderTextSuggestions;
+extern const base::Feature kEnableClipboardProviderImageSuggestions;
 extern const base::Feature kSearchProviderWarmUpOnFocus;
 extern const base::Feature kZeroSuggestRedirectToChrome;
 extern const base::Feature kDisplayTitleForCurrentUrl;
@@ -49,7 +51,6 @@ extern const base::Feature kUIExperimentVerticalMargin;
 extern const base::Feature kSpeculativeServiceWorkerStartOnQueryInput;
 extern const base::Feature kDocumentProvider;
 extern const base::Feature kOmniboxPopupShortcutIconsInZeroState;
-extern const base::Feature kCopiedTextBehavior;
 
 }  // namespace omnibox
 

ios/chrome/browser/about_flags.mm

@@ -485,12 +485,19 @@
     {"find-in-page-iframe", flag_descriptions::kFindInPageiFrameName,
      flag_descriptions::kFindInPageiFrameDescription, flags_ui::kOsIos,
      FEATURE_VALUE_TYPE(kFindInPageiFrame)},
-    {"search-copied-image", flag_descriptions::kSearchCopiedImageName,
-     flag_descriptions::kSearchCopiedImageDescription, flags_ui::kOsIos,
-     FEATURE_VALUE_TYPE(kSearchCopiedImage)},
-    {"copied-text-behavior", flag_descriptions::kCopiedTextBehaviorName,
-     flag_descriptions::kCopiedTextBehaviorName, flags_ui::kOsIos,
-     FEATURE_VALUE_TYPE(omnibox::kCopiedTextBehavior)},
+    {"enable-clipboard-provider-text-suggestions",
+     flag_descriptions::kEnableClipboardProviderTextSuggestionsName,
+     flag_descriptions::kEnableClipboardProviderTextSuggestionsDescription,
+     flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(omnibox::kEnableClipboardProviderTextSuggestions)},
+    {"enable-clipboard-provider-image-suggestions",
+     flag_descriptions::kEnableClipboardProviderImageSuggestionsName,
+     flag_descriptions::kEnableClipboardProviderImageSuggestionsDescription,
+     flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(omnibox::kEnableClipboardProviderImageSuggestions)},
+    {"copied-content-behavior", flag_descriptions::kCopiedContentBehaviorName,
+     flag_descriptions::kCopiedContentBehaviorName, flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(kCopiedContentBehavior)},
     {"new-password-form-parsing-for-saving",
      flag_descriptions::kNewPasswordFormParsingForSavingName,
      flag_descriptions::kNewPasswordFormParsingForSavingDescription,

ios/chrome/browser/ios_chrome_flag_descriptions.cc

@@ -218,6 +218,12 @@ const char kContextualSearch[] = "Contextual Search";
 const char kContextualSearchDescription[] =
     "Whether or not Contextual Search is enabled.";
 
+const char kCopiedContentBehaviorName[] =
+    "Enable differentiating between copied urls, text, and images";
+const char kCopiedContentBehaviorDescription[] =
+    "When enabled, places that handled copied urls (omnibox long-press, toolbar"
+    "menus) will differentiate between copied urls, text, and images.";
+
 const char kDetectMainThreadFreezeName[] = "Detect freeze in the main thread.";
 const char kDetectMainThreadFreezeDescription[] =
     "A crash report will be uploaded if the main thread is frozen more than "
@@ -230,6 +236,16 @@ const char kNewClearBrowsingDataUIName[] = "Clear Browsing Data UI";
 const char kNewClearBrowsingDataUIDescription[] =
     "Enable new Clear Browsing Data UI.";
 
+const char kEnableClipboardProviderTextSuggestionsName[] =
+    "Enable copied text provider";
+const char kEnableClipboardProviderTextSuggestionsDescription[] =
+    "Enable suggesting a search for text copied to the clipboard";
+
+const char kEnableClipboardProviderImageSuggestionsName[] =
+    "Enable copied image provider";
+const char kEnableClipboardProviderImageSuggestionsDescription[] =
+    "Enable suggesting a search for the image copied to the clipboard";
+
 const char kFCMInvalidationsName[] =
     "Enable invalidations delivery via new FCM based protocol";
 const char kFCMInvalidationsDescription[] =
@@ -326,11 +342,6 @@ const char kIgnoresViewportScaleLimitsName[] = "Ignore Viewport Scale Limits";
 const char kIgnoresViewportScaleLimitsDescription[] =
     "When enabled the page can always be scaled, regardless of author intent.";
 
-const char kSearchCopiedImageName[] = "Search copied image";
-const char kSearchCopiedImageDescription[] =
-    "Enable searching from the toolbar search button for an image copied to "
-    "the system pasteboard.";
-
 const char kSearchIconToggleName[] = "Change the icon for the search button";
 const char kSearchIconToggleDescription[] =
     "Different icons for the search button.";
@@ -349,12 +360,6 @@ const char kSSOWithWKWebViewName[] = "SSO with WKWebView";
 const char kSSOWithWKWebViewDescription[] =
     "Using WKWebView instead of UIWebView in SSO";
 
-const char kCopiedTextBehaviorName[] =
-    "Enable differentiating between copied text and urls";
-const char kCopiedTextBehaviorDescription[] =
-    "When enabled, places that handled copied urls (omnibox long-press, toolbar"
-    "menus) will differentiate between copied text and copied images.";
-
 const char kToolbarContainerName[] = "Use Toolbar Containers";
 const char kToolbarContainerDescription[] =
     "When enabled, the toolbars and their fullscreen animations will be "

ios/chrome/browser/ios_chrome_flag_descriptions.h

@@ -172,6 +172,11 @@ extern const char kClosingLastIncognitoTabDescription[];
 extern const char kContextualSearch[];
 extern const char kContextualSearchDescription[];
 
+// Title and description for the flag to diffentiate between copied
+// urls, strings, and images.
+extern const char kCopiedContentBehaviorName[];
+extern const char kCopiedContentBehaviorDescription[];
+
 // Title and description for the flag to enable drag and drop.
 extern const char kDragAndDropName[];
 extern const char kDragAndDropDescription[];
@@ -180,6 +185,16 @@ extern const char kDragAndDropDescription[];
 extern const char kNewClearBrowsingDataUIName[];
 extern const char kNewClearBrowsingDataUIDescription[];
 
+// Title and description for the flag to enable the clipboard provider to
+// suggest copied text
+extern const char kEnableClipboardProviderTextSuggestionsName[];
+extern const char kEnableClipboardProviderTextSuggestionsDescription[];
+
+// Title and description for the flag to enable the clipboard provider to
+// suggest searchihng for copied imagse
+extern const char kEnableClipboardProviderImageSuggestionsName[];
+extern const char kEnableClipboardProviderImageSuggestionsDescription[];
+
 // Title and description for the flag to enable invaliations delivery via FCM.
 extern const char kFCMInvalidationsName[];
 extern const char kFCMInvalidationsDescription[];
@@ -261,10 +276,6 @@ extern const char kPhysicalWebDescription[];
 extern const char kIgnoresViewportScaleLimitsName[];
 extern const char kIgnoresViewportScaleLimitsDescription[];
 
-// Title and description for the flag to enable searching for a copied image.
-extern const char kSearchCopiedImageName[];
-extern const char kSearchCopiedImageDescription[];
-
 // Title and description for the flag to toggle the flag of the search button.
 extern const char kSearchIconToggleName[];
 extern const char kSearchIconToggleDescription[];
@@ -283,11 +294,6 @@ extern const char kShowAutofillTypePredictionsDescription[];
 extern const char kSSOWithWKWebViewName[];
 extern const char kSSOWithWKWebViewDescription[];
 
-// Title and description for the flag to diffentiate between copied
-// text and urls.
-extern const char kCopiedTextBehaviorName[];
-extern const char kCopiedTextBehaviorDescription[];
-
 // Title and description for the flag to enable the toolbar container
 // implementation.
 extern const char kToolbarContainerName[];

ios/chrome/browser/ui/location_bar/location_bar_view_controller.mm

@@ -432,7 +432,7 @@ - (void)showLongPressMenu:(UILongPressGestureRecognizer*)sender {
     // when it's the first time setting the first responder.
     dispatch_async(dispatch_get_main_queue(), ^{
       UIMenuController* menu = [UIMenuController sharedMenuController];
-      if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+      if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
         UIMenuItem* visitCopiedLink = [[UIMenuItem alloc]
             initWithTitle:l10n_util::GetNSString(IDS_IOS_VISIT_COPIED_LINK)
                    action:@selector(visitCopiedLink:)];
@@ -458,19 +458,19 @@ - (BOOL)canPerformAction:(SEL)action withSender:(id)sender {
     return true;
   }
 
-  // Remove along with flag kCopiedTextBehavior
+  // remove along with flag kCopiedContentBehavior
   if (action == @selector(pasteAndGo:)) {
-    DCHECK(!base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(!base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return UIPasteboard.generalPasteboard.string.length > 0;
   }
   ClipboardRecentContent* clipboardRecentContent =
       ClipboardRecentContent::GetInstance();
   if (action == @selector(visitCopiedLink:)) {
-    DCHECK(base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return clipboardRecentContent->GetRecentURLFromClipboard().has_value();
   }
   if (action == @selector(searchCopiedText:)) {
-    DCHECK(base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return !clipboardRecentContent->GetRecentURLFromClipboard().has_value() &&
            clipboardRecentContent->GetRecentTextFromClipboard().has_value();
   }
@@ -493,7 +493,7 @@ - (void)searchCopiedText:(id)sender {
 // so we need two different selectors.
 - (void)pasteAndGo:(id)sender {
   NSString* query;
-  if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
     ClipboardRecentContent* clipboardRecentContent =
         ClipboardRecentContent::GetInstance();
     if (base::Optional<GURL> optional_url =

ios/chrome/browser/ui/omnibox/omnibox_view_controller.mm

@@ -81,7 +81,7 @@ - (void)viewDidLoad {
 
   // Add Paste and Go option to the editing menu
   UIMenuController* menu = [UIMenuController sharedMenuController];
-  if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
     UIMenuItem* visitCopiedLink = [[UIMenuItem alloc]
         initWithTitle:l10n_util::GetNSString(IDS_IOS_VISIT_COPIED_LINK)
                action:@selector(visitCopiedLink:)];
@@ -243,19 +243,19 @@ - (void)updateClearButtonVisibility {
 #pragma mark - UIMenuItem
 
 - (BOOL)canPerformAction:(SEL)action withSender:(id)sender {
-  // Remove with flag kCopiedTextBehavior
+  // Remove with flag kCopiedContentBehavior
   if (action == @selector(pasteAndGo:)) {
-    DCHECK(!base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(!base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return UIPasteboard.generalPasteboard.string.length > 0;
   }
   ClipboardRecentContent* clipboardRecentContent =
       ClipboardRecentContent::GetInstance();
   if (action == @selector(visitCopiedLink:)) {
-    DCHECK(base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return clipboardRecentContent->GetRecentURLFromClipboard().has_value();
   }
   if (action == @selector(searchCopiedText:)) {
-    DCHECK(base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+    DCHECK(base::FeatureList::IsEnabled(kCopiedContentBehavior));
     return !clipboardRecentContent->GetRecentURLFromClipboard().has_value() &&
            clipboardRecentContent->GetRecentTextFromClipboard().has_value();
   }
@@ -274,7 +274,7 @@ - (void)searchCopiedText:(id)sender {
 // so we need two different selectors.
 - (void)pasteAndGo:(id)sender {
   NSString* query;
-  if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
     ClipboardRecentContent* clipboardRecentContent =
         ClipboardRecentContent::GetInstance();
     if (base::Optional<GURL> optional_url =

ios/chrome/browser/ui/popup_menu/BUILD.gn

@@ -40,7 +40,6 @@ source_set("popup_menu") {
     "//base",
     "//components/bookmarks/browser",
     "//components/feature_engagement/public",
-    "//components/omnibox/browser:browser",
     "//components/open_from_clipboard",
     "//ios/chrome/app/strings",
     "//ios/chrome/browser",
@@ -51,6 +50,7 @@ source_set("popup_menu") {
     "//ios/chrome/browser/reading_list",
     "//ios/chrome/browser/search_engines",
     "//ios/chrome/browser/ui",
+    "//ios/chrome/browser/ui:feature_flags",
     "//ios/chrome/browser/ui/activity_services",
     "//ios/chrome/browser/ui/bookmarks",
     "//ios/chrome/browser/ui/bubble",

ios/chrome/browser/ui/popup_menu/popup_menu_action_handler.mm

@@ -9,7 +9,6 @@
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/strings/sys_string_conversions.h"
-#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/open_from_clipboard/clipboard_recent_content.h"
 #import "ios/chrome/browser/ui/commands/application_commands.h"
 #import "ios/chrome/browser/ui/commands/browser_commands.h"
@@ -18,6 +17,7 @@
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_action_handler_commands.h"
 #import "ios/chrome/browser/ui/popup_menu/public/cells/popup_menu_item.h"
 #import "ios/chrome/browser/ui/popup_menu/public/popup_menu_table_view_controller.h"
+#import "ios/chrome/browser/ui/ui_feature_flags.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -132,7 +132,7 @@ - (void)popupMenuTableViewController:(PopupMenuTableViewController*)sender
     case PopupMenuActionPasteAndGo: {
       RecordAction(UserMetricsAction("MobileMenuPasteAndGo"));
       NSString* query;
-      if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+      if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
         ClipboardRecentContent* clipboardRecentContent =
             ClipboardRecentContent::GetInstance();
         if (base::Optional<GURL> optional_url =
@@ -157,7 +157,7 @@ - (void)popupMenuTableViewController:(PopupMenuTableViewController*)sender
       [self.dispatcher showQRScanner];
       break;
     case PopupMenuActionSearchCopiedImage: {
-      DCHECK(base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior));
+      DCHECK(base::FeatureList::IsEnabled(kCopiedContentBehavior));
       RecordAction(UserMetricsAction("MobileMenuSearchCopiedImage"));
       ClipboardRecentContent* clipboardRecentContent =
           ClipboardRecentContent::GetInstance();

ios/chrome/browser/ui/popup_menu/popup_menu_mediator.mm

@@ -10,7 +10,6 @@
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/feature_engagement/public/feature_constants.h"
 #include "components/feature_engagement/public/tracker.h"
-#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/open_from_clipboard/clipboard_recent_content.h"
 #import "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
@@ -28,6 +27,7 @@
 #import "ios/chrome/browser/ui/popup_menu/public/popup_menu_consumer.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_menu_notification_delegate.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_menu_notifier.h"
+#import "ios/chrome/browser/ui/ui_feature_flags.h"
 #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
@@ -589,7 +589,7 @@ - (void)createTabGridMenuItems {
 - (void)createSearchMenuItems {
   NSMutableArray* items = [NSMutableArray array];
 
-  if (base::FeatureList::IsEnabled(omnibox::kCopiedTextBehavior)) {
+  if (base::FeatureList::IsEnabled(kCopiedContentBehavior)) {
     ClipboardRecentContent* clipboardRecentContent =
         ClipboardRecentContent::GetInstance();
     PopupMenuToolsItem* copiedContentItem = nil;

ios/chrome/browser/ui/ui_feature_flags.cc

@@ -14,8 +14,8 @@ const base::Feature kBrowserContainerContainsNTP{
 const base::Feature kOmniboxPopupShortcutIconsInZeroState{
     "OmniboxPopupShortcutIconsInZeroState", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kSearchCopiedImage{"SearchCopiedImage",
-                                       base::FEATURE_DISABLED_BY_DEFAULT};
-
 const base::Feature kSnapshotDrawView{"SnapshotDrawView",
                                       base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kCopiedContentBehavior{"CopiedContentBehavior",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};

ios/chrome/browser/ui/ui_feature_flags.h

@@ -18,10 +18,10 @@ extern const base::Feature kBrowserContainerContainsNTP;
 // popup instead of ZeroSuggest.
 extern const base::Feature kOmniboxPopupShortcutIconsInZeroState;
 
-// Feature to allow user to search for a copied image.
-extern const base::Feature kSearchCopiedImage;
-
 // Feature to take snapshots using |-drawViewHierarchy:|.
 extern const base::Feature kSnapshotDrawView;
 
+// Feature to rework handling of copied content (url/string/image) in the ui.
+extern const base::Feature kCopiedContentBehavior;
+
 #endif  // IOS_CHROME_BROWSER_UI_UI_FEATURE_FLAGS_H_