The UI part of search-by-image search.

chrome/browser/tab_contents/render_view_context_menu.cc

Index: chrome/browser/tab_contents/render_view_context_menu.cc
===================================================================
--- chrome/browser/tab_contents/render_view_context_menu.cc	(revision 214769)
+++ chrome/browser/tab_contents/render_view_context_menu.cc	(working copy)
@@ -16,6 +16,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -42,6 +43,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/search/search.h"
+#include "chrome/browser/search_engines/search_terms_data.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -65,6 +67,7 @@
 #include "chrome/common/net/url_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/print_messages.h"
+#include "chrome/common/render_messages.h"
 #include "chrome/common/spellcheck_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/child_process_security_policy.h"
@@ -92,6 +95,8 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/favicon_size.h"
+#include "ui/gfx/point.h"
+#include "ui/gfx/size.h"
 
 using WebKit::WebContextMenuData;
 using WebKit::WebMediaPlayerAction;
@@ -115,6 +120,10 @@
 
 namespace {
 
+const int kImageSearchThumbnailMinSize = 300 * 300;
+const int kImageSearchThumbnailMaxWidth = 600;
+const int kImageSearchThumbnailMaxHeight = 600;

[Kibeom Kim (inactive), 2013/07/31 21:25:54]

Actually, Android Chrome need to access these values from the outside. The call
stack of Android popup goes like:

1. content::WebContentsImpl::ShowContextMenu(...)
content/browser/web_contents/web_contents_impl.cc

2. content::WebContentsImpl::ShowContextMenu(...)
content/browser/web_contents/web_contents_view_android.cc

3. content::WebContentsViewAndroid::ShowContextMenu(...)
chrome/browser/ui/android/tab_contents/chrome_web_contents_view_delegate_android.cc

4.TabAndroidImpl::ShowContextMenu(...)
clank/native/framework/chrome/tab_android_impl.cc //private

5. // Android Shows context menu

6. TabAndroidImpl::SearchByImage(...)
clank/native/framework/chrome/tab_android_impl.cc //private

7. call:  web_contents()->GetRenderViewHost()->Send(new
ChromeViewMsg_RequestThumbnailForContextNode(.....))

+
 // Maps UMA enumeration to IDC. IDC could be changed so we can't use
 // just them and |UMA_HISTOGRAM_CUSTOM_ENUMERATION|.
 // Never change mapping or reuse |enum_id|. Always push back new items.
@@ -181,8 +190,9 @@
   { 54, IDC_SPELLCHECK_MENU },
   { 55, IDC_CONTENT_CONTEXT_SPELLING_TOGGLE },
   { 56, IDC_SPELLCHECK_LANGUAGES_FIRST },
+  {57, IDC_CONTENT_CONTEXT_SEARCHIMAGENEWTAB },

[Avi (use Gerrit), 2013/07/31 20:11:05]

Match style; space after {

[Johnny(Jianning) Ding, 2013/07/31 21:38:09]

Done.

   // Add new items here and use |enum_id| from the next line.
-  { 57, 0 },  // Must be the last. Increment |enum_id| when new IDC was added.
+  { 58, 0 },  // Must be the last. Increment |enum_id| when new IDC was added.
 };
 
 // Collapses large ranges of ids before looking for UMA enum.
@@ -843,6 +853,21 @@
   menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB,
                                   IDS_CONTENT_CONTEXT_OPENIMAGENEWTAB);
   AppendPrintItem();
+  const TemplateURL* const default_provider =
+      TemplateURLServiceFactory::GetForProfile(profile_)->
+      GetDefaultSearchProvider();
+  if (!default_provider)
+    return;
+  SearchTermsData search_terms;
+  if (!default_provider->image_url().empty() &&
+      default_provider->image_url_ref().IsValidUsingTermsData(search_terms)) {
+    std::string search_provider_name =
+        UTF16ToUTF8(default_provider->short_name());
+    std::string menu_item_name =
+        base::StringPrintf("Search Image By %s", search_provider_name.c_str());

[jingbinw, 2013/07/31 20:04:45]

I would suggest to rename this as

"Search %s for this image"

just to keep consistent with the existing 

"Search Google for 'highlighted text'" function in the context menu.

Also, is there any i18n support for translating such messages?

[Avi (use Gerrit), 2013/07/31 20:11:05]

Um...

Every one of our UI strings is localized and translated. How is this different?

[Kibeom Kim (inactive), 2013/07/31 20:20:53]

note: just like IDS_CONTENT_CONTEXT_SEARCHWEBFOR in generated_resources.grd

[Avi (use Gerrit), 2013/07/31 20:35:53]

Yes. Look for examples (in this very file) with l10n_util::GetStringFUTF16,
where that retrieves a localized string and substitutes in a variable.

[Johnny(Jianning) Ding, 2013/07/31 21:26:36]

The hard-code "Search Image By %s" should be replaced by the translated message,
but I am not familiar with this part. Please guide me how to change this part.

[jingbinw, 2013/07/31 23:31:29]

Can you rewording the message to be "Search %s for this image"?

BTW: In terms of translated message, I think what you did is already correct
given Avi's last comment.

On 2013/07/31 21:26:36, Johnny(Jianning) Ding wrote:

[Peter Kasting, 2013/07/31 23:38:22]

No, it's not correct.  You should never see a .cc file with a string in it that
gets displayed to a user.  All strings should be handled by the l10n framework.

Look at line 952 for an example of how to do this correctly.

[Kibeom Kim (inactive), 2013/08/01 00:14:59]

here:
menu_model_.AddItem(
    IDC_CONTENT_CONTEXT_SEARCHIMAGENEWTAB,
    l10n_util::GetStringFUTF16(IDS_CONTENT_CONTEXT_SEARCHWEBFORIMAGE,
                               default_provider->short_name()));

in generated_resources.grd, near the two IDS_CONTENT_CONTEXT_SEARCHWEBFOR s:

<message name="IDS_CONTENT_CONTEXT_SEARCHWEBFORIMAGE" desc="The name of the
Search the Web for the image in the content area context menu">
  &amp;Search image by <ph name="SEARCH_ENGINE">$1<ex>Google</ex></ph>
</message>

and

<message name="IDS_CONTENT_CONTEXT_SEARCHWEBFORIMAGE" desc="The name of the
Search the Web for the image in the content area context menu">
  &amp;Search Image by <ph name="SEARCH_ENGINE">$1<ex>Google</ex></ph>
</message>

+    menu_model_.AddItem(IDC_CONTENT_CONTEXT_SEARCHIMAGENEWTAB,
+                        UTF8ToUTF16(menu_item_name));
+  }
 }
 
 void RenderViewContextMenu::AppendAudioItems() {
@@ -1253,6 +1278,7 @@
     }
 
     case IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB:
+    case IDC_CONTENT_CONTEXT_SEARCHIMAGENEWTAB:
       // The images shown in the most visited thumbnails do not currently open
       // in a new tab as they should. Disabling this context menu option for
       // now, as a quick hack, before we resolve this issue (Issue = 2608).
@@ -1606,6 +1632,10 @@
       CopyImageAt(params_.x, params_.y);
       break;
 
+    case IDC_CONTENT_CONTEXT_SEARCHIMAGENEWTAB:
+      GetImageThumbnailForSearch(params_.x, params_.y);
+      break;
+
     case IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB:
     case IDC_CONTENT_CONTEXT_OPENAVNEWTAB:
       OpenURL(
@@ -2034,6 +2064,15 @@
   source_web_contents_->GetRenderViewHost()->CopyImageAt(x, y);
 }
 
+void RenderViewContextMenu::GetImageThumbnailForSearch(int x, int y) {

[Kibeom Kim (inactive), 2013/07/31 21:49:49]

How x and y are used?

[Johnny(Jianning) Ding, 2013/07/31 21:53:47]

they are gone.

+  source_web_contents_->GetRenderViewHost()->Send(
+       new ChromeViewMsg_RequestThumbnailForContextNode(
+           source_web_contents_->GetRenderViewHost()->GetRoutingID(),
+           kImageSearchThumbnailMinSize,
+           gfx::Size(kImageSearchThumbnailMaxWidth,
+                     kImageSearchThumbnailMaxHeight)));
+}
+
 void RenderViewContextMenu::Inspect(int x, int y) {
   content::RecordAction(UserMetricsAction("DevTools_InspectElement"));
   source_web_contents_->GetRenderViewHostAtPosition(