From 926e0a4d0439b024780cfa00ad63382c5aa287ea Mon Sep 17 00:00:00 2001 From: Ali Stanfield Date: Thu, 1 Jul 2021 23:08:47 +0000 Subject: [PATCH] Add a new menu item to the context menu for Lens Region Search. The menu item will appear in its own group and display under all circumstances when the feature flag is enabled. For now, it will do nothing when clicked. Bug: 1222313 Change-Id: I7dbda04fb370527fa4f43934028e046bfeb8ff89 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2991961 Commit-Queue: Ali Stanfield Reviewed-by: Ben Goldberger Reviewed-by: Avi Drissman Cr-Commit-Position: refs/heads/master@{#897944} --- chrome/app/chrome_command_ids.h | 1 + chrome/app/generated_resources.grd | 6 ++++ ...ONTENT_CONTEXT_LENS_REGION_SEARCH.png.sha1 | 1 + .../context_menu_content_type_platform_app.cc | 1 + .../context_menu_content_type_unittest.cc | 14 ++++++++++ .../render_view_context_menu.cc | 28 +++++++++++++++++-- .../render_view_context_menu.h | 2 ++ .../render_view_context_menu_unittest.cc | 22 +++++++++++++++ chrome/test/BUILD.gn | 1 + components/lens/lens_features.cc | 4 ++- components/lens/lens_features.h | 8 ++++++ .../context_menu_content_type.cc | 3 ++ .../context_menu_content_type.h | 1 + 13 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 chrome/app/generated_resources_grd/IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH.png.sha1 diff --git a/chrome/app/chrome_command_ids.h b/chrome/app/chrome_command_ids.h index 11db51f968c27d..77da078caf07bd 100644 --- a/chrome/app/chrome_command_ids.h +++ b/chrome/app/chrome_command_ids.h @@ -330,6 +330,7 @@ #define IDC_CONTENT_CONTEXT_INSPECTBACKGROUNDPAGE 50161 #define IDC_CONTENT_CONTEXT_RELOAD_PACKAGED_APP 50162 #define IDC_CONTENT_CONTEXT_RESTART_PACKAGED_APP 50163 +#define IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH 50164 // A gap here. Feel free to insert new IDs. #define IDC_CONTENT_CONTEXT_GENERATEPASSWORD 50166 #define IDC_CONTENT_CONTEXT_EXIT_FULLSCREEN 50167 diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 865ae615a4d4a1..eec19d12c3b9a4 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -749,6 +749,9 @@ are declared in tools/grit/grit_rule.gni. Search image with Google Lens + + Search part of the page with Google Lens + &Go to $1http://www.google.com/ @@ -990,6 +993,9 @@ are declared in tools/grit/grit_rule.gni. Search Image with Google Lens + + Search Part of the Page with Google Lens + &Go to $1http://www.google.com/ diff --git a/chrome/app/generated_resources_grd/IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH.png.sha1 b/chrome/app/generated_resources_grd/IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH.png.sha1 new file mode 100644 index 00000000000000..a91e49b707dcf1 --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH.png.sha1 @@ -0,0 +1 @@ +9e611e69507be1a051ddbce6a1315ffd2891ca12 \ No newline at end of file diff --git a/chrome/browser/renderer_context_menu/context_menu_content_type_platform_app.cc b/chrome/browser/renderer_context_menu/context_menu_content_type_platform_app.cc index b44f9065c04bb4..424eee8cc53efa 100644 --- a/chrome/browser/renderer_context_menu/context_menu_content_type_platform_app.cc +++ b/chrome/browser/renderer_context_menu/context_menu_content_type_platform_app.cc @@ -45,6 +45,7 @@ bool ContextMenuContentTypePlatformApp::SupportsGroup(int group) { case ITEM_GROUP_COPY: return ContextMenuContentType::SupportsGroup(group); case ITEM_GROUP_CURRENT_EXTENSION: + case ITEM_GROUP_LENS_REGION_SEARCH: return true; case ITEM_GROUP_DEVTOOLS_UNPACKED_EXT: // Add dev tools for unpacked extensions. diff --git a/chrome/browser/renderer_context_menu/context_menu_content_type_unittest.cc b/chrome/browser/renderer_context_menu/context_menu_content_type_unittest.cc index 596fb48df5b6a3..134ed1754d152c 100644 --- a/chrome/browser/renderer_context_menu/context_menu_content_type_unittest.cc +++ b/chrome/browser/renderer_context_menu/context_menu_content_type_unittest.cc @@ -72,6 +72,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { ContextMenuContentType::ITEM_GROUP_LINK)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_ALL_EXTENSION)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); EXPECT_FALSE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_CURRENT_EXTENSION)); } @@ -88,6 +90,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { ContextMenuContentType::ITEM_GROUP_EDITABLE)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_SEARCH_PROVIDER)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); } { @@ -100,6 +104,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { ContextMenuContentType::ITEM_GROUP_COPY)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_EDITABLE)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); } { @@ -112,6 +118,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { ContextMenuContentType::ITEM_GROUP_SEARCHWEBFORIMAGE)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_PRINT)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); EXPECT_FALSE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_MEDIA_VIDEO)); @@ -127,6 +135,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { Create(web_contents(), params)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_MEDIA_VIDEO)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); EXPECT_FALSE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_MEDIA_IMAGE)); @@ -142,6 +152,8 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { Create(web_contents(), params)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_MEDIA_AUDIO)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); EXPECT_FALSE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_MEDIA_IMAGE)); @@ -159,5 +171,7 @@ TEST_F(ContextMenuContentTypeTest, CheckTypes) { ContextMenuContentType::ITEM_GROUP_FRAME)); EXPECT_TRUE(content_type->SupportsGroup( ContextMenuContentType::ITEM_GROUP_PAGE)); + EXPECT_TRUE(content_type->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)); } } diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc index fc7615ab0e5c03..67c33ece7b079d 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -389,13 +389,14 @@ const std::map& GetIdcToUmaMap(UmaEnumIdLookupType type) { {IDC_CONTENT_CONTEXT_COPYLINKTOTEXT, 112}, {IDC_CONTENT_CONTEXT_SEARCHLENSFORIMAGE, 113}, {IDC_CONTENT_CONTEXT_REMOVELINKTOTEXT, 114}, + {IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH, 115}, // To add new items: // - Add one more line above this comment block, using the UMA value // from the line below this comment block. // - Increment the UMA value in that latter line. // - Add the new item to the RenderViewContextMenuItem enum in // tools/metrics/histograms/enums.xml. - {0, 115}}); + {0, 116}}); // These UMA values are for the the ContextMenuOptionDesktop enum, used for // the ContextMenu.SelectedOptionDesktop histograms. @@ -422,13 +423,14 @@ const std::map& GetIdcToUmaMap(UmaEnumIdLookupType type) { {IDC_CONTENT_CONTEXT_GOTOURL, 19}, {IDC_CONTENT_CONTEXT_COPYLINKTOTEXT, 20}, {IDC_CONTENT_CONTEXT_SEARCHLENSFORIMAGE, 21}, + {IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH, 22}, // To add new items: // - Add one more line above this comment block, using the UMA value // from the line below this comment block. // - Increment the UMA value in that latter line. // - Add the new item to the ContextMenuOptionDesktop enum in // tools/metrics/histograms/enums.xml. - {0, 22}}); + {0, 23}}); return *(type == UmaEnumIdLookupType::GeneralEnumId ? kGeneralMap : kSpecificMap); @@ -946,6 +948,14 @@ void RenderViewContextMenu::InitMenu() { AppendCurrentExtensionItems(); } + if (content_type_->SupportsGroup( + ContextMenuContentType::ITEM_GROUP_LENS_REGION_SEARCH)) { + if (base::FeatureList::IsEnabled(lens::features::kLensRegionSearch)) { + menu_model_.AddSeparator(ui::NORMAL_SEPARATOR); + AppendLensRegionSearchItem(); + } + } + // Accessibility label items are appended to all menus when a screen reader // is enabled. It can be difficult to open a specific context menu with a // screen reader, so this is a UX approved solution. @@ -1969,6 +1979,11 @@ void RenderViewContextMenu::AppendSharedClipboardItem() { shared_clipboard_context_menu_observer_->InitMenu(params_); } +void RenderViewContextMenu::AppendLensRegionSearchItem() { + menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH, + IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH); +} + // Menu delegate functions ----------------------------------------------------- bool RenderViewContextMenu::IsCommandIdEnabled(int id) const { @@ -2163,6 +2178,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const { case IDC_CONTENT_CONTEXT_LANGUAGE_SETTINGS: case IDC_SEND_TAB_TO_SELF: case IDC_SEND_TAB_TO_SELF_SINGLE_TARGET: + case IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH: return true; case IDC_CONTENT_CONTEXT_GENERATE_QR_CODE: @@ -2348,6 +2364,10 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { ExecSearchLensForImage(); break; + case IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH: + ExecLensRegionSearch(); + break; + case IDC_CONTENT_CONTEXT_OPEN_ORIGINAL_IMAGE_NEW_TAB: OpenURLWithExtraHeaders(params_.src_url, GetDocumentURL(params_), WindowOpenDisposition::NEW_BACKGROUND_TAB, @@ -3120,6 +3140,10 @@ void RenderViewContextMenu::ExecSearchLensForImage() { core_tab_helper->SearchWithLensInNewTab(render_frame_host, params().src_url); } +void RenderViewContextMenu::ExecLensRegionSearch() { + // TODO(crbug.com/1222313): Add click and drag image selection functionality. +} + void RenderViewContextMenu::ExecSearchWebForImage() { CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(source_web_contents_); diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.h b/chrome/browser/renderer_context_menu/render_view_context_menu.h index fe38d65dcd7301..45aee4dcd8e53e 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu.h +++ b/chrome/browser/renderer_context_menu/render_view_context_menu.h @@ -206,6 +206,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase { void AppendSharingItems(); void AppendClickToCallItem(); void AppendSharedClipboardItem(); + void AppendLensRegionSearchItem(); void AppendQRCodeGeneratorItem(bool for_image, bool draw_icon); std::unique_ptr CreateDataEndpoint( @@ -240,6 +241,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase { void ExecCopyLinkText(); void ExecCopyImageAt(); void ExecSearchLensForImage(); + void ExecLensRegionSearch(); void ExecSearchWebForImage(); void ExecLoadImage(); void ExecPlayPause(); diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc b/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc index 0b576181007812..42a5ccd9422ebd 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc +++ b/chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc @@ -7,6 +7,7 @@ #include "base/bind.h" #include "base/run_loop.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/scoped_feature_list.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/custom_handlers/protocol_handler_registry.h" @@ -25,6 +26,7 @@ #include "chrome/test/base/testing_profile.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h" #include "components/policy/core/common/policy_pref_names.h" +#include "components/lens/lens_features.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" #include "components/proxy_config/proxy_config_pref_names.h" @@ -686,6 +688,26 @@ TEST_F(RenderViewContextMenuPrefsTest, ShowAllPasswordsIncognito) { EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_SHOWALLSAVEDPASSWORDS)); } +// Verify that the Lens Region Search menu item is displayed when the feature +// is enabled. +TEST_F(RenderViewContextMenuPrefsTest, LensRegionSearch) { + base::test::ScopedFeatureList features; + features.InitAndEnableFeature(lens::features::kLensRegionSearch); + std::unique_ptr menu(CreateContextMenu()); + + EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH)); +} + +// Verify that the Lens Region Search menu item is disabled when the feature +// is disabled. +TEST_F(RenderViewContextMenuPrefsTest, LensRegionSearchDisabled) { + base::test::ScopedFeatureList features; + features.InitAndDisableFeature(lens::features::kLensRegionSearch); + std::unique_ptr menu(CreateContextMenu()); + + EXPECT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH)); +} + // Test FormatUrlForClipboard behavior // ------------------------------------------- diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 32b5994b96f11f..365aeb4ad7868c 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -5039,6 +5039,7 @@ test("unit_tests") { "//components/invalidation/impl", "//components/invalidation/impl:test_support", "//components/language/core/browser", + "//components/lens", "//components/leveldb_proto:test_support", "//components/live_caption:constants", "//components/lookalikes/core:features", diff --git a/components/lens/lens_features.cc b/components/lens/lens_features.cc index ccbe505ff8bc53..3589c5c5d4454c 100644 --- a/components/lens/lens_features.cc +++ b/components/lens/lens_features.cc @@ -10,10 +10,12 @@ namespace lens { namespace features { -// Enables context menu search by image sending to lens.google.com. const base::Feature kLensStandalone{"LensStandalone", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kLensRegionSearch{"LensRegionSearch", + base::FEATURE_DISABLED_BY_DEFAULT}; + constexpr base::FeatureParam kMaxPixels{&kLensStandalone, "dimensions-max-pixels", 1000}; diff --git a/components/lens/lens_features.h b/components/lens/lens_features.h index 5dc323894ed58d..d44e5557829d31 100644 --- a/components/lens/lens_features.h +++ b/components/lens/lens_features.h @@ -12,8 +12,16 @@ namespace lens { namespace features { +// Enables context menu search by image sending to the Lens homepage. extern const base::Feature kLensStandalone; + +// Enables Lens Region Search from the context menu. +extern const base::Feature kLensRegionSearch; + +// Returns the max pixel width/height for the image to be sent to Lens. extern int GetMaxPixels(); + +// The URL for the Lens home page. extern std::string GetHomepageURL(); } // namespace features diff --git a/components/renderer_context_menu/context_menu_content_type.cc b/components/renderer_context_menu/context_menu_content_type.cc index 08a46044b06887..9337643642ca44 100644 --- a/components/renderer_context_menu/context_menu_content_type.cc +++ b/components/renderer_context_menu/context_menu_content_type.cc @@ -159,6 +159,9 @@ bool ContextMenuContentType::SupportsGroupInternal(int group) { return params_.input_field_type == blink::mojom::ContextMenuDataInputFieldType::kPassword; + case ITEM_GROUP_LENS_REGION_SEARCH: + return true; + default: NOTREACHED(); return false; diff --git a/components/renderer_context_menu/context_menu_content_type.h b/components/renderer_context_menu/context_menu_content_type.h index 9b8581fe38df27..c3f4579d0025ad 100644 --- a/components/renderer_context_menu/context_menu_content_type.h +++ b/components/renderer_context_menu/context_menu_content_type.h @@ -30,6 +30,7 @@ class ContextMenuContentType { ITEM_GROUP_LINK, ITEM_GROUP_SMART_SELECTION, ITEM_GROUP_MEDIA_IMAGE, + ITEM_GROUP_LENS_REGION_SEARCH, ITEM_GROUP_SEARCHLENSFORIMAGE, ITEM_GROUP_SEARCHWEBFORIMAGE, ITEM_GROUP_MEDIA_VIDEO,