Migrate ios/chrome/browser/net/retryable_url_fetcher.mm using SimpleURLLoader

This isn't strictly needed since iOS will always use net-in-process, but is
good for consistency with the rest of the code that ios shares with chrome
that has switched to SimpleURLLoader.

Bug: 879770
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I49665d4b6054c1a882a42d4661f03162bfceebc7
Reviewed-on: https://chromium-review.googlesource.com/1235722
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#593169}

ios/chrome/browser/net/BUILD.gn

@@ -69,8 +69,7 @@ source_set("unit_tests") {
     "//ios/net",
     "//ios/net:test_support",
     "//ios/web/public/test",
-    "//net",
-    "//net:test_support",
+    "//services/network:test_support",
     "//testing/gtest",
   ]
 }

ios/chrome/browser/net/retryable_url_fetcher.h

@@ -7,11 +7,13 @@
 
 #import <Foundation/Foundation.h>
 
+#include "base/memory/scoped_refptr.h"
 #include "net/base/backoff_entry.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
-namespace net {
-class URLRequestContextGetter;
-}  // namespace net
+namespace network {
+class SharedURLLoaderFactory;
+}
 
 // Delegate protocol for RetryableURLFetcher object.
 @protocol RetryableURLFetcherDelegate<NSObject>
@@ -28,13 +30,14 @@ class URLRequestContextGetter;
 
 @interface RetryableURLFetcher : NSObject
 
-// Designated initializer. |context| and |delegate| must not be nil. If |policy|
-// is not null, it specifies how often to retry the URL fetch on a call to
-// -startFetch. If |policy| is null, there is no retry.
+// Designated initializer. |shared_url_loader_factory| and |delegate| must not
+// be nil. If |policy| is not null, it specifies how often to retry the URL
+// fetch on a call to -startFetch. If |policy| is null, there is no retry.
 - (instancetype)
-    initWithRequestContextGetter:(net::URLRequestContextGetter*)context
-                        delegate:(id<RetryableURLFetcherDelegate>)delegate
-                   backoffPolicy:(const net::BackoffEntry::Policy*)policy;
+initWithURLLoaderFactory:
+    (scoped_refptr<network::SharedURLLoaderFactory>)shared_url_loader_factory
+                delegate:(id<RetryableURLFetcherDelegate>)delegate
+           backoffPolicy:(const net::BackoffEntry::Policy*)policy;
 
 // Starts fetching URL. Uses the backoff policy specified when the object was
 // initialized.

ios/chrome/browser/net/retryable_url_fetcher.mm

@@ -8,10 +8,9 @@
 
 #include "base/logging.h"
 #include "base/strings/sys_string_conversions.h"
-#include "net/http/http_status_code.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -19,38 +18,27 @@
 #endif
 
 @interface RetryableURLFetcher ()
-- (void)urlFetchDidComplete:(const net::URLFetcher*)fetcher;
+- (void)urlFetchDidComplete:(std::unique_ptr<std::string>)response_body;
 @end
 
-class URLRequestDelegate : public net::URLFetcherDelegate {
- public:
-  explicit URLRequestDelegate(RetryableURLFetcher* owner) : owner_(owner) {}
-  void OnURLFetchComplete(const net::URLFetcher* source) override {
-    [owner_ urlFetchDidComplete:source];
-  }
-
- private:
-  __weak RetryableURLFetcher* owner_ = nil;
-};
-
 @implementation RetryableURLFetcher {
-  scoped_refptr<net::URLRequestContextGetter> requestContextGetter_;
-  std::unique_ptr<URLRequestDelegate> fetcherDelegate_;
-  std::unique_ptr<net::URLFetcher> fetcher_;
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
   std::unique_ptr<net::BackoffEntry> backoffEntry_;
+  std::unique_ptr<network::SimpleURLLoader> simple_loader_;
   int retryCount_;
   __weak id<RetryableURLFetcherDelegate> delegate_;
 }
 
 - (instancetype)
-    initWithRequestContextGetter:(net::URLRequestContextGetter*)context
-                        delegate:(id<RetryableURLFetcherDelegate>)delegate
-                   backoffPolicy:(const net::BackoffEntry::Policy*)policy {
+initWithURLLoaderFactory:
+    (scoped_refptr<network::SharedURLLoaderFactory>)shared_url_loader_factory
+                delegate:(id<RetryableURLFetcherDelegate>)delegate
+           backoffPolicy:(const net::BackoffEntry::Policy*)policy {
   self = [super init];
   if (self) {
-    DCHECK(context);
+    DCHECK(shared_url_loader_factory);
     DCHECK(delegate);
-    requestContextGetter_ = context;
+    shared_url_loader_factory_ = shared_url_loader_factory;
     delegate_ = delegate;
     if (policy)
       backoffEntry_.reset(new net::BackoffEntry(policy));
@@ -59,14 +47,20 @@ class URLRequestDelegate : public net::URLFetcherDelegate {
 }
 
 - (void)startFetch {
-  DCHECK(requestContextGetter_.get());
+  DCHECK(shared_url_loader_factory_.get());
   GURL url(base::SysNSStringToUTF8([delegate_ urlToFetch]));
   if (url.is_valid()) {
-    fetcherDelegate_.reset(new URLRequestDelegate(self));
-    fetcher_ = net::URLFetcher::Create(url, net::URLFetcher::GET,
-                                       fetcherDelegate_.get());
-    fetcher_->SetRequestContext(requestContextGetter_.get());
-    fetcher_->Start();
+    auto resource_request = std::make_unique<network::ResourceRequest>();
+    resource_request->url = url;
+    simple_loader_ = network::SimpleURLLoader::Create(
+        std::move(resource_request), NO_TRAFFIC_ANNOTATION_YET);
+
+    simple_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+        shared_url_loader_factory_.get(),
+        base::BindOnce(^(std::unique_ptr<std::string> response) {
+          [self urlFetchDidComplete:std::forward<std::unique_ptr<std::string>>(
+                                        response)];
+        }));
   } else {
     // Invalid URLs returned from delegate method are considered a permanent
     // failure. Delegate method is called with nil to indicate failure.
@@ -78,9 +72,8 @@ class URLRequestDelegate : public net::URLFetcherDelegate {
   return backoffEntry_ ? backoffEntry_->failure_count() : 0;
 }
 
-- (void)urlFetchDidComplete:(const net::URLFetcher*)fetcher {
-  BOOL success = fetcher->GetResponseCode() == net::HTTP_OK;
-  if (!success && backoffEntry_) {
+- (void)urlFetchDidComplete:(std::unique_ptr<std::string>)response_body {
+  if (!response_body && backoffEntry_) {
     backoffEntry_->InformOfRequest(false);
     double nextRetry = backoffEntry_->GetTimeUntilRelease().InSecondsF();
     dispatch_after(dispatch_time(DISPATCH_TIME_NOW, nextRetry * NSEC_PER_SEC),
@@ -90,11 +83,8 @@ class URLRequestDelegate : public net::URLFetcherDelegate {
     return;
   }
   NSString* response = nil;
-  if (success) {
-    std::string responseString;
-    if (fetcher->GetResponseAsString(&responseString))
-      response = base::SysUTF8ToNSString(responseString);
-  }
+  if (response_body)
+    response = base::SysUTF8ToNSString(*response_body);
   [delegate_ processSuccessResponse:response];
 }
 

ios/chrome/browser/net/retryable_url_fetcher_unittest.mm

@@ -6,10 +6,10 @@
 
 #include "base/message_loop/message_loop.h"
 #import "base/strings/sys_string_conversions.h"
+#include "base/test/scoped_task_environment.h"
 #include "ios/web/public/test/test_web_thread.h"
-#include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_test_util.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
 
@@ -47,7 +47,7 @@ NSString* const kFakeResponseString = @"Something interesting here.";
 @end
 
 @interface TestFailingURLFetcherDelegate : NSObject<RetryableURLFetcherDelegate>
-@property(nonatomic, assign) BOOL responsesProcessed;
+@property(nonatomic, assign) NSUInteger responsesProcessed;
 @end
 
 @implementation TestFailingURLFetcherDelegate
@@ -59,7 +59,7 @@ NSString* const kFakeResponseString = @"Something interesting here.";
 
 - (void)processSuccessResponse:(NSString*)response {
   EXPECT_FALSE(response);
-  responsesProcessed = YES;
+  ++responsesProcessed;
 }
 
 @end
@@ -67,80 +67,76 @@ NSString* const kFakeResponseString = @"Something interesting here.";
 namespace {
 
 class RetryableURLFetcherTest : public PlatformTest {
+ public:
+  RetryableURLFetcherTest()
+      : test_shared_url_loader_factory_(
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)) {}
+
  protected:
   void SetUp() override {
     PlatformTest::SetUp();
     test_delegate_ = [[TestRetryableURLFetcherDelegate alloc] init];
-    io_thread_.reset(
-        new web::TestWebThread(web::WebThread::IO, &message_loop_));
   }
 
-  net::TestURLFetcherFactory factory_;
-  std::unique_ptr<web::TestWebThread> io_thread_;
-  base::MessageLoop message_loop_;
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory>
+      test_shared_url_loader_factory_;
   TestRetryableURLFetcherDelegate* test_delegate_;
 };
 
 TEST_F(RetryableURLFetcherTest, TestResponse200) {
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter =
-      new net::TestURLRequestContextGetter(message_loop_.task_runner());
   RetryableURLFetcher* retryableFetcher = [[RetryableURLFetcher alloc]
-      initWithRequestContextGetter:request_context_getter.get()
-                          delegate:test_delegate_
-                     backoffPolicy:nil];
-  [retryableFetcher startFetch];
+      initWithURLLoaderFactory:test_shared_url_loader_factory_
+                      delegate:test_delegate_
+                 backoffPolicy:nil];
 
-  // Manually calls the delegate.
-  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
-  DCHECK(fetcher);
-  DCHECK(fetcher->delegate());
   [test_delegate_ setResponsesProcessed:0U];
-  fetcher->set_response_code(200);
-  fetcher->SetResponseString(base::SysNSStringToUTF8(kFakeResponseString));
-  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  std::string url = base::SysNSStringToUTF8([test_delegate_ urlToFetch]);
+  test_url_loader_factory_.AddResponse(
+      url, base::SysNSStringToUTF8(kFakeResponseString), net::HTTP_OK);
+
+  [retryableFetcher startFetch];
+  scoped_task_environment_.RunUntilIdle();
+
   EXPECT_EQ(1U, [test_delegate_ responsesProcessed]);
 }
 
 TEST_F(RetryableURLFetcherTest, TestResponse404) {
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter =
-      new net::TestURLRequestContextGetter(message_loop_.task_runner());
   RetryableURLFetcher* retryableFetcher = [[RetryableURLFetcher alloc]
-      initWithRequestContextGetter:request_context_getter.get()
-                          delegate:test_delegate_
-                     backoffPolicy:nil];
-  [retryableFetcher startFetch];
+      initWithURLLoaderFactory:test_shared_url_loader_factory_
+                      delegate:test_delegate_
+                 backoffPolicy:nil];
 
-  // Manually calls the delegate.
-  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
-  DCHECK(fetcher);
-  DCHECK(fetcher->delegate());
   [test_delegate_ setResponsesProcessed:0U];
-  fetcher->set_response_code(404);
-  fetcher->SetResponseString("");
-  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  std::string url = base::SysNSStringToUTF8([test_delegate_ urlToFetch]);
+  test_url_loader_factory_.AddResponse(url, "", net::HTTP_NOT_FOUND);
+
+  [retryableFetcher startFetch];
+  scoped_task_environment_.RunUntilIdle();
+
   EXPECT_EQ(0U, [test_delegate_ responsesProcessed]);
 }
 
 // Tests that response callback method is called if delegate returns an
 // invalid URL.
 TEST_F(RetryableURLFetcherTest, TestFailingURLNoRetry) {
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter =
-      new net::TestURLRequestContextGetter(message_loop_.task_runner());
   TestFailingURLFetcherDelegate* failing_delegate =
       [[TestFailingURLFetcherDelegate alloc] init];
-  RetryableURLFetcher* retryable_fetcher = [[RetryableURLFetcher alloc]
-      initWithRequestContextGetter:request_context_getter.get()
-                          delegate:failing_delegate
-                     backoffPolicy:nil];
-  [retryable_fetcher startFetch];
-
-  // |failing_delegate| does not have URL to fetch, so a fetcher should never
-  // be created.
-  net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
-  EXPECT_FALSE(fetcher);
-
-  // Verify that response has been called.
-  EXPECT_TRUE([failing_delegate responsesProcessed]);
+  RetryableURLFetcher* retryableFetcher = [[RetryableURLFetcher alloc]
+      initWithURLLoaderFactory:test_shared_url_loader_factory_
+                      delegate:failing_delegate
+                 backoffPolicy:nil];
+
+  [failing_delegate setResponsesProcessed:0U];
+
+  [retryableFetcher startFetch];
+  scoped_task_environment_.RunUntilIdle();
+
+  // Verify that a response has been received, even if the failing delegate
+  // received a nil in processSuccessResponse to indicate the failure.
+  EXPECT_EQ(1U, [failing_delegate responsesProcessed]);
 }
 
 }  // namespace