Deflakify DownloadExtensionTest by not depending on the icon loader

chrome/browser/extensions/api/downloads/downloads_api_unittest.cc

 // Copyright (c) 2012 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 <algorithm>
 
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop.h"
@@ skipping 27 matching lines @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/page_transition_types.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/test/net/url_request_slow_download_job.h"
 #include "net/base/data_url.h"
 #include "net/base/net_util.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_factory.h"
-#include "ui/gfx/codec/png_codec.h"
 #include "webkit/blob/blob_data.h"
 #include "webkit/blob/blob_storage_controller.h"
 #include "webkit/blob/blob_url_request_job.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_operation_interface.h"
 #include "webkit/fileapi/file_system_url.h"
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::DownloadItem;
@@ skipping 441 matching lines @@
     SetUpExtensionFunction(function);
     scoped_ptr<base::Value> result(RunFunctionAndReturnResult(function, args));
     EXPECT_TRUE(result.get());
     return result.get() && result->GetAsString(result_string);
   }
 
   std::string DownloadItemIdAsArgList(const DownloadItem* download_item) {
     return base::StringPrintf("[%d]", download_item->GetId());
   }
 
-  // Checks if a data URL encoded image is a PNG of a given size.
-  void ExpectDataURLIsPNGWithSize(const std::string& url, int expected_size) {
-    std::string mime_type;
-    std::string charset;
-    std::string data;
-    EXPECT_FALSE(url.empty());
-    EXPECT_TRUE(net::DataURL::Parse(GURL(url), &mime_type, &charset, &data));
-    EXPECT_STREQ("image/png", mime_type.c_str());
-    EXPECT_FALSE(data.empty());
-
-    if (data.empty())
-      return;
-
-    int width = -1, height = -1;
-    std::vector<unsigned char> decoded_data;
-    EXPECT_TRUE(gfx::PNGCodec::Decode(
-        reinterpret_cast<const unsigned char*>(data.c_str()), data.length(),
-        gfx::PNGCodec::FORMAT_RGBA, &decoded_data,
-        &width, &height));
-    EXPECT_EQ(expected_size, width);
-    EXPECT_EQ(expected_size, height);
-  }
-
   const FilePath& downloads_directory() {
     return downloads_directory_.path();
   }
 
   DownloadsEventsListener* events_listener() { return events_listener_.get(); }
 
  private:
   void SetUpExtensionFunction(UIThreadExtensionFunction* function) {
     if (extension_) {
       // Recreate the tab each time for insulation.
@@ skipping 347 matching lines @@
   EXPECT_STREQ(download_extension_errors::kInvalidOperationError,
                error.c_str());
 
   // Calling resume on a non-existent download yields kInvalidOperationError
   error = RunFunctionAndReturnError(
       new DownloadsResumeFunction(), "[-42]");
   EXPECT_STREQ(download_extension_errors::kInvalidOperationError,
                error.c_str());
 }
 
+UIThreadExtensionFunction* MockedGetFileIconFunction(
+    const FilePath& expected_path,
+    IconLoader::IconSize icon_size,
+    const std::string& response) {
+  scoped_refptr<DownloadsGetFileIconFunction> function(
+      new DownloadsGetFileIconFunction());
+  function->SetIconExtractorForTesting(new MockIconExtractorImpl(
+      expected_path, icon_size, response));
+  return function.release();
+}
+
 // Test downloads.getFileIcon() on in-progress, finished, cancelled and deleted
 // download items.
 // The test fails under ASan, see http://crbug.com/138211

[asanka, 2012/08/17 20:35:36]

You should re-enable these tests. The failure mode is the same.

[benjhayden, 2012/08/17 21:35:57]

Done.

 #if defined(ADDRESS_SANITIZER)
 #define MAYBE_DownloadExtensionTest_FileIcon_Active \
     DISABLED_DownloadExtensionTest_FileIcon_Active
 #else
 #define MAYBE_DownloadExtensionTest_FileIcon_Active \
     DownloadExtensionTest_FileIcon_Active
 #endif
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
     MAYBE_DownloadExtensionTest_FileIcon_Active) {
   DownloadItem* download_item = CreateSlowTestDownload();
   ASSERT_TRUE(download_item);
+  std::string args32(base::StringPrintf("[%d, {\"size\": 32}]",
+                     download_item->GetId()));
+  std::string result_string;
 
   // Get the icon for the in-progress download.  This call should succeed even
   // if the file type isn't registered.
-  std::string args = base::StringPrintf("[%d, {}]", download_item->GetId());
-  std::string result_string;
-  EXPECT_TRUE(RunFunctionAndReturnString(new DownloadsGetFileIconFunction(),
-                                         args, &result_string));
-
-  // Note: We are checking if the result is a Data URL that has encoded
-  // image/png data for an icon of a specific size. Of these, only the icon size
-  // is specified in the API contract. The image type (image/png) and URL type
-  // (Data) are implementation details.
-
-  // The default size for icons returned by getFileIcon() is 32x32.
-  ExpectDataURLIsPNGWithSize(result_string, 32);
-
   // Test whether the correct path is being pased into the icon extractor.
-  FilePath expected_path(download_item->GetTargetFilePath());
-  scoped_refptr<DownloadsGetFileIconFunction> function(
-      new DownloadsGetFileIconFunction());
-  function->SetIconExtractorForTesting(new MockIconExtractorImpl(
-      expected_path, IconLoader::NORMAL, "foo"));
-  EXPECT_TRUE(RunFunctionAndReturnString(function.release(), args,
-                                         &result_string));
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
+      base::StringPrintf("[%d, {}]", download_item->GetId()), &result_string));
 
   // Now try a 16x16 icon.
