Add browser test for CustomizationWallpaperDownloader.

chrome/browser/chromeos/customization_wallpaper_downloader_browsertest.cc

Index: chrome/browser/chromeos/customization_wallpaper_downloader_browsertest.cc
diff --git a/chrome/browser/chromeos/customization_wallpaper_downloader_browsertest.cc b/chrome/browser/chromeos/customization_wallpaper_downloader_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fc661079470d65445ef8cd4e6deb89040096465f
--- /dev/null
+++ b/chrome/browser/chromeos/customization_wallpaper_downloader_browsertest.cc
@@ -0,0 +1,485 @@
+// 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/chromeos/customization_wallpaper_downloader.h"

[Nikita (slow), 2014/04/29 08:56:03]

nit: This include should go with the others.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+
+#include <vector>
+
+#include "ash/ash_switches.h"
+#include "ash/desktop_background/desktop_background_controller.h"
+#include "ash/shell.h"
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/run_loop.h"
+#include "base/time/time.h"
+#include "chrome/browser/chromeos/customization_document.h"
+#include "chrome/browser/chromeos/login/wallpaper_manager.h"
+#include "chrome/browser/google/google_url_tracker.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chromeos/chromeos_switches.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_fetcher_impl.h"
+#include "testing/gmock/include/gmock/gmock.h"

[Daniel Erat, 2014/04/29 16:41:16]

are you actually using this? it doesn't look like it

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/codec/jpeg_codec.h"
+#include "ui/gfx/point.h"
+#include "ui/gfx/rect.h"
+
+namespace chromeos {
+
+namespace {
+
+#define OEM_WALLPAPER_URL "http://somedomain.com/image.png"

[Daniel Erat, 2014/04/29 16:41:16]

i think it'd be preferable to just hardcode this in both the constant and the
string literal instead of using a macro here

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+const char kOEMWallpaperURL[] = OEM_WALLPAPER_URL;
+
+const char kServicesManifest[] =
+    "{"
+    "  \"version\": \"1.0\","
+    "  \"default_wallpaper\": \"" OEM_WALLPAPER_URL "\",\n"
+    "  \"default_apps\": [\n"
+    "    \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\n"
+    "    \"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\"\n"
+    "  ],\n"
+    "  \"localized_content\": {\n"
+    "    \"en-US\": {\n"
+    "      \"default_apps_folder_name\": \"EN-US OEM Name\"\n"
+    "    },\n"
+    "    \"en\": {\n"
+    "      \"default_apps_folder_name\": \"EN OEM Name\"\n"
+    "    },\n"
+    "    \"default\": {\n"
+    "      \"default_apps_folder_name\": \"Default OEM Name\"\n"
+    "    }\n"
+    "  }\n"
+    "}";
+
+#undef OEM_WALLPAPER_URL
+
+// Expected minimal wallpaper download retry interval in seconds.
+const size_t kMinOEMWallpaperRetryIntervalSec = 10;
+
+bool CreateJPEGImage(int width,

[Daniel Erat, 2014/04/29 16:41:16]

please don't just copy-and-paste all of this code from
wallpaper_manager_browsertest.cc. move it to a shared location.

the same goes for all the other stuff that's being duplicated
(CreateCmdlineWallpapers looks like a copy of WriteWallpapers, WriteJPEGFile,
etc.)

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+                     int height,
+                     SkColor color,
+                     std::vector<unsigned char>* output) {
+  SkBitmap bitmap;
+  bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height, 0);
+  bitmap.allocPixels();
+  bitmap.eraseColor(color);
+
+  const int kQuality = 80;
+  if (!gfx::JPEGCodec::Encode(
+          static_cast<const unsigned char*>(bitmap.getPixels()),
+          gfx::JPEGCodec::FORMAT_SkBitmap,
+          width,
+          height,
+          bitmap.rowBytes(),
+          kQuality,
+          output)) {
+    LOG(ERROR) << "Unable to encode " << width << "x" << height << " bitmap";
+    return false;
+  }
+  return true;
+}
+
+}  // namespace
+
+class TestObserver : public WallpaperManager::Observer {

[Nikita (slow), 2014/04/29 08:56:03]

Please move to the unnamed namespace.

[Nikita (slow), 2014/04/29 08:56:03]

nit: TestWallpaperObserver.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+ public:
+  explicit TestObserver(WallpaperManager* wallpaper_manager)
+      : finished_(false), wallpaper_manager_(wallpaper_manager) {
+    DCHECK(wallpaper_manager_);
+    wallpaper_manager_->AddObserver(this);
+  }
+
+  virtual ~TestObserver() {
+    wallpaper_manager_->RemoveObserver(this);
+  }
+
+  virtual void OnWallpaperAnimationFinished(const std::string&) OVERRIDE {
+    finished_ = true;
+    base::MessageLoop::current()->Quit();
+  }
+
+  void WaitForWallpaperAnimationFinished() {
+    while (!finished_) {

[Nikita (slow), 2014/04/29 08:56:03]

nit: drop {}

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+      base::RunLoop().Run();
+    }
+  }
+
+ private:
+  bool finished_;
+  WallpaperManager* wallpaper_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+class CustomizationWallpaperDownloaderBrowserTest
+    : public InProcessBrowserTest {
+ public:
+  CustomizationWallpaperDownloaderBrowserTest()
+      : controller_(NULL), local_state_(NULL) {
+  }
+
+  virtual ~CustomizationWallpaperDownloaderBrowserTest() {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    controller_ = ash::Shell::GetInstance()->desktop_background_controller();
+    local_state_ = g_browser_process->local_state();
+  }
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(chromeos::switches::kLoginManager);
+    command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "user");
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE { controller_ = NULL; }
+
+  void WaitAsyncWallpaperLoadFinished() {
+    base::RunLoop().RunUntilIdle();
+    while (WallpaperManager::Get()->loading_.size()) {
+      base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));

[Nikita (slow), 2014/04/29 08:56:03]

Potential source of flakiness.

Please eliminate sleep approach. This seems like a workaround that will break on
some platform/configuration etc.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+      base::RunLoop().RunUntilIdle();
+    }
+  }
+
+ protected:
+  // Colors used for different default wallpapers by
+  // CreateCmdlineWallpapers().
+  static const SkColor kLargeDefaultWallpaperColor = SK_ColorRED;
+  static const SkColor kSmallDefaultWallpaperColor = SK_ColorGREEN;
+  static const SkColor kLargeGuestWallpaperColor = SK_ColorBLUE;
+  static const SkColor kSmallGuestWallpaperColor = SK_ColorYELLOW;
+
+  // A color of "remote OEM wallpaper". Specifically chosen to not
+  // conflict with any of the default wallpaper colors.
+  static const SkColor kOEMWallpaperColor = SK_ColorMAGENTA;
+
+  // Dimension used for width and height of default wallpaper images. A
+  // small value is used to minimize the amount of time spent compressing
+  // and writing images.
+  static const int kWallpaperSize = 2;
+
+  // Return custom wallpaper path. Create directory if not exist.
+  base::FilePath GetCustomWallpaperPath(const char* sub_dir,
+                                        const std::string& username_hash,
+                                        const std::string& id) {
+    base::FilePath wallpaper_path =
+        WallpaperManager::Get()->GetCustomWallpaperPath(
+            sub_dir, username_hash, id);
+    if (!base::DirectoryExists(wallpaper_path.DirName()))
+      base::CreateDirectory(wallpaper_path.DirName());
+
+    return wallpaper_path;
+  }
+
+  // Creates a test image of given size

[Nikita (slow), 2014/04/29 08:56:03]

