Support POST requests in Download Manager on iOS.

According to Download.IOSDownloadedFileStatusCode UMA metric there is
non-trivial number of download failures with 405 http code (Method Not
Allowed). This is because DownloadTask always creates NSURLTask with
GET HTTP method regardless of the original HTTP method in the request.

This CL adds support for POST request, because this is likely the
easiest fix for the problem (I would be surprised if PUT and DELETE
methods are used for downloads). We can add support for all HTTP
methods later if necessary.

Bug: 1030164
Change-Id: Ic3023fee44c580af7e4540b9de15ca957d43d047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948283
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721118}

ios/web/download/download_controller_impl.h

@@ -32,6 +32,7 @@ class DownloadControllerImpl : public DownloadController,
   void CreateDownloadTask(WebState* web_state,
                           NSString* identifier,
                           const GURL& original_url,
+                          NSString* http_method,
                           const std::string& content_disposition,
                           int64_t total_bytes,
                           const std::string& mime_type,

ios/web/download/download_controller_impl.mm

@@ -51,6 +51,7 @@ void DownloadControllerImpl::CreateDownloadTask(
     WebState* web_state,
     NSString* identifier,
     const GURL& original_url,
+    NSString* http_method,
     const std::string& content_disposition,
     int64_t total_bytes,
     const std::string& mime_type,
@@ -60,8 +61,8 @@ void DownloadControllerImpl::CreateDownloadTask(
     return;
 
   auto task = std::make_unique<DownloadTaskImpl>(
-      web_state, original_url, content_disposition, total_bytes, mime_type,
-      page_transition, identifier, this);
+      web_state, original_url, http_method, content_disposition, total_bytes,
+      mime_type, page_transition, identifier, this);
   alive_tasks_.insert(task.get());
   delegate_->OnDownloadCreated(this, web_state, std::move(task));
 }

ios/web/download/download_controller_impl_unittest.mm

@@ -65,7 +65,7 @@ TEST_F(DownloadControllerImplTest, OnDownloadCreated) {
   NSString* identifier = [NSUUID UUID].UUIDString;
   GURL url("https://download.test");
   download_controller_->CreateDownloadTask(
-      &web_state_, identifier, url, kContentDisposition,
+      &web_state_, identifier, url, @"POST", kContentDisposition,
       /*total_bytes=*/-1, kMimeType, ui::PageTransition::PAGE_TRANSITION_TYPED);
 
   ASSERT_EQ(1U, delegate_.alive_download_tasks().size());
@@ -73,6 +73,7 @@ TEST_F(DownloadControllerImplTest, OnDownloadCreated) {
   EXPECT_EQ(&web_state_, delegate_.alive_download_tasks()[0].first);
   EXPECT_NSEQ(identifier, task->GetIndentifier());
   EXPECT_EQ(url, task->GetOriginalUrl());
+  EXPECT_NSEQ(@"POST", task->GetHttpMethod());
   EXPECT_FALSE(task->IsDone());
   EXPECT_EQ(0, task->GetErrorCode());
   EXPECT_EQ(-1, task->GetTotalBytes());
@@ -90,7 +91,7 @@ TEST_F(DownloadControllerImplTest, NullDelegate) {
   download_controller_->SetDelegate(nullptr);
   GURL url("https://download.test");
   download_controller_->CreateDownloadTask(
-      &web_state_, [NSUUID UUID].UUIDString, url, kContentDisposition,
+      &web_state_, [NSUUID UUID].UUIDString, url, @"GET", kContentDisposition,
       /*total_bytes=*/-1, kMimeType, ui::PageTransition::PAGE_TRANSITION_LINK);
 }
 

ios/web/download/download_task_impl.h

@@ -47,6 +47,7 @@ class DownloadTaskImpl : public DownloadTask {
   // |delegate| must be valid.
   DownloadTaskImpl(const WebState* web_state,
                    const GURL& original_url,
+                   NSString* http_method,
                    const std::string& content_disposition,
                    int64_t total_bytes,
                    const std::string& mime_type,
@@ -64,6 +65,7 @@ class DownloadTaskImpl : public DownloadTask {
   net::URLFetcherResponseWriter* GetResponseWriter() const override;
   NSString* GetIndentifier() const override;
   const GURL& GetOriginalUrl() const override;
+  NSString* GetHttpMethod() const override;
   bool IsDone() const override;
   int GetErrorCode() const override;
   int GetHttpCode() const override;
@@ -119,6 +121,7 @@ class DownloadTaskImpl : public DownloadTask {
   State state_ = State::kNotStarted;
   std::unique_ptr<net::URLFetcherResponseWriter> writer_;
   GURL original_url_;
+  NSString* http_method_ = nil;
   int error_code_ = 0;
   int http_code_ = -1;
   int64_t total_bytes_ = -1;

ios/web/download/download_task_impl.mm

@@ -175,6 +175,7 @@ namespace web {
 
 DownloadTaskImpl::DownloadTaskImpl(const WebState* web_state,
                                    const GURL& original_url,
+                                   NSString* http_method,
                                    const std::string& content_disposition,
                                    int64_t total_bytes,
                                    const std::string& mime_type,
@@ -182,6 +183,7 @@ DownloadTaskImpl::DownloadTaskImpl(const WebState* web_state,
                                    NSString* identifier,
                                    Delegate* delegate)
     : original_url_(original_url),
+      http_method_(http_method),
       total_bytes_(total_bytes),
       content_disposition_(content_disposition),
       original_mime_type_(mime_type),
@@ -270,6 +272,11 @@ const GURL& DownloadTaskImpl::GetOriginalUrl() const {
   return original_url_;
 }
 
+NSString* DownloadTaskImpl::GetHttpMethod() const {
+  DCHECK_CURRENTLY_ON(web::WebThread::UI);
+  return http_method_;
+}
+
 bool DownloadTaskImpl::IsDone() const {
   DCHECK_CURRENTLY_ON(web::WebThread::UI);
   return state_ == State::kComplete || state_ == State::kCancelled;
@@ -448,7 +455,9 @@ void DownloadTaskImpl::StartWithCookies(NSArray<NSHTTPCookie*>* cookies) {
       UIApplicationStateActive;
 
   NSURL* url = net::NSURLWithGURL(GetOriginalUrl());
-  session_task_ = [session_ dataTaskWithURL:url];
+  NSMutableURLRequest* request = [[NSMutableURLRequest alloc] initWithURL:url];
+  request.HTTPMethod = GetHttpMethod();
+  session_task_ = [session_ dataTaskWithRequest:request];
   [session_task_ resume];
   OnDownloadUpdated();
 }

ios/web/download/download_task_impl_unittest.mm

@@ -49,6 +49,7 @@ namespace {
 const char kUrl[] = "chromium://download.test/";
 const char kContentDisposition[] = "attachment; filename=file.test";
 const char kMimeType[] = "application/pdf";
+NSString* const kHttpMethod = @"POST";
 
 class MockDownloadTaskObserver : public DownloadTaskObserver {
  public:
@@ -128,6 +129,7 @@ class DownloadTaskImplTest : public PlatformTest {
       : task_(std::make_unique<DownloadTaskImpl>(
             &web_state_,
             GURL(kUrl),
+            kHttpMethod,
             kContentDisposition,
             /*total_bytes=*/-1,
             kMimeType,
@@ -152,7 +154,9 @@ class DownloadTaskImplTest : public PlatformTest {
     CRWFakeNSURLSessionTask* session_task =
         [[CRWFakeNSURLSessionTask alloc] initWithURL:url];
     EXPECT_TRUE(task_delegate_.session());
-    OCMExpect([task_delegate_.session() dataTaskWithURL:url])
+    NSMutableURLRequest* request = [NSMutableURLRequest requestWithURL:url];
+    request.HTTPMethod = kHttpMethod;
+    OCMExpect([task_delegate_.session() dataTaskWithRequest:request])
         .andReturn(session_task);
 
     // Start the download.
@@ -693,7 +697,7 @@ TEST_F(DownloadTaskImplTest, ValidDataUrl) {
   // Create data:// url download task.
   char kDataUrl[] = "data:text/plain;base64,Q2hyb21pdW0=";
   auto task = std::make_unique<DownloadTaskImpl>(
-      &web_state_, GURL(kDataUrl), kContentDisposition,
+      &web_state_, GURL(kDataUrl), @"GET", kContentDisposition,
       /*total_bytes=*/-1, kMimeType, ui::PageTransition::PAGE_TRANSITION_TYPED,
       task_delegate_.configuration().identifier, &task_delegate_);
 
@@ -725,7 +729,7 @@ TEST_F(DownloadTaskImplTest, EmptyDataUrl) {
   // Create data:// url download task.
   char kDataUrl[] = "data://";
   auto task = std::make_unique<DownloadTaskImpl>(
-      &web_state_, GURL(kDataUrl), kContentDisposition,
+      &web_state_, GURL(kDataUrl), @"GET", kContentDisposition,
       /*total_bytes=*/-1, kMimeType, ui::PageTransition::PAGE_TRANSITION_TYPED,
       task_delegate_.configuration().identifier, &task_delegate_);
 

ios/web/navigation/crw_wk_navigation_handler.mm

@@ -1415,11 +1415,15 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
   }
 
   ui::PageTransition transition = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
+  NSString* HTTPMethod = @"GET";
   if (WKResponse.forMainFrame) {
     web::NavigationContextImpl* context =
         [self contextForPendingMainFrameNavigationWithURL:responseURL];
     context->SetIsDownload(true);
     context->ReleaseItem();
+    if (context->IsPost()) {
+      HTTPMethod = @"POST";
+    }
     // Navigation callbacks can only be called for the main frame.
     self.webStateImpl->OnNavigationFinished(context);
     transition = context->GetPageTransition();
@@ -1436,8 +1440,8 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
   web::DownloadController::FromBrowserState(
       self.webStateImpl->GetBrowserState())
       ->CreateDownloadTask(self.webStateImpl, [NSUUID UUID].UUIDString,
-                           responseURL, contentDisposition, contentLength,
-                           MIMEType, transition);
+                           responseURL, HTTPMethod, contentDisposition,
+                           contentLength, MIMEType, transition);
 }
 
 // Updates URL for navigation context and navigation item.

ios/web/public/download/download_controller.h

@@ -67,6 +67,7 @@ class WebState;
 //       [self webStateAtIndex:info.webStateIndex],
 //       identifier,
 //       info.originalURL,
+//       info.originalHTTPMethod,
 //       info.contentDisposition,
 //       info.totalBytes,
 //       info.MIMEType,
@@ -100,6 +101,7 @@ class DownloadController {
   virtual void CreateDownloadTask(WebState* web_state,
                                   NSString* identifier,
                                   const GURL& original_url,
+                                  NSString* http_method,
                                   const std::string& content_disposition,
                                   int64_t total_bytes,
                                   const std::string& mime_type,

ios/web/public/download/download_task.h

@@ -66,6 +66,10 @@ class DownloadTask {
   // differ from the final download URL if there were redirects.
   virtual const GURL& GetOriginalUrl() const = 0;
 
+  // HTTP method for this download task (only @"GET" and @"POST" are currently
+  // supported).
+  virtual NSString* GetHttpMethod() const = 0;
+
   // Returns true if the download is in a terminal state. This includes
   // completed downloads, cancelled downloads, and interrupted downloads that
   // can't be resumed.

ios/web/public/test/fakes/fake_download_task.h

@@ -27,6 +27,7 @@ class FakeDownloadTask : public DownloadTask {
   net::URLFetcherResponseWriter* GetResponseWriter() const override;
   NSString* GetIndentifier() const override;
   const GURL& GetOriginalUrl() const override;
+  NSString* GetHttpMethod() const override;
   bool IsDone() const override;
   int GetErrorCode() const override;
   int GetHttpCode() const override;

ios/web/public/test/fakes/fake_download_task.mm

@@ -53,6 +53,10 @@ const GURL& FakeDownloadTask::GetOriginalUrl() const {
   return original_url_;
 }
 
+NSString* FakeDownloadTask::GetHttpMethod() const {
+  return @"GET";
+}
+
 bool FakeDownloadTask::IsDone() const {
   return state_ == State::kComplete;
 }

ios/web/web_state/ui/crw_web_controller_unittest.mm

@@ -708,6 +708,7 @@ TEST_P(CRWWebControllerResponseTest,
   EXPECT_EQ(-1, task->GetTotalBytes());
   EXPECT_TRUE(task->GetContentDisposition().empty());
   EXPECT_TRUE(task->GetMimeType().empty());
+  EXPECT_NSEQ(@"GET", task->GetHttpMethod());
   EXPECT_TRUE(ui::PageTransitionTypeIncludingQualifiersIs(
       task->GetTransitionType(),
       ui::PageTransition::PAGE_TRANSITION_CLIENT_REDIRECT));
@@ -736,6 +737,37 @@ TEST_P(CRWWebControllerResponseTest,
   ASSERT_TRUE(download_delegate_.alive_download_tasks().empty());
 }
 
+// Tests that webView:decidePolicyForNavigationResponse:decisionHandler:
+// correctly uses POST HTTP method for post requests.
+TEST_P(CRWWebControllerResponseTest, DownloadForPostRequest) {
+  // Simulate regular navigation response for post request with text/html MIME
+  // type.
+  GURL url(kTestURLString);
+  AddPendingItem(url, ui::PAGE_TRANSITION_TYPED);
+  web_controller()
+      .webStateImpl->GetNavigationManagerImpl()
+      .GetPendingItemInCurrentOrRestoredSession()
+      ->SetPostData([NSData data]);
+  [web_controller() loadCurrentURLWithRendererInitiatedNavigation:NO];
+  NSURLResponse* response = [[NSHTTPURLResponse alloc]
+       initWithURL:[NSURL URLWithString:@(kTestURLString)]
+        statusCode:200
+       HTTPVersion:nil
+      headerFields:nil];
+  WKNavigationResponsePolicy policy = WKNavigationResponsePolicyAllow;
+  ASSERT_TRUE(CallDecidePolicyForNavigationResponseWithResponse(
+      response, /*for_main_frame=*/YES, /*can_show_mime_type=*/NO, &policy));
+  EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
+
+  // Verify that download task was created with POST method (crbug.com/.
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
+  DownloadTask* task =
+      download_delegate_.alive_download_tasks()[0].second.get();
+  ASSERT_TRUE(task);
+  EXPECT_TRUE(task->GetIndentifier());
+  EXPECT_NSEQ(@"POST", task->GetHttpMethod());
+}
+
 // Tests that webView:decidePolicyForNavigationResponse:decisionHandler: creates
 // the DownloadTask for NSURLResponse.
 TEST_P(CRWWebControllerResponseTest, DownloadWithNSURLResponse) {