For extension context menus, only apply targetUrlPatterns to appropriate contexts.

BUG=63545
TEST=Use contextMenus.create() with "page" context and targetUrlPattern; verify that menu does show.


Review URL: http://codereview.chromium.org/7057029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86628 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/tab_contents/render_view_context_menu.cc

@@ -204,29 +204,51 @@ void RenderViewContextMenu::Init() {
   PlatformInit();
 }
 
-static bool ExtensionContextMatch(const ContextMenuParams& params,
-                                  ExtensionMenuItem::ContextList contexts) {
+static bool ExtensionPatternMatch(const URLPatternSet& patterns,
+                                  const GURL& url) {
+  // No patterns means no restriction, so that implicitly matches.
+  if (patterns.is_empty())
+    return true;
+  return patterns.MatchesURL(url);
+}
+
+// static
+bool RenderViewContextMenu::ExtensionContextAndPatternMatch(
+    const ContextMenuParams& params,
+    ExtensionMenuItem::ContextList contexts,
+    const URLPatternSet& target_url_patterns) {
   bool has_link = !params.link_url.is_empty();
   bool has_selection = !params.selection_text.empty();
   bool in_frame = !params.frame_url.is_empty();
 
   if (contexts.Contains(ExtensionMenuItem::ALL) ||
       (has_selection && contexts.Contains(ExtensionMenuItem::SELECTION)) ||
-      (has_link && contexts.Contains(ExtensionMenuItem::LINK)) ||
       (params.is_editable && contexts.Contains(ExtensionMenuItem::EDITABLE)) ||
-      (in_frame && contexts.Contains(ExtensionMenuItem::FRAME))) {
+      (in_frame && contexts.Contains(ExtensionMenuItem::FRAME)))
+    return true;
+
+  if (has_link && contexts.Contains(ExtensionMenuItem::LINK) &&
+      ExtensionPatternMatch(target_url_patterns, params.link_url))
     return true;
-  }
 
   switch (params.media_type) {
     case WebContextMenuData::MediaTypeImage:
-      return contexts.Contains(ExtensionMenuItem::IMAGE);
+      if (contexts.Contains(ExtensionMenuItem::IMAGE) &&
+          ExtensionPatternMatch(target_url_patterns, params.src_url))
+        return true;
+      break;
 
     case WebContextMenuData::MediaTypeVideo:
-      return contexts.Contains(ExtensionMenuItem::VIDEO);
+      if (contexts.Contains(ExtensionMenuItem::VIDEO) &&
+          ExtensionPatternMatch(target_url_patterns, params.src_url))
+        return true;
+      break;
 
     case WebContextMenuData::MediaTypeAudio:
-      return contexts.Contains(ExtensionMenuItem::AUDIO);
+      if (contexts.Contains(ExtensionMenuItem::AUDIO) &&
+          ExtensionPatternMatch(target_url_patterns, params.src_url))
+        return true;
+      break;
 
     default:
       break;
@@ -243,21 +265,14 @@ static bool ExtensionContextMatch(const ContextMenuParams& params,
   return false;
 }
 
-static bool ExtensionPatternMatch(const URLPatternSet& patterns,
-                                  const GURL& url) {
-  // No patterns means no restriction, so that implicitly matches.
-  if (patterns.is_empty())
-    return true;
-  return patterns.MatchesURL(url);
-}
-
 static const GURL& GetDocumentURL(const ContextMenuParams& params) {
   return params.frame_url.is_empty() ? params.page_url : params.frame_url;
 }
 
 // Given a list of items, returns the ones that match given the contents
 // of |params| and the profile.
-static ExtensionMenuItem::List GetRelevantExtensionItems(
+// static
+ExtensionMenuItem::List RenderViewContextMenu::GetRelevantExtensionItems(
     const ExtensionMenuItem::List& items,
     const ContextMenuParams& params,
     Profile* profile,
@@ -267,18 +282,14 @@ static ExtensionMenuItem::List GetRelevantExtensionItems(
        i != items.end(); ++i) {
     const ExtensionMenuItem* item = *i;
 
-    if (!ExtensionContextMatch(params, item->contexts()))
+    if (!ExtensionContextAndPatternMatch(params, item->contexts(),
+        item->target_url_patterns()))
       continue;
 
     const GURL& document_url = GetDocumentURL(params);
     if (!ExtensionPatternMatch(item->document_url_patterns(), document_url))
       continue;
 
-    const GURL& target_url =
-        params.src_url.is_empty() ? params.link_url : params.src_url;
-    if (!ExtensionPatternMatch(item->target_url_patterns(), target_url))
-      continue;
-
     if (item->id().profile == profile || can_cross_incognito)
       result.push_back(*i);
   }

chrome/browser/tab_contents/render_view_context_menu.h

@@ -78,8 +78,19 @@ class RenderViewContextMenu : public ui::SimpleMenuModel::Delegate {
   std::map<int, ExtensionMenuItem::Id> extension_item_map_;
 
  private:
+  friend class RenderViewContextMenuTest;
+
   static bool IsDevToolsURL(const GURL& url);
   static bool IsInternalResourcesURL(const GURL& url);
+  static bool ExtensionContextAndPatternMatch(
+      const ContextMenuParams& params,
+      ExtensionMenuItem::ContextList contexts,
+      const URLPatternSet& target_url_patterns);
+  static ExtensionMenuItem::List GetRelevantExtensionItems(
+      const ExtensionMenuItem::List& items,
+      const ContextMenuParams& params,
+      Profile* profile,
+      bool can_cross_incognito);
   bool AppendCustomItems();
   void AppendDeveloperItems();
   void AppendLinkItems();

chrome/browser/tab_contents/render_view_context_menu_unittest.cc

+// Copyright (c) 2011 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.
+
+#include <string>
+
+#include "chrome/browser/tab_contents/render_view_context_menu.h"
+
+#include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/common/extensions/url_pattern.h"
+#include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/glue/context_menu.h"
+
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebContextMenuData.h"
+
+class RenderViewContextMenuTest : public testing::Test {
+ public:
+  RenderViewContextMenuTest() { }
+
+ protected:
+  // Proxy defined here to minimize friend classes in RenderViewContextMenu
+  static bool ExtensionContextAndPatternMatch(
+      const ContextMenuParams& params,
+      ExtensionMenuItem::ContextList contexts,
+      const URLPatternSet& patterns) {
+    return RenderViewContextMenu::ExtensionContextAndPatternMatch(params,
+        contexts, patterns);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RenderViewContextMenuTest);
+};
+
+// Generates a ContextMenuParams that matches the specified contexts.
+static ContextMenuParams CreateParams(int contexts) {
+  ContextMenuParams rv;
+  rv.is_editable = false;
+  rv.media_type = WebKit::WebContextMenuData::MediaTypeNone;
+  rv.page_url = GURL("http://test.page/");
+
+  static const char16 selected_text[] = { 's', 'e', 'l', 0 };
+  if (contexts & ExtensionMenuItem::SELECTION)
+    rv.selection_text = selected_text;
+
+  if (contexts & ExtensionMenuItem::LINK)
+    rv.link_url = GURL("http://test.link/");
+
+  if (contexts & ExtensionMenuItem::EDITABLE)
+    rv.is_editable = true;
+
+  if (contexts & ExtensionMenuItem::IMAGE) {
+    rv.src_url = GURL("http://test.image/");
+    rv.media_type = WebKit::WebContextMenuData::MediaTypeImage;
+  }
+
+  if (contexts & ExtensionMenuItem::VIDEO) {
+    rv.src_url = GURL("http://test.video/");
+    rv.media_type = WebKit::WebContextMenuData::MediaTypeVideo;
+  }
+
+  if (contexts & ExtensionMenuItem::AUDIO) {
+    rv.src_url = GURL("http://test.audio/");
+    rv.media_type = WebKit::WebContextMenuData::MediaTypeAudio;
+  }
+
+  if (contexts & ExtensionMenuItem::FRAME)
+    rv.frame_url = GURL("http://test.frame/");
+
+  return rv;
+}
+
+// Generates a URLPatternSet with a single pattern
+static URLPatternSet CreatePatternSet(const std::string& pattern) {
+  URLPattern target(URLPattern::SCHEME_HTTP);
+  target.Parse(pattern, URLPattern::PARSE_LENIENT);
+
+  URLPatternSet rv;
+  rv.AddPattern(target);
+
+  return rv;
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForPage) {
+  ContextMenuParams params = CreateParams(0);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::PAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetCheckedForLink) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::LINK);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::PAGE);
+  contexts.Add(ExtensionMenuItem::LINK);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_FALSE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetCheckedForImage) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::IMAGE);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::PAGE);
+  contexts.Add(ExtensionMenuItem::IMAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_FALSE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetCheckedForVideo) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::VIDEO);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::PAGE);
+  contexts.Add(ExtensionMenuItem::VIDEO);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_FALSE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetCheckedForAudio) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::AUDIO);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::PAGE);
+  contexts.Add(ExtensionMenuItem::AUDIO);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_FALSE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, MatchWhenLinkedImageMatchesTarget) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::IMAGE |
+                                          ExtensionMenuItem::LINK);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::LINK);
+  contexts.Add(ExtensionMenuItem::IMAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.link/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, MatchWhenLinkedImageMatchesSource) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::IMAGE |
+                                          ExtensionMenuItem::LINK);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::LINK);
+  contexts.Add(ExtensionMenuItem::IMAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.image/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, NoMatchWhenLinkedImageMatchesNeither) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::IMAGE |
+                                          ExtensionMenuItem::LINK);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::LINK);
+  contexts.Add(ExtensionMenuItem::IMAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_FALSE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForFrame) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::FRAME);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::FRAME);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForEditable) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::EDITABLE);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::EDITABLE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForSelection) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::SELECTION);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::SELECTION);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForSelectionOnLink) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::SELECTION |
+                                          ExtensionMenuItem::LINK);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::SELECTION);
+  contexts.Add(ExtensionMenuItem::LINK);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+
+TEST_F(RenderViewContextMenuTest, TargetIgnoredForSelectionOnImage) {
+  ContextMenuParams params = CreateParams(ExtensionMenuItem::SELECTION |
+                                          ExtensionMenuItem::IMAGE);
+
+  ExtensionMenuItem::ContextList contexts;
+  contexts.Add(ExtensionMenuItem::SELECTION);
+  contexts.Add(ExtensionMenuItem::IMAGE);
+
+  URLPatternSet patterns = CreatePatternSet("*://test.none/*");
+
+  EXPECT_TRUE(ExtensionContextAndPatternMatch(params, contexts, patterns));
+}
+

chrome/chrome_tests.gypi

@@ -1611,6 +1611,7 @@
         'browser/sync/test_profile_sync_service.h',
         'browser/sync/util/cryptographer_unittest.cc',
         'browser/sync/util/nigori_unittest.cc',
+        'browser/tab_contents/render_view_context_menu_unittest.cc',
         'browser/tab_contents/thumbnail_generator_unittest.cc',
         'browser/tab_contents/web_contents_unittest.cc',
         'browser/tabs/pinned_tab_codec_unittest.cc',

chrome/test/data/extensions/context_menus/target_urls/test.js

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
 window.onload = function() {
   var patterns = [ "http://*.google.com/*" ];
-  chrome.contextMenus.create({"title":"item1", "contexts": ["all"],
+  chrome.contextMenus.create({"title":"item1", "contexts": ["link"],
                               "targetUrlPatterns": patterns}, function() {
     if (!chrome.extension.lastError) {
       chrome.test.sendMessage("created items");