-  args = base::StringPrintf("[%d, {\"size\": 16}]", download_item->GetId());
-  EXPECT_TRUE(RunFunctionAndReturnString(new DownloadsGetFileIconFunction(),
-                                         args, &result_string));
-  ExpectDataURLIsPNGWithSize(result_string, 16);
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::SMALL, "foo"),
+      base::StringPrintf("[%d, {\"size\": 16}]", download_item->GetId()),
+      &result_string));
 
   // Explicitly asking for 32x32 should give us a 32x32 icon.
-  args = base::StringPrintf("[%d, {\"size\": 32}]", download_item->GetId());
-  EXPECT_TRUE(RunFunctionAndReturnString(new DownloadsGetFileIconFunction(),
-                                         args, &result_string));
-  ExpectDataURLIsPNGWithSize(result_string, 32);
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
+      args32, &result_string));
 
   // Finish the download and try again.
   FinishPendingSlowDownloads();
   EXPECT_EQ(DownloadItem::COMPLETE, download_item->GetState());
-  EXPECT_TRUE(RunFunctionAndReturnString(new DownloadsGetFileIconFunction(),
-                                         args, &result_string));
-  ExpectDataURLIsPNGWithSize(result_string, 32);
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
+      args32, &result_string));
 
   // Check the path passed to the icon extractor post-completion.
-  function = new DownloadsGetFileIconFunction();
-  function->SetIconExtractorForTesting(new MockIconExtractorImpl(
-      expected_path, IconLoader::NORMAL, "foo"));
-  EXPECT_TRUE(RunFunctionAndReturnString(function.release(), args,
-                                         &result_string));
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
+      args32, &result_string));
 
   // Now create another download.
   download_item = CreateSlowTestDownload();
+  args32 = base::StringPrintf("[%d, {\"size\": 32}]", download_item->GetId());
   ASSERT_TRUE(download_item);
-  expected_path = download_item->GetTargetFilePath();
 
   // Cancel the download. As long as the download has a target path, we should
   // be able to query the file icon.
   download_item->Cancel(true);
   // Let cleanup complete on the FILE thread.
   content::RunAllPendingInMessageLoop(BrowserThread::FILE);
-  args = base::StringPrintf("[%d, {\"size\": 32}]", download_item->GetId());
-  EXPECT_TRUE(RunFunctionAndReturnString(new DownloadsGetFileIconFunction(),
-                                         args, &result_string));
-  ExpectDataURLIsPNGWithSize(result_string, 32);
-
   // Check the path passed to the icon extractor post-cancellation.
-  function = new DownloadsGetFileIconFunction();
-  function->SetIconExtractorForTesting(new MockIconExtractorImpl(
-      expected_path, IconLoader::NORMAL, "foo"));
-  EXPECT_TRUE(RunFunctionAndReturnString(function.release(), args,
-                                         &result_string));
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          download_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
+      args32,
+      &result_string));
 
   // Simulate an error during icon load by invoking the mock with an empty
   // result string.
-  function = new DownloadsGetFileIconFunction();
-  function->SetIconExtractorForTesting(new MockIconExtractorImpl(
-      expected_path, IconLoader::NORMAL, ""));
-  std::string error = RunFunctionAndReturnError(function.release(), args);
+  std::string error = RunFunctionAndReturnError(MockedGetFileIconFunction(
+        download_item->GetTargetFilePath(), IconLoader::NORMAL, ""),
+      args32);
   EXPECT_STREQ(download_extension_errors::kIconNotFoundError,
                error.c_str());
 
   // Once the download item is deleted, we should return kInvalidOperationError.
   download_item->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
-  error = RunFunctionAndReturnError(new DownloadsGetFileIconFunction(), args);
+  download_item = NULL;
+  error = RunFunctionAndReturnError(new DownloadsGetFileIconFunction(), args32);
   EXPECT_STREQ(download_extension_errors::kInvalidOperationError,
                error.c_str());
 }
 
 // Test that we can acquire file icons for history downloads regardless of
 // whether they exist or not.  If the file doesn't exist we should receive a
 // generic icon from the OS/toolkit that may or may not be specific to the file
 // type.
 // The test fails under ASan, see http://crbug.com/138211
 #if defined(ADDRESS_SANITIZER)
@@ skipping 20 matching lines @@
   FilePath real_path = all_downloads[0]->GetFullPath();
   FilePath fake_path = all_downloads[1]->GetFullPath();
 
   EXPECT_EQ(0, file_util::WriteFile(real_path, "", 0));
   ASSERT_TRUE(file_util::PathExists(real_path));
   ASSERT_FALSE(file_util::PathExists(fake_path));
 
   for (DownloadManager::DownloadVector::iterator iter = all_downloads.begin();
        iter != all_downloads.end();
        ++iter) {
-    std::string args(base::StringPrintf("[%d, {\"size\": 32}]",
-                                        (*iter)->GetId()));
     std::string result_string;
-    EXPECT_TRUE(RunFunctionAndReturnString(
-        new DownloadsGetFileIconFunction(), args, &result_string));
-    // Note: We are checking if the result is a Data URL that has encoded
-    // image/png data for an icon of a specific size. Of these, only the icon
-    // size is specified in the API contract. The image type (image/png) and URL
-    // type (Data) are implementation details.
-    ExpectDataURLIsPNGWithSize(result_string, 32);
-
     // Use a MockIconExtractorImpl to test if the correct path is being passed
     // into the DownloadFileIconExtractor.
-    scoped_refptr<DownloadsGetFileIconFunction> function(
-        new DownloadsGetFileIconFunction());
-    function->SetIconExtractorForTesting(new MockIconExtractorImpl(
-        (*iter)->GetFullPath(), IconLoader::NORMAL, "hello"));
-    EXPECT_TRUE(RunFunctionAndReturnString(function.release(), args,
-                                           &result_string));
+    EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+            (*iter)->GetFullPath(), IconLoader::NORMAL, "hello"),
+        base::StringPrintf("[%d, {\"size\": 32}]", (*iter)->GetId()),
+        &result_string));
     EXPECT_STREQ("hello", result_string.c_str());
   }
 
   // The temporary files should be cleaned up when the ScopedTempDir is removed.
 }
 
 // Test passing the empty query to search().
 IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
     DownloadExtensionTest_SearchEmptyQuery) {
   ScopedCancellingItem item(CreateSlowTestDownload());
@@ skipping 336 matching lines @@
       new DownloadsGetFileIconFunction(),
       base::StringPrintf("[%d, {}]", off_item->GetId()));
   EXPECT_STREQ(download_extension_errors::kInvalidOperationError,
                error.c_str());
 
   GoOffTheRecord();
 
   // Do the FileIcon test for both the on- and off-items while off the record.
   // NOTE(benjhayden): This does not include the FileIcon test from history,
   // just active downloads. This shouldn't be a problem.
-  EXPECT_TRUE(RunFunctionAndReturnString(
-      new DownloadsGetFileIconFunction(),
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          on_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
       base::StringPrintf("[%d, {}]", on_item->GetId()), &result_string));
-  EXPECT_TRUE(RunFunctionAndReturnString(
-      new DownloadsGetFileIconFunction(),
+  EXPECT_TRUE(RunFunctionAndReturnString(MockedGetFileIconFunction(
+          off_item->GetTargetFilePath(), IconLoader::NORMAL, "foo"),
       base::StringPrintf("[%d, {}]", off_item->GetId()), &result_string));
 
   // Do the pause/resume/cancel test for both the on- and off-items while off
   // the record.
   EXPECT_TRUE(RunFunction(new DownloadsPauseFunction(), on_item_arg));
   EXPECT_TRUE(on_item->IsPaused());
   EXPECT_TRUE(RunFunction(new DownloadsPauseFunction(), on_item_arg));
   EXPECT_TRUE(on_item->IsPaused());
   EXPECT_TRUE(RunFunction(new DownloadsResumeFunction(), on_item_arg));
   EXPECT_FALSE(on_item->IsPaused());
@@ skipping 726 matching lines @@
                           "  \"state\": {"
                           "    \"previous\": \"in_progress\","
                           "    \"current\": \"complete\"}}]",
                           result_id,
                           GetFilename("on_record.txt.crdownload").c_str(),
                           GetFilename("on_record.txt").c_str())));
   std::string disk_data;
   EXPECT_TRUE(file_util::ReadFileToString(item->GetFullPath(), &disk_data));
   EXPECT_STREQ(kPayloadData, disk_data.c_str());
 }