nit: dot at the end.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+  gfx::ImageSkia CreateTestImage(int width, int height, SkColor color) {

[Nikita (slow), 2014/04/29 08:56:03]

Can be moved to unnamed namespace?

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+    SkBitmap bitmap;
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height);
+    bitmap.allocPixels();
+    bitmap.eraseColor(color);
+    return gfx::ImageSkia::CreateFrom1xBitmap(bitmap);
+  }
+
+  // Writes a JPEG image of the specified size and color to |path|. Returns
+  // true on success.
+  bool WriteJPEGFile(const base::FilePath& path,

[Nikita (slow), 2014/04/29 08:56:03]

Can be moved to unnamed namespace?

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+                     int width,
+                     int height,
+                     SkColor color) {
+    std::vector<unsigned char> output;
+    if (!CreateJPEGImage(width, height, color, &output))
+      return false;
+
+    size_t bytes_written = base::WriteFile(
+        path, reinterpret_cast<const char*>(&output[0]), output.size());
+    if (bytes_written != output.size()) {
+      LOG(ERROR) << "Wrote " << bytes_written << " byte(s) instead of "
+                 << output.size() << " to " << path.value();
+      return false;
+    }
+    return true;
+  }
+
+  // Initializes default wallpaper paths "*default_*file" and writes JPEG
+  // wallpaper images to them.
+  // Only needs to be called (once) by tests that want to test loading of
+  // default wallpapers.
+  void CreateCmdlineWallpapers() {
+    cmdline_wallpaper_dir_.reset(new base::ScopedTempDir);
+    ASSERT_TRUE(cmdline_wallpaper_dir_->CreateUniqueTempDir());
+
+    std::vector<std::string> options;
+    options.push_back(std::string("WM_Test_cmdline"));
+    const base::FilePath small_file =
+        cmdline_wallpaper_dir_->path().Append(FILE_PATH_LITERAL("small.jpg"));
+    options.push_back(std::string("--") +
+                      ash::switches::kAshDefaultWallpaperSmall + "=" +
+                      small_file.value());
+    const base::FilePath large_file =
+        cmdline_wallpaper_dir_->path().Append(FILE_PATH_LITERAL("large.jpg"));
+    options.push_back(std::string("--") +
+                      ash::switches::kAshDefaultWallpaperLarge + "=" +
+                      large_file.value());
+
+    const base::FilePath guest_small_file =
+        cmdline_wallpaper_dir_->path().Append(
+            FILE_PATH_LITERAL("guest_small.jpg"));
+    options.push_back(std::string("--") +
+                      ash::switches::kAshGuestWallpaperSmall + "=" +
+                      guest_small_file.value());
+    const base::FilePath guest_large_file =
+        cmdline_wallpaper_dir_->path().Append(
+            FILE_PATH_LITERAL("guest_large.jpg"));
+    options.push_back(std::string("--") +
+                      ash::switches::kAshGuestWallpaperLarge + "=" +
+                      guest_large_file.value());
+
+    ASSERT_TRUE(WriteJPEGFile(small_file,
+                              kWallpaperSize,
+                              kWallpaperSize,
+                              kSmallDefaultWallpaperColor));
+    ASSERT_TRUE(WriteJPEGFile(large_file,
+                              kWallpaperSize,
+                              kWallpaperSize,
+                              kLargeDefaultWallpaperColor));
+
+    ASSERT_TRUE(WriteJPEGFile(guest_small_file,
+                              kWallpaperSize,
+                              kWallpaperSize,
+                              kSmallGuestWallpaperColor));
+    ASSERT_TRUE(WriteJPEGFile(guest_large_file,
+                              kWallpaperSize,
+                              kWallpaperSize,
+                              kLargeGuestWallpaperColor));
+
+    wallpaper_manager_command_line_.reset(new base::CommandLine(options));
+    WallpaperManager::Get()->SetCommandLineForTesting(
+        wallpaper_manager_command_line_.get());
+  }
+
+  // Returns true if the color at the center of |image| is close to
+  // |expected_color|. (The center is used so small wallpaper images can be
+  // used.)
+  bool ImageIsNearColor(gfx::ImageSkia image, SkColor expected_color) {

[Nikita (slow), 2014/04/29 08:56:03]

Can be moved to unnamed namespace?

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+    if (image.size().IsEmpty()) {
+      LOG(ERROR) << "Image is empty";
+      return false;
+    }
+
+    const SkBitmap* bitmap = image.bitmap();
+    if (!bitmap) {
+      LOG(ERROR) << "Unable to get bitmap from image";
+      return false;
+    }
+
+    bitmap->lockPixels();
+    gfx::Point center = gfx::Rect(image.size()).CenterPoint();
+    SkColor image_color = bitmap->getColor(center.x(), center.y());
+    bitmap->unlockPixels();
+
+    const int kDiff = 3;
+    if (std::abs(static_cast<int>(SkColorGetA(image_color)) -
+                 static_cast<int>(SkColorGetA(expected_color))) > kDiff ||
+        std::abs(static_cast<int>(SkColorGetR(image_color)) -
+                 static_cast<int>(SkColorGetR(expected_color))) > kDiff ||
+        std::abs(static_cast<int>(SkColorGetG(image_color)) -
+                 static_cast<int>(SkColorGetG(expected_color))) > kDiff ||
+        std::abs(static_cast<int>(SkColorGetB(image_color)) -
+                 static_cast<int>(SkColorGetB(expected_color))) > kDiff) {
+      LOG(ERROR) << "Expected color near 0x" << std::hex << expected_color
+                 << " but got 0x" << image_color;
+      return false;
+    }
+
+    return true;
+  }
+  bool LoadManifestFromString(

[Nikita (slow), 2014/04/29 08:56:03]

Eliminate this method.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+      chromeos::ServicesCustomizationDocument* customization,
+      const std::string& manifest) {
+    return customization->LoadManifestFromString(manifest);
+  }
+
+  ash::DesktopBackgroundController* controller_;
+  PrefService* local_state_;
+  scoped_ptr<base::CommandLine> wallpaper_manager_command_line_;
+
+  // Directory created by CreateCmdlineWallpapersAndSetFlags() to store default
+  // wallpaper images.
+  scoped_ptr<base::ScopedTempDir> cmdline_wallpaper_dir_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CustomizationWallpaperDownloaderBrowserTest);
+};
+
+class WallpaperImageURLFetcherCallback {

[Nikita (slow), 2014/04/29 08:56:03]

Please move to the unnamed namespace and add short comment for this class.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+ public:
+  WallpaperImageURLFetcherCallback(
+      const GURL& url,
+      const size_t require_retries,
+      const std::vector<unsigned char>& jpeg_data_raw)
+      : url_(url), require_retries_(require_retries), factory_(NULL) {
+    jpeg_data_.resize(jpeg_data_raw.size());
+    std::copy(jpeg_data_raw.begin(), jpeg_data_raw.end(), jpeg_data_.begin());
+  }
+
+  scoped_ptr<net::FakeURLFetcher> CreateURLFetcher(
+      const GURL& url,
+      net::URLFetcherDelegate* d,
+      const std::string& response_data,
+      net::HttpStatusCode response_code,
+      net::URLRequestStatus::Status status) {
+    attempts_.push_back(base::Time::Now());
+    if (attempts_.size() > 1) {
+      const base::TimeDelta passed =
+          attempts_.back() - attempts_[attempts_.size() - 2];
+      EXPECT_GE(passed,

[Nikita (slow), 2014/04/29 08:56:03]

I don't think you should check for specific timings and timeouts in tests.

Our overall approach is to set such timeouts to zero i.e. add new test method
like SetZeroWallpaperFetchTimeoutForTesting()

[Alexander Alekseev, 2014/04/29 21:42:40]

This checks that retry is not sent too early to prevent server from overloading.
So this is an intended check.

+                base::TimeDelta::FromSeconds(kMinOEMWallpaperRetryIntervalSec))
+          << "Retry too fast. Actual interval " << passed.InSecondsF()
+          << " seconds, but expected at least "
+          << kMinOEMWallpaperRetryIntervalSec << " seconds.";
+    }
+    if (attempts_.size() > require_retries_) {
+      response_code = net::HTTP_OK;
+      status = net::URLRequestStatus::SUCCESS;
+      factory_->SetFakeResponse(url, response_data, response_code, status);
+    }
+    scoped_ptr<net::FakeURLFetcher> fetcher(
+        new net::FakeURLFetcher(url, d, response_data, response_code, status));
+    scoped_refptr<net::HttpResponseHeaders> download_headers =
+        new net::HttpResponseHeaders("");

[Nikita (slow), 2014/04/29 08:56:03]

nit: std::string

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+    download_headers->AddHeader("Content-Type: image/jpeg");
+    fetcher->set_response_headers(download_headers);
+    return fetcher.Pass();
+  }
+
+  void Initialize(net::FakeURLFetcherFactory* factory) {
+    factory_ = factory;
+    factory_->SetFakeResponse(url_,
+                              jpeg_data_,
+                              net::HTTP_INTERNAL_SERVER_ERROR,
+                              net::URLRequestStatus::FAILED);
+  }
+
+  size_t attempts() const { return attempts_.size(); }
+
+ private:
+  const GURL url_;
+  // Respond with OK on required retry attempt.
+  const size_t require_retries_;
+  net::FakeURLFetcherFactory* factory_;
+  std::vector<base::Time> attempts_;
+  std::string jpeg_data_;
+
+  DISALLOW_COPY_AND_ASSIGN(WallpaperImageURLFetcherCallback);
+};
+
+class WallpaperImageFetcherFactory {

[Nikita (slow), 2014/04/29 08:56:03]

Please move to the unnamed namespace and add short comment for this class.

[Alexander Alekseev, 2014/04/29 21:42:40]

Done.

+ public:
+  WallpaperImageFetcherFactory(const GURL& url,
+                               int width,
+                               int height,
+                               SkColor color,
+                               const size_t require_retries) {
+    // ASSERT_TRUE() cannot be directly used in constructor.
+    Initialize(url, width, height, color, require_retries);
+  }
+
+  ~WallpaperImageFetcherFactory() {
+    fetcher_factory_.reset();
+    net::URLFetcherImpl::set_factory(fallback_fetcher_factory_.get());
+    fallback_fetcher_factory_.reset();
+  }
+
+  size_t attempts() const { return url_callback_->attempts(); }
+
+ private:
+  void Initialize(const GURL& url,
+                  int width,
+                  int height,
+                  SkColor color,
+                  const size_t require_retries) {
+    std::vector<unsigned char> oem_wallpaper_;
+    ASSERT_TRUE(CreateJPEGImage(width, height, color, &oem_wallpaper_));
+
+    url_callback_.reset(new WallpaperImageURLFetcherCallback(
+        url, require_retries, oem_wallpaper_));
+    fallback_fetcher_factory_.reset(new net::TestURLFetcherFactory);
+    net::URLFetcherImpl::set_factory(NULL);
+    fetcher_factory_.reset(new net::FakeURLFetcherFactory(
+        fallback_fetcher_factory_.get(),
+        base::Bind(&WallpaperImageURLFetcherCallback::CreateURLFetcher,
+                   base::Unretained(url_callback_.get()))));
+    url_callback_->Initialize(fetcher_factory_.get());
+  }
+
+  scoped_ptr<WallpaperImageURLFetcherCallback> url_callback_;
+
+  // Use a test factory as a fallback so we don't have to deal with other
+  // requests.
+  scoped_ptr<net::TestURLFetcherFactory> fallback_fetcher_factory_;
+  scoped_ptr<net::FakeURLFetcherFactory> fetcher_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(WallpaperImageFetcherFactory);
+};
+
+IN_PROC_BROWSER_TEST_F(CustomizationWallpaperDownloaderBrowserTest,
+                       OEMWallpaperIsPresent) {
+  CreateCmdlineWallpapers();
+  WallpaperManager::Get()->SetDefaultWallpaperNow(std::string());
+  WaitAsyncWallpaperLoadFinished();
+  EXPECT_TRUE(ImageIsNearColor(controller_->GetWallpaper(),
+                               kSmallDefaultWallpaperColor));
+
+  WallpaperImageFetcherFactory url_factory(GURL(kOEMWallpaperURL),
+                                           kWallpaperSize,
+                                           kWallpaperSize,
+                                           kOEMWallpaperColor,
+                                           0 /* require_retries */);
+
+  TestObserver observer(WallpaperManager::Get());
+  chromeos::ServicesCustomizationDocument* customization =
+      chromeos::ServicesCustomizationDocument::GetInstance();
+  EXPECT_TRUE(
+      LoadManifestFromString(customization, std::string(kServicesManifest)));
+
+  observer.WaitForWallpaperAnimationFinished();
+  EXPECT_TRUE(
+      ImageIsNearColor(controller_->GetWallpaper(), kOEMWallpaperColor));
+  EXPECT_EQ(size_t(1), url_factory.attempts());
+}
+
+IN_PROC_BROWSER_TEST_F(CustomizationWallpaperDownloaderBrowserTest,
+                       OEMWallpaperRetryFetch) {
+  CreateCmdlineWallpapers();
+  WallpaperManager::Get()->SetDefaultWallpaperNow(std::string());
+  WaitAsyncWallpaperLoadFinished();
+  EXPECT_TRUE(ImageIsNearColor(controller_->GetWallpaper(),
+                               kSmallDefaultWallpaperColor));
+
+  WallpaperImageFetcherFactory url_factory(GURL(kOEMWallpaperURL),
+                                           kWallpaperSize,
+                                           kWallpaperSize,
+                                           kOEMWallpaperColor,
+                                           1 /* require_retries */);
+
+  TestObserver observer(WallpaperManager::Get());
+  chromeos::ServicesCustomizationDocument* customization =
+      chromeos::ServicesCustomizationDocument::GetInstance();
+  EXPECT_TRUE(
+      LoadManifestFromString(customization, std::string(kServicesManifest)));
+
+  observer.WaitForWallpaperAnimationFinished();
+  EXPECT_TRUE(
+      ImageIsNearColor(controller_->GetWallpaper(), kOEMWallpaperColor));
+
+  EXPECT_EQ(size_t(2), url_factory.attempts());
+}
+
+}  // namespace chromeos