Significantly cleans up the ImageWriter Operation class and subclasses.

chrome/browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc

Index: chrome/browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc
diff --git a/chrome/browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..484c8d2380b63d9aa06c76caad9ffc77214d2603
--- /dev/null
+++ b/chrome/browser/extensions/api/image_writer_private/write_from_url_operation_unittest.cc
@@ -0,0 +1,207 @@
+// Copyright 2014 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.
+
+#include "chrome/browser/extensions/api/image_writer_private/error_messages.h"
+#include "chrome/browser/extensions/api/image_writer_private/test_utils.h"
+#include "chrome/browser/extensions/api/image_writer_private/write_from_url_operation.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/test/net/url_request_prepackaged_interceptor.h"
+#include "net/url_request/url_fetcher.h"
+
+namespace extensions {
+namespace image_writer {
+
+namespace {
+
+using testing::_;
+using testing::AnyNumber;
+using testing::AtLeast;

[tbarzic, 2014/02/07 22:06:42]

AtLeast before AnyNumber (I think)

[Drew Haven, 2014/02/11 00:50:15]

t comes after n, so I think AnyNumber is before AtLeast.

[tbarzic, 2014/02/13 06:58:52]

right, I'm not sure why I looked at the third character here...

+using testing::Gt;
+using testing::Lt;
+
+const char kDownloadManagerKeyName[] = "download_manager";
+
+const GURL kTestImageUrl("http://localhost/test/image.zip");

[tbarzic, 2014/02/07 22:06:42]

make this const char[]
(only POD may be static)

[Drew Haven, 2014/02/11 00:50:15]

Done.

+
+typedef content::URLLocalHostRequestPrepackagedInterceptor GetInterceptor;
+
+// This class gives us a generic Operation with the ability to set or inspect
+// the current path to the image file.
+class OperationForTest : public WriteFromUrlOperation {
+ public:
+  OperationForTest(
+      base::WeakPtr<OperationManager> manager_,
+      const ExtensionId& extension_id,
+      net::URLRequestContextGetter* request_context,
+      GURL url,
+      const std::string& hash,
+      bool saveImageAsDownload,
+      const std::string& storage_unit_id)
+      : WriteFromUrlOperation(manager_,
+                              extension_id,
+                              request_context,
+                              url,
+                              hash,
+                              saveImageAsDownload,
+                              storage_unit_id) {}
+
+  virtual void Start() OVERRIDE {}
+
+  // Expose stages for testing.
+  void GetDownloadTarget(const base::Closure& continuation) {
+    WriteFromUrlOperation::GetDownloadTarget(continuation);
+  }
+
+  void Download(const base::Closure& continuation) {
+    WriteFromUrlOperation::Download(continuation);
+  }
+
+  void VerifyDownload(const base::Closure& continuation) {
+    WriteFromUrlOperation::VerifyDownload(continuation);
+  }
+
+  // Helpers to set-up state for intermediate stages.
+  void SetImagePath(const base::FilePath image_path) {
+    image_path_ = image_path;
+  }
+
+  base::FilePath GetImagePath() {
+    return image_path_;
+  }
+
+ private:
+  virtual ~OperationForTest() {};
+};
+
+class ImageWriterWriteFromUrlOperationTest : public ImageWriterUnitTestBase {
+ protected:
+  ImageWriterWriteFromUrlOperationTest() : manager_(&test_profile_) {}
+
+  virtual void SetUp() OVERRIDE {
+    ImageWriterUnitTestBase::SetUp();
+
+    // Turn on interception and set up our dummy file.
+    net::URLFetcher::SetEnableInterceptionForTests(true);
+    get_interceptor_.reset(new GetInterceptor());
+    get_interceptor_->SetResponse(kTestImageUrl, test_image_path_);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    ImageWriterUnitTestBase::TearDown();
+
+    // Remember to turn off global interception.
+    net::URLFetcher::SetEnableInterceptionForTests(false);
+  }
+
+  scoped_refptr<OperationForTest> CreateOperation(const GURL& url,
+                                                  const std::string& hash,
+                                                  bool saveAsDownload) {
+    scoped_refptr<OperationForTest> operation(
+        new OperationForTest(manager_.AsWeakPtr(),
+                             kDummyExtensionId,
+                             test_profile_.GetRequestContext(),
+                             url,
+                             hash,
+                             saveAsDownload,
+                             test_device_path_.AsUTF8Unsafe()));
+    return operation;
+  }
+
+
+  TestingProfile test_profile_;
+  scoped_ptr<GetInterceptor> get_interceptor_;
+
+  MockOperationManager manager_;
+};
+
+TEST_F(ImageWriterWriteFromUrlOperationTest, SelectTargetWithoutExtension) {
+  scoped_refptr<OperationForTest> operation =
+      CreateOperation(GURL("http://localhost/foo/bar"), "", false);
+
+  operation->GetDownloadTarget(base::Bind(&base::DoNothing));
+
+  EXPECT_EQ("bar", operation->GetImagePath().BaseName().value());
+}
+
+TEST_F(ImageWriterWriteFromUrlOperationTest, SelectTargetWithExtension) {
+  scoped_refptr<OperationForTest> operation =
+      CreateOperation(GURL("http://localhost/foo/bar.zip"), "", false);
+
+  operation->GetDownloadTarget(base::Bind(&base::DoNothing));
+
+  EXPECT_EQ("bar.zip", operation->GetImagePath().BaseName().value());
+}
+
+TEST_F(ImageWriterWriteFromUrlOperationTest, DownloadFile) {
+  // This test actually triggers the URL fetch code, which will drain the
+  // message queues while waiting for IO, thus we have to run until the
+  // operation completes.
+  base::RunLoop runloop;
+  base::Closure quit_closure = runloop.QuitClosure();
+  base::FilePath download_target_path;
+  scoped_refptr<OperationForTest> operation =
+      CreateOperation(kTestImageUrl, "", false);
+
+  EXPECT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(),
+                                             &download_target_path));
+  operation->SetImagePath(download_target_path);
+
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_DOWNLOAD,
+                                  _)).Times(AtLeast(1));
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_DOWNLOAD,
+                                  0)).Times(AnyNumber());
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_DOWNLOAD,
+                                  100)).Times(AnyNumber());
+
+  content::BrowserThread::PostTask(content::BrowserThread::FILE,
+                                   FROM_HERE,
+                                   base::Bind(&OperationForTest::Download,
+                                              operation,
+                                              quit_closure));
+
+  runloop.Run();
+
+  EXPECT_TRUE(base::ContentsEqual(test_image_path_, operation->GetImagePath()));
+
+  EXPECT_EQ(1, get_interceptor_->GetHitCount());
+}
+
+TEST_F(ImageWriterWriteFromUrlOperationTest, VerifyFile) {
+  scoped_ptr<char[]> data_buffer(new char[kTestFileSize]);
+  base::ReadFile(test_image_path_, data_buffer.get(), kTestFileSize);
+  base::MD5Digest expected_digest;
+  base::MD5Sum(data_buffer.get(), kTestFileSize, &expected_digest);
+  std::string expected_hash = base::MD5DigestToBase16(expected_digest);
+
+  scoped_refptr<OperationForTest> operation =
+      CreateOperation(GURL(""), expected_hash, false);
+
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_VERIFYDOWNLOAD,
+                                  _)).Times(AtLeast(1));
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_VERIFYDOWNLOAD,

[tbarzic, 2014/02/07 22:06:42]

nit: alignment

[Drew Haven, 2014/02/11 00:50:15]

Done. Did a `git cl format`.

+                                  0)).Times(AtLeast(1));
+  EXPECT_CALL(manager_, OnProgress(kDummyExtensionId,
+                                  image_writer_api::STAGE_VERIFYDOWNLOAD,
+                                  100)).Times(AtLeast(1));
+
+  operation->SetImagePath(test_image_path_);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&OperationForTest::VerifyDownload,
+                 operation,
+                 base::Bind(&base::DoNothing)));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+} // namespace
+
+} // namespace image_writer
+} // namespace extensions