Commit 4f917f87 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

Use wildcard matching algorithm for URL matching in previews

Currently, in previews, we match the document URL with the the
provided hints using substring matching. This CL changes the
matching algorithm to a wildcard matcher instead of substring
matcher.

Change-Id: I3cf827d094ee5e15a64737d9691743b29d4c0bf3
Bug: 870039
Reviewed-on: https://chromium-review.googlesource.com/1176594Reviewed-by: default avatarRyan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584083}
parent ce33a6ad
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "components/optimization_guide/optimization_guide_service_observer.h" #include "components/optimization_guide/optimization_guide_service_observer.h"
#include "components/optimization_guide/url_pattern_with_wildcards.h"
#include "components/previews/core/previews_features.h" #include "components/previews/core/previews_features.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -235,11 +236,15 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig( ...@@ -235,11 +236,15 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
const optimization_guide::proto::PageHint* PreviewsHints::FindPageHint( const optimization_guide::proto::PageHint* PreviewsHints::FindPageHint(
const GURL& document_url, const GURL& document_url,
const optimization_guide::proto::Hint& hint) { const optimization_guide::proto::Hint& hint) {
if (hint.page_hints_size() == 0)
return nullptr;
std::string url = document_url.spec(); std::string url = document_url.spec();
for (const auto& page_hint : hint.page_hints()) { for (const auto& page_hint : hint.page_hints()) {
// TODO(dougarnett): Support wildcards. https://crbug.com/870039 if (page_hint.page_pattern().empty())
if (!page_hint.page_pattern().empty() && continue;
url.find(page_hint.page_pattern()) != std::string::npos) { optimization_guide::URLPatternWithWildcards url_pattern(
page_hint.page_pattern());
if (url_pattern.Matches(url)) {
// Return the first matching page hint. // Return the first matching page hint.
return &page_hint; return &page_hint;
} }
......
...@@ -18,7 +18,7 @@ TEST(PreviewsHintsTest, FindPageHintForSubstringPagePattern) { ...@@ -18,7 +18,7 @@ TEST(PreviewsHintsTest, FindPageHintForSubstringPagePattern) {
// Page hint for "/one/" // Page hint for "/one/"
optimization_guide::proto::PageHint* page_hint1 = hint1.add_page_hints(); optimization_guide::proto::PageHint* page_hint1 = hint1.add_page_hints();
page_hint1->set_page_pattern("/one/"); page_hint1->set_page_pattern("foo.org/*/one/");
// Page hint for "two" // Page hint for "two"
optimization_guide::proto::PageHint* page_hint2 = hint1.add_page_hints(); optimization_guide::proto::PageHint* page_hint2 = hint1.add_page_hints();
...@@ -34,17 +34,25 @@ TEST(PreviewsHintsTest, FindPageHintForSubstringPagePattern) { ...@@ -34,17 +34,25 @@ TEST(PreviewsHintsTest, FindPageHintForSubstringPagePattern) {
EXPECT_EQ(nullptr, PreviewsHints::FindPageHint( EXPECT_EQ(nullptr, PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one"), hint1)); GURL("https://www.foo.org/one"), hint1));
EXPECT_EQ(nullptr, PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one/"), hint1));
EXPECT_EQ(page_hint1, PreviewsHints::FindPageHint( EXPECT_EQ(page_hint1, PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one/"), hint1)); GURL("https://www.foo.org/pages/one/"), hint1));
EXPECT_EQ(page_hint1,
PreviewsHints::FindPageHint(
GURL("https://www.foo.org/pages/subpages/one/"), hint1));
EXPECT_EQ(page_hint1, PreviewsHints::FindPageHint( EXPECT_EQ(page_hint1, PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one/two"), hint1)); GURL("https://www.foo.org/pages/one/two"), hint1));
EXPECT_EQ(page_hint1, EXPECT_EQ(page_hint1,
PreviewsHints::FindPageHint( PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one/two/three.jpg"), hint1)); GURL("https://www.foo.org/pages/one/two/three.jpg"), hint1));
EXPECT_EQ(page_hint2, EXPECT_EQ(page_hint2,
PreviewsHints::FindPageHint( PreviewsHints::FindPageHint(
GURL("https://www.foo.org/onetwo/three.jpg"), hint1)); GURL("https://www.foo.org/pages/onetwo/three.jpg"), hint1));
EXPECT_EQ(page_hint2,
PreviewsHints::FindPageHint(
GURL("https://www.foo.org/one/two/three.jpg"), hint1));
EXPECT_EQ(page_hint2, EXPECT_EQ(page_hint2,
PreviewsHints::FindPageHint(GURL("https://one.two.org"), hint1)); PreviewsHints::FindPageHint(GURL("https://one.two.org"), hint1));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment