Add the JobComplete callback and error/interrupt information

content/browser/background_fetch/background_fetch_data_manager_unittest.cc

Index: content/browser/background_fetch/background_fetch_data_manager_unittest.cc
diff --git a/content/browser/background_fetch/background_fetch_data_manager_unittest.cc b/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
index 031aabdc129b37718b972868f3969089971ac68c..374d213d6c931602d29475eb3e2d50d8ec8be8fe 100644
--- a/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
+++ b/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
@@ -10,6 +10,8 @@
 #include "content/browser/background_fetch/background_fetch_request_info.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/public/browser/download_interrupt_reasons.h"
+#include "content/public/browser/download_item.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -70,21 +72,35 @@ TEST_F(BackgroundFetchDataManagerTest, AddRequest) {
     const BackgroundFetchRequestInfo& request_info =
         job_data->GetNextBackgroundFetchRequestInfo();
     EXPECT_EQ(request_info.tag(), kTag);
+    EXPECT_EQ(request_info.state(),
+              DownloadItem::DownloadState::MAX_DOWNLOAD_STATE);

[Peter Beverloo, 2017/03/15 16:53:38]

What does this mean? What sort of state is "MAX_DOWNLOAD"? Do they mean
"Uninitialized" or similar with that?

[harkness, 2017/03/16 11:41:09]

I actually meant to send a mail to David to see if we can add a "NOT_STARTED" or
"UNINITIALIZED" state. Right now, the DownloadItem doesn't have any concept of
that. If a DownloadItem has been created at all, it is considerd to be
IN_PROGRESS. So I am using MAX_DOWNLOAD_STATE to represent a request that hasn't
been sent to the download system.

The alternative would be to also have a bool |started| on the RequestInfo on our
side and use the combination of the two to explain the state of the request.

[Peter Beverloo, 2017/03/16 12:37:09]

The download item notification also uses this as an uninitialized state value.
This is the sort of thing that I wouldn't send an e-mail about, but instead just
send a CL to add the value and see what they say :).

I won't block you on this since there's precedent, but please do consider
following up.

[harkness, 2017/03/17 11:14:11]

ok, I'll follow up on that.

+    EXPECT_EQ(request_info.interrupt_reason(),
+              DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_NONE);
   }
 
   // At this point, all the fetches have been started, but none finished.
   EXPECT_FALSE(job_data->HasRequestsRemaining());
   EXPECT_FALSE(job_data->IsComplete());
 
-  // Complete all buy one of the fetch requests.
+  // Complete all but one of the fetch requests.
   for (int i = 0; i < 9; i++) {
-    EXPECT_FALSE(
-        job_data->BackgroundFetchRequestInfoComplete(request_guids[i]));
+    EXPECT_FALSE(job_data->UpdateBackgroundFetchRequestState(
+        request_guids[i], DownloadItem::DownloadState::COMPLETE,
+        DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_NONE));
     EXPECT_FALSE(job_data->IsComplete());
   }
 
+  // Set the state of the last task to be interrupted. This should not complete
+  // the job.
+  EXPECT_FALSE(job_data->UpdateBackgroundFetchRequestState(
+      request_guids[9], DownloadItem::DownloadState::INTERRUPTED,
+      DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN));
+  EXPECT_FALSE(job_data->IsComplete());
+
   // Complete the final fetch request.
-  EXPECT_FALSE(job_data->BackgroundFetchRequestInfoComplete(request_guids[9]));
+  EXPECT_FALSE(job_data->UpdateBackgroundFetchRequestState(
+      request_guids[9], DownloadItem::DownloadState::COMPLETE,
+      DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_NONE));
   EXPECT_TRUE(job_data->IsComplete());
 }