[Offline Pages] Remove unused URLSearchMode.

Also fixed lint and presubmit errors.

Change-Id: I559d8eed024d84b6fdc0bcc0e7d6019dadb4b5f2
Reviewed-on: https://chromium-review.googlesource.com/1162275Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Yafei Duan <romax@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581063}

chrome/browser/offline_pages/android/offline_page_bridge.cc

@@ -576,8 +576,7 @@ void OfflinePageBridge::SelectPageForOnlineUrl(
 
   OfflinePageUtils::SelectPagesForURL(
       browser_context_, GURL(ConvertJavaStringToUTF8(env, j_online_url)),
-      URLSearchMode::SEARCH_BY_ALL_URLS, tab_id,
-      base::Bind(&SelectPageCallback, j_callback_ref));
+      tab_id, base::BindOnce(&SelectPageCallback, j_callback_ref));
 }
 
 void OfflinePageBridge::SavePage(JNIEnv* env,

chrome/browser/offline_pages/offline_page_request_handler.cc

@@ -4,7 +4,7 @@
 
 #include "chrome/browser/offline_pages/offline_page_request_handler.h"
 
-#include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -415,10 +415,9 @@ void GetPagesToServeURL(
   }
 
   OfflinePageUtils::SelectPagesForURL(
-      web_contents->GetBrowserContext(), url, URLSearchMode::SEARCH_BY_ALL_URLS,
-      tab_id,
-      base::Bind(&SelectPagesForURLDone, url, offline_header, network_state,
-                 job, web_contents_getter));
+      web_contents->GetBrowserContext(), url, tab_id,
+      base::BindOnce(&SelectPagesForURLDone, url, offline_header, network_state,
+                     job, web_contents_getter));
 }
 
 // Do all the things needed to be done on UI thread after a trusted offline
@@ -967,7 +966,7 @@ void OfflinePageRequestHandler::DidOpenForServing(int result) {
 }
 
 void OfflinePageRequestHandler::DidSeekForServing(int64_t result) {
-  DCHECK(result <= 0);
+  DCHECK_LE(result, 0);
 
   ReportSeekResult(result);
 

chrome/browser/offline_pages/offline_page_request_handler.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_OFFLINE_PAGES_OFFLINE_PAGE_REQUEST_HANDLER_H_
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/memory/ref_counted.h"

chrome/browser/offline_pages/offline_page_tab_helper.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/offline_pages/offline_page_tab_helper.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/guid.h"
@@ -282,10 +284,9 @@ void OfflinePageTabHelper::TryLoadingOfflinePageOnNetError(
   }
 
   OfflinePageUtils::SelectPagesForURL(
-      web_contents()->GetBrowserContext(), navigation_handle->GetURL(),
-      URLSearchMode::SEARCH_BY_ALL_URLS, tab_id,
-      base::Bind(&OfflinePageTabHelper::SelectPagesForURLDone,
-                 weak_ptr_factory_.GetWeakPtr()));
+      web_contents()->GetBrowserContext(), navigation_handle->GetURL(), tab_id,
+      base::BindOnce(&OfflinePageTabHelper::SelectPagesForURLDone,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void OfflinePageTabHelper::SelectPagesForURLDone(

chrome/browser/offline_pages/offline_page_tab_helper.h

@@ -5,6 +5,10 @@
 #ifndef CHROME_BROWSER_OFFLINE_PAGES_OFFLINE_PAGE_TAB_HELPER_H_
 #define CHROME_BROWSER_OFFLINE_PAGES_OFFLINE_PAGE_TAB_HELPER_H_
 
+#include <memory>
+#include <string>
+#include <vector>
+
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"

chrome/browser/offline_pages/offline_page_utils.cc

@@ -60,7 +60,7 @@ void OnGetPagesByURLDone(
     const GURL& url,
     int tab_id,
     const std::vector<std::string>& namespaces_to_show_in_original_tab,
-    const base::Callback<void(const std::vector<OfflinePageItem>&)>& callback,
+    base::OnceCallback<void(const std::vector<OfflinePageItem>&)> callback,
     const MultipleOfflinePageItemResult& pages) {
   std::vector<OfflinePageItem> selected_pages;
   std::string tab_id_str = base::IntToString(tab_id);
@@ -79,7 +79,7 @@ void OnGetPagesByURLDone(
   std::sort(selected_pages.begin(), selected_pages.end(),
             OfflinePageComparer());
 
-  callback.Run(selected_pages);
+  std::move(callback).Run(selected_pages);
 }
 
 bool IsSupportedByDownload(content::BrowserContext* browser_context,
@@ -201,24 +201,22 @@ const base::FilePath::CharType OfflinePageUtils::kMHTMLExtension[] =
 void OfflinePageUtils::SelectPagesForURL(
     content::BrowserContext* browser_context,
     const GURL& url,
-    URLSearchMode url_search_mode,
     int tab_id,
-    const base::Callback<void(const std::vector<OfflinePageItem>&)>& callback) {
+    base::OnceCallback<void(const std::vector<OfflinePageItem>&)> callback) {
   OfflinePageModel* offline_page_model =
       OfflinePageModelFactory::GetForBrowserContext(browser_context);
   if (!offline_page_model) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(callback, std::vector<OfflinePageItem>()));
+        FROM_HERE,
+        base::BindOnce(std::move(callback), std::vector<OfflinePageItem>()));
     return;
   }
 
   offline_page_model->GetPagesByURL(
-      url,
-      url_search_mode,
-      base::Bind(&OnGetPagesByURLDone, url, tab_id,
-                 offline_page_model->GetPolicyController()
-                     ->GetNamespacesRestrictedToOriginalTab(),
-                 callback));
+      url, base::BindOnce(&OnGetPagesByURLDone, url, tab_id,
+                          offline_page_model->GetPolicyController()
+                              ->GetNamespacesRestrictedToOriginalTab(),
+                          std::move(callback)));
 }
 
 const OfflinePageItem* OfflinePageUtils::GetOfflinePageFromWebContents(
@@ -329,8 +327,7 @@ void OfflinePageUtils::CheckDuplicateDownloads(
   };
 
   offline_page_model->GetPagesByURL(
-      url, URLSearchMode::SEARCH_BY_ALL_URLS,
-      base::Bind(continuation, browser_context, url, callback));
+      url, base::BindOnce(continuation, browser_context, url, callback));
 }
 
 // static

chrome/browser/offline_pages/offline_page_utils.h

@@ -60,18 +60,17 @@ class OfflinePageUtils {
   // Callback to inform the duplicate checking result.
   using DuplicateCheckCallback = base::Callback<void(DuplicateCheckResult)>;
 
-  // Returns via callback all offline pages related to |url|. The offline page
-  // captured from last visit in the tab will be excluded if its tab id does not
-  // match the provided |tab_id|. The returned list is sorted based creation
-  // date in descending order. That is, the most recently created offline will
-  // appear as the first element of the list.
+  // Returns via callback all offline pages related to |url|. The provided URL
+  // is matched both against the original and the actual URL fields (they
+  // sometimes differ because of possible redirects). If |tab_id| is provided
+  // with a valid ID, offline pages bound to that tab will also be included in
+  // the search. The returned list is sorted by descending creation date so that
+  // the most recent offline page will be the first element of the list.
   static void SelectPagesForURL(
       content::BrowserContext* browser_context,
       const GURL& url,
-      URLSearchMode url_search_mode,
       int tab_id,
-      const base::Callback<void(const std::vector<OfflinePageItem>&)>&
-          callback);
+      base::OnceCallback<void(const std::vector<OfflinePageItem>&)> callback);
 
   // Gets the offline page corresponding to the given web contents.  The
   // returned pointer is owned by the web_contents and may be deleted by user

components/offline_pages/core/model/offline_page_model_taskified.cc

@@ -358,7 +358,6 @@ void OfflinePageModelTaskified::GetPagesByClientIds(
 
 void OfflinePageModelTaskified::GetPagesByURL(
     const GURL& url,
-    URLSearchMode url_search_mode,
     MultipleOfflinePageItemCallback callback) {
   auto task = GetPagesTask::CreateTaskMatchingUrl(store_.get(),
                                                   std::move(callback), url);
@@ -748,7 +747,7 @@ void OfflinePageModelTaskified::OnPersistentPageConsistencyCheckDone(
     bool success,
     const std::vector<int64_t>& pages_deleted) {
   // If there's no persistent page expired, save some effort by exiting early.
-  // TODO(https://crbug.com/834909), use the temporary hidden bit in
+  // TODO(https://crbug.com/834909): Use the temporary hidden bit in
   // DownloadUIAdapter instead of calling remove directly.
   if (pages_deleted.size() > 0)
     download_manager_->Remove(pages_deleted);

components/offline_pages/core/model/offline_page_model_taskified.h

@@ -105,7 +105,6 @@ class OfflinePageModelTaskified : public OfflinePageModel,
   void GetPagesByClientIds(const std::vector<ClientId>& client_ids,
                            MultipleOfflinePageItemCallback callback) override;
   void GetPagesByURL(const GURL& url,
-                     URLSearchMode url_search_mode,
                      MultipleOfflinePageItemCallback callback) override;
   void GetPagesByNamespace(const std::string& name_space,
                            MultipleOfflinePageItemCallback callback) override;

components/offline_pages/core/model/offline_page_model_taskified_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 #include <memory>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_enumerator.h"
@@ -65,9 +66,9 @@ const ClientId kTestUserRequestedClientId(kDownloadNamespace, "714");
 const ClientId kTestBrowserActionsClientId(kBrowserActionsNamespace, "999");
 const int64_t kTestFileSize = 876543LL;
 const base::string16 kTestTitle = base::UTF8ToUTF16("a title");
-const std::string kTestRequestOrigin("abc.xyz");
-const std::string kEmptyRequestOrigin;
-const std::string kTestDigest("test digest");
+const char kTestRequestOrigin[] = "abc.xyz";
+const char kEmptyRequestOrigin[] = "";
+const char kTestDigest[] = "test digest";
 const int64_t kDownloadId = 42LL;
 
 }  // namespace
@@ -941,21 +942,18 @@ TEST_F(OfflinePageModelTaskifiedTest, GetPagesByUrl_FinalUrl) {
   // Search by kTestUrl.
   base::MockCallback<MultipleOfflinePageItemCallback> callback;
   EXPECT_CALL(callback, Run(ElementsAre(page1)));
-  model()->GetPagesByURL(kTestUrl, URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
-                         callback.Get());
+  model()->GetPagesByURL(kTestUrl, callback.Get());
   EXPECT_TRUE(task_queue()->HasRunningTask());
   PumpLoop();
 
   // Search by kTestUrl2.
   EXPECT_CALL(callback, Run(ElementsAre(page2)));
-  model()->GetPagesByURL(kTestUrl2, URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
-                         callback.Get());
+  model()->GetPagesByURL(kTestUrl2, callback.Get());
   PumpLoop();
 
   // Search by random url, which should return no pages.
   EXPECT_CALL(callback, Run(IsEmpty()));
-  model()->GetPagesByURL(kOtherUrl, URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
-                         callback.Get());
+  model()->GetPagesByURL(kOtherUrl, callback.Get());
   EXPECT_TRUE(task_queue()->HasRunningTask());
   PumpLoop();
 }
@@ -973,22 +971,19 @@ TEST_F(OfflinePageModelTaskifiedTest,
   base::MockCallback<MultipleOfflinePageItemCallback> callback;
   EXPECT_CALL(callback, Run(ElementsAre(page1)));
   model()->GetPagesByURL(kTestUrlWithFragment,
-                         URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
                          callback.Get());
   EXPECT_TRUE(task_queue()->HasRunningTask());
   PumpLoop();
 
   // Search by kTestUrl2.
   EXPECT_CALL(callback, Run(ElementsAre(page2)));
-  model()->GetPagesByURL(kTestUrl2, URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
-                         callback.Get());
+  model()->GetPagesByURL(kTestUrl2, callback.Get());
   EXPECT_TRUE(task_queue()->HasRunningTask());
   PumpLoop();
 
   // Search by kTestUrl2WithFragment.
   EXPECT_CALL(callback, Run(ElementsAre(page2)));
   model()->GetPagesByURL(kTestUrl2WithFragment,
-                         URLSearchMode::SEARCH_BY_FINAL_URL_ONLY,
                          callback.Get());
   EXPECT_TRUE(task_queue()->HasRunningTask());
   PumpLoop();
@@ -1006,8 +1001,7 @@ TEST_F(OfflinePageModelTaskifiedTest, GetPagesByUrl_AllUrls) {
 
   base::MockCallback<MultipleOfflinePageItemCallback> callback;
   EXPECT_CALL(callback, Run(UnorderedElementsAre(page1, page2)));
-  model()->GetPagesByURL(kTestUrl2, URLSearchMode::SEARCH_BY_ALL_URLS,
-                         callback.Get());
+  model()->GetPagesByURL(kTestUrl2, callback.Get());
   PumpLoop();
 }
 

components/offline_pages/core/offline_page_model.h

@@ -191,10 +191,12 @@ class OfflinePageModel : public base::SupportsUserData, public KeyedService {
       const std::vector<ClientId>& client_ids,
       MultipleOfflinePageItemCallback callback) = 0;
 
-  // Returns the offline pages that are related to |url|. |url_search_mode|
-  // controls how the url match is done. See URLSearchMode for more details.
+  // Returns via callback all offline pages related to |url|. The provided URL
+  // is matched both against the original and the actual URL fields (they
+  // sometimes differ because of possible redirects). The returned list is
+  // sorted by descending creation date so that the most recent offline page
+  // will be the first element of the list.
   virtual void GetPagesByURL(const GURL& url,
-                             URLSearchMode url_search_mode,
                              MultipleOfflinePageItemCallback callback) = 0;
 
   // Returns the offline pages that belong in |name_space|.

components/offline_pages/core/offline_page_types.h

@@ -6,7 +6,7 @@
 #define COMPONENTS_OFFLINE_PAGES_CORE_OFFLINE_PAGE_TYPES_H_
 
 #include <stdint.h>
-
+#include <memory>
 #include <set>
 #include <vector>
 
@@ -81,19 +81,9 @@ enum class DeletePageResult {
   RESULT_COUNT,
 };
 
-// Controls how to search on differnt URLs for pages.
-enum class URLSearchMode {
-  // Match against the last committed URL only.
-  SEARCH_BY_FINAL_URL_ONLY,
-  // Match against all stored URLs, including the last committed URL and
-  // the original request URL.
-  SEARCH_BY_ALL_URLS,
-};
-
 typedef std::vector<int64_t> MultipleOfflineIdResult;
 typedef std::vector<OfflinePageItem> MultipleOfflinePageItemResult;
 
-// TODO(carlosk): All or most of these should use base::OnceCallback.
 typedef base::OnceCallback<void(SavePageResult, int64_t)> SavePageCallback;
 typedef base::OnceCallback<void(AddPageResult, int64_t)> AddPageCallback;
 typedef base::OnceCallback<void(DeletePageResult)> DeletePageCallback;

components/offline_pages/core/stub_offline_page_model.cc

@@ -55,7 +55,6 @@ void StubOfflinePageModel::GetPageByGuid(
     SingleOfflinePageItemCallback callback) {}
 void StubOfflinePageModel::GetPagesByURL(
     const GURL& url,
-    URLSearchMode url_search_mode,
     MultipleOfflinePageItemCallback callback) {}
 void StubOfflinePageModel::GetPagesByRequestOrigin(
     const std::string& origin,
@@ -81,7 +80,7 @@ void StubOfflinePageModel::HasThumbnailForOfflineId(
 void StubOfflinePageModel::PublishInternalArchive(
     const OfflinePageItem& offline_page,
     std::unique_ptr<OfflinePageArchiver> archiver,
-    PublishPageCallback publish_done_callback){};
+    PublishPageCallback publish_done_callback) {}
 const base::FilePath& StubOfflinePageModel::GetInternalArchiveDirectory(
     const std::string& name_space) const {
   return archive_directory_;

components/offline_pages/core/stub_offline_page_model.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_OFFLINE_PAGES_CORE_STUB_OFFLINE_PAGE_MODEL_H_
 #define COMPONENTS_OFFLINE_PAGES_CORE_STUB_OFFLINE_PAGE_MODEL_H_
 
+#include <memory>
 #include <set>
 #include <string>
 #include <vector>
@@ -51,7 +52,6 @@ class StubOfflinePageModel : public OfflinePageModel {
   void GetPageByGuid(const std::string& guid,
                      SingleOfflinePageItemCallback callback) override;
   void GetPagesByURL(const GURL& url,
-                     URLSearchMode url_search_mode,
                      MultipleOfflinePageItemCallback callback) override;
   void GetPagesByRequestOrigin(
       const std::string& origin,