Skip to content

Commit

Permalink
Add a new menu item to the context menu for Lens Region Search.
Browse files Browse the repository at this point in the history
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 <stanfield@google.com>
Reviewed-by: Ben Goldberger <benwgold@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#897944}
  • Loading branch information
astanfield authored and Chromium LUCI CQ committed Jul 1, 2021
1 parent a0f6de4 commit 926e0a4
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 3 deletions.
1 change: 1 addition & 0 deletions chrome/app/chrome_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,9 @@ are declared in tools/grit/grit_rule.gni.
<message name="IDS_CONTENT_CONTEXT_SEARCHLENSFORIMAGE" desc="The name of the Search Lens for Image command in the content area context menu">
Search image with Google Lens
</message>
<message name="IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH" desc="The name of the Lens Region Search command in the content area context menu">
Search part of the page with Google Lens
</message>
<message name="IDS_CONTENT_CONTEXT_GOTOURL" desc="The name of the Go to 'url' command in the content area context menu">
&amp;Go to <ph name="URL">$1<ex>http://www.google.com/</ex></ph>
</message>
Expand Down Expand Up @@ -990,6 +993,9 @@ are declared in tools/grit/grit_rule.gni.
<message name="IDS_CONTENT_CONTEXT_SEARCHLENSFORIMAGE" desc="In Title Case: The name of the Search Lens for Image command in the content area context menu">
Search Image with Google Lens
</message>
<message name="IDS_CONTENT_CONTEXT_LENS_REGION_SEARCH" desc="In Title Case: The name of the Lens Region Search command in the content area context menu">
Search Part of the Page with Google Lens
</message>
<message name="IDS_CONTENT_CONTEXT_GOTOURL" desc="In Title Case: The name of the Go to url for 'string' command in the content area context menu">
&amp;Go to <ph name="URL">$1<ex>http://www.google.com/</ex></ph>
</message>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9e611e69507be1a051ddbce6a1315ffd2891ca12
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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));
}

{
Expand All @@ -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));
}

{
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
}
}
28 changes: 26 additions & 2 deletions chrome/browser/renderer_context_menu/render_view_context_menu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,14 @@ const std::map<int, int>& 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.
Expand All @@ -422,13 +423,14 @@ const std::map<int, int>& 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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ui::DataTransferEndpoint> CreateDataEndpoint(
Expand Down Expand Up @@ -240,6 +241,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase {
void ExecCopyLinkText();
void ExecCopyImageAt();
void ExecSearchLensForImage();
void ExecLensRegionSearch();
void ExecSearchWebForImage();
void ExecLoadImage();
void ExecPlayPause();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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<TestRenderViewContextMenu> 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<TestRenderViewContextMenu> menu(CreateContextMenu());

EXPECT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_LENS_REGION_SEARCH));
}

// Test FormatUrlForClipboard behavior
// -------------------------------------------

Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion components/lens/lens_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> kMaxPixels{&kLensStandalone,
"dimensions-max-pixels", 1000};

Expand Down
8 changes: 8 additions & 0 deletions components/lens/lens_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions components/renderer_context_menu/context_menu_content_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 926e0a4

Please sign in to comment.