[Feed] Use requested URLs when fetching offline status.

Offline pages uses some non-exact url matching, such as removing
fragments and matching redirects. The host communicates with the Feed
only through exact matches. The change switches the get status path to
full use exact requested URL matches.

Unfortunately observations from OfflinePagesModel still must trust the
offline pages URL, but this is not easily fixable.

Bug: 888770
Change-Id: I9b6d7734547af149d2e0e5af28aea071b4f18a36
Reviewed-on: https://chromium-review.googlesource.com/c/1278933Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599994}

components/feed/content/feed_offline_host.cc

@@ -46,15 +46,19 @@ class CallbackAggregator : public base::RefCounted<CallbackAggregator> {
         on_each_result_(std::move(on_each_result)),
         start_time_(base::Time::Now()) {}
 
-  void OnGetPages(const std::vector<OfflinePageItem>& pages) {
+  // We curry |feed_url|, which is the URL the Feed requested with. This is used
+  // instead of the URLs present in the |pages| because offline pages has
+  // non-exact URL matching, and we must communicate with the Feed with exact
+  // matches.
+  void OnGetPages(std::string feed_url,
+                  const std::vector<OfflinePageItem>& pages) {
     if (!pages.empty()) {
       OfflinePageItem newest =
           *std::max_element(pages.begin(), pages.end(), [](auto lhs, auto rhs) {
             return lhs.creation_time < rhs.creation_time;
           });
-      const std::string& url = PreferOriginal(newest).spec();
-      urls_.push_back(url);
-      on_each_result_.Run(url, newest.offline_id);
+      urls_.push_back(feed_url);
+      on_each_result_.Run(std::move(feed_url), newest.offline_id);
     }
   }
 
@@ -172,8 +176,10 @@ void FeedOfflineHost::GetOfflineStatus(
                               weak_factory_.GetWeakPtr()));
 
   for (std::string url : urls) {
+    GURL gurl(url);
     offline_page_model_->GetPagesByURL(
-        GURL(url), base::BindOnce(&CallbackAggregator::OnGetPages, aggregator));
+        gurl, base::BindOnce(&CallbackAggregator::OnGetPages, aggregator,
+                             std::move(url)));
   }
 }
 

components/feed/content/feed_offline_host_unittest.cc

@@ -245,6 +245,19 @@ TEST_F(FeedOfflineHostTest, GetOfflineIdOriginalUrl) {
   EXPECT_EQ(host()->GetOfflineId(kUrl2).value(), 4);
 }
 
+TEST_F(FeedOfflineHostTest, GetOfflineIdRequestUrl) {
+  offline_page_model()->AddOfflinedPage(kUrl2, kUrl1, 4, base::Time());
+
+  std::vector<std::string> actual;
+  host()->GetOfflineStatus({kUrl2}, base::BindOnce(&CopyStatus, &actual));
+  RunUntilIdle();
+
+  EXPECT_EQ(1U, actual.size());
+  EXPECT_EQ(kUrl2, actual[0]);
+  EXPECT_FALSE(host()->GetOfflineId(kUrl1).has_value());
+  EXPECT_EQ(host()->GetOfflineId(kUrl2).value(), 4);
+}
+
 TEST_F(FeedOfflineHostTest, GetOfflineIdNewer) {
   offline_page_model()->AddOfflinedPage(kUrl1, "", 4, base::Time());
   offline_page_model()->AddOfflinedPage(