Move timeout timer into WebApkIconHasher

chrome/browser/android/webapk/webapk_icon_hasher.h

 // Copyright 2016 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.
 
 #ifndef CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_ICON_HASHER_H_
 #define CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_ICON_HASHER_H_
 
 #include <memory>
 #include <string>
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "net/url_request/url_fetcher_delegate.h"
+#include "url/gurl.h"
 
 namespace net {
 class URLFetcher;
 class URLRequestContextGetter;
 }
 
 class GURL;

[dominickn, 2017/03/24 02:41:01]

If you are #including GURL, you can remove this.

[F, 2017/03/27 20:37:09]

Done.

 
 // Downloads an icon and takes a Murmur2 hash of the downloaded image.
 class WebApkIconHasher : public net::URLFetcherDelegate {
  public:
   using Murmur2HashCallback =
       base::Callback<void(const std::string& /* icon_murmur2_hash */)>;
 
-  WebApkIconHasher();
-  ~WebApkIconHasher() override;
-
-  // Downloads |icon_url|. Calls |callback| with the Murmur2 hash of the
-  // downloaded image. The hash is taken over the raw image bytes (no image
-  // encoding/decoding beforehand). |callback| is called with an empty string if
-  // the image cannot not be downloaded (e.g. 404 HTTP error code).
-  void DownloadAndComputeMurmur2Hash(
+  WebApkIconHasher(
       net::URLRequestContextGetter* request_context_getter,
       const GURL& icon_url,
       const Murmur2HashCallback& callback);
+  ~WebApkIconHasher() override;
+
+  // Downloads icon_url_. Calls callback_ with the Murmur2 hash of the

[dominickn, 2017/03/24 02:41:01]

|icon_url_|, |callback_|

[F, 2017/03/27 20:37:09]

Done.

+  // downloaded image. The hash is taken over the raw image bytes (no image
+  // encoding/decoding beforehand). callback_ is called with an empty string if

[dominickn, 2017/03/24 02:41:00]

|callback_|

[F, 2017/03/27 20:37:08]

Done.

+  // the image cannot not be downloaded in time (e.g. 404 HTTP error code).
+  void DownloadAndComputeMurmur2Hash();
 
  private:
   // net::URLFetcherDelegate:
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
-  // Fetches the image.
-  std::unique_ptr<net::URLFetcher> url_fetcher_;
+  // Called if downloading the icon takes too long.
+  void OnDownloadTimedOut();
+
+  // For retrieving URLRequestContext.
+  net::URLRequestContextGetter* url_request_context_getter_;
+
+  // The icon URL.
+  GURL icon_url_;
 
   // Called with the image hash.
   Murmur2HashCallback callback_;
 
+  // Fetches the image.

[dominickn, 2017/03/24 02:41:01]

This comment isn't needed

[F, 2017/03/27 20:37:09]

Done.

+  std::unique_ptr<net::URLFetcher> url_fetcher_;
+
+  // Whether the download has completed or timed out.
+  bool is_download_complete_;
+
+  // Fails WebApkIconHasher if the download takes too long.
+  base::OneShotTimer download_timeout_timer_;
+
+  // Used to get |weak_ptr_|.
+  base::WeakPtrFactory<WebApkIconHasher> weak_ptr_factory_;

[dominickn, 2017/03/24 02:41:01]

Instead of  a weak_ptr_factory_ and is_download_complete_, can't you just do
url_fetcher_.reset() in OnDownloadTimedOut() in OnDownloadTimedOut()? That
should guarantee that if there is a timeout, the task is cancelled (never calls
OnURLFetchComplete), and when the fetch completes, the timeout timer is stopped
immediately (OnDownloadTimedOut is never called).

[F, 2017/03/27 20:37:09]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(WebApkIconHasher);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_ICON_HASHER_H_