Fix the failed cases in URLFetcherFileTest on Android.

net/url_request/url_fetcher_impl_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 "net/url_request/url_fetcher_impl.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/file_util.h"
@@ skipping 363 matching lines @@
  public:
   // URLFetcherDelegate
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
  private:
   std::string data_;
 };
 
 class URLFetcherFileTest : public URLFetcherTest {
  public:
   URLFetcherFileTest() : take_ownership_of_file_(false),
-                         expected_file_error_(base::PLATFORM_FILE_OK) {}
+                         expected_file_error_(base::PLATFORM_FILE_OK) {
+    PathService::Get(base::DIR_SOURCE_ROOT, &document_root_);
+#if defined(OS_ANDROID)
+    document_root_= document_root_.AppendASCII(kDocRoot);
+#endif
+  }
 
   void CreateFetcherForFile(const GURL& url, const FilePath& file_path);
   void CreateFetcherForTempFile(const GURL& url);
 
+  FilePath GetDocumentRoot(const TestServer& test_server) {
+#if defined(OS_ANDROID)
+    return document_root_;
+#else
+    return test_server.document_root();

[wtc, 2012/10/16 21:43:03]

What does test_server.document_root() return for OS_ANDROID?
Does the return value contain kDocRoot?

If so, can we extract the kDocRoot part from the return value
and append that to base::DIR_SOURCE_ROOT?

What I want to avoid is the hardcoded knowledge of kDocRoot
on line 387. I'd like to get that from test_server.

[Shouqun Liu, 2012/10/17 08:44:30]

Hi wtc, thanks for your advice!
For Android,  test_server.document_root()  simply returns the kDocRoot
("chrome/test/data"), as the document root, but this patch is not a valid path
for Android test case to use. 
So "base::DIR_SOURCE_ROOT" is appended here to let the case to find the correct
path.

And here are two options: 
(1)  [patchset 6] keep this change which uses "base::DIR_SOURCE_ROOT"
specifically for Android  ; 
(2)  [patchset 7] change the implementation of document_root() in test_server,
but the test server also use this "document_root" value to launch the test
server in host PC. 
And here is the conflict: test server in PC and test case in Android can't share
the same "document_root".   So need to add a additional method to get the
document root in device (e.g. GetDocumentRootInDevice).

Which one do you thinks is better? 

On 2012/10/16 21:43:03, wtc wrote:

+#endif
+  }
+
   // URLFetcherDelegate
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
  protected:
+  FilePath document_root_;

[wtc, 2012/10/16 21:43:03]

The document_root_ member should be inside #if defined(OS_ANDROID).

   FilePath expected_file_;
   FilePath file_path_;
 
   // Set by the test. Used in OnURLFetchComplete() to decide if
   // the URLFetcher should own the temp file, so that we can test
   // disowning prevents the file from being deleted.
   bool take_ownership_of_file_;
 
   // Expected file error code for the test.
   // PLATFORM_FILE_OK when expecting success.
@@ skipping 677 matching lines @@
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Get a small file.
   static const char kFileToFetch[] = "simple.html";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch),
       temp_dir.path().AppendASCII(kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 TEST_F(URLFetcherFileTest, LargeGet) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Get a file large enough to require more than one read into
   // URLFetcher::Core's IOBuffer.
   static const char kFileToFetch[] = "animate1.gif";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch),
       temp_dir.path().AppendASCII(kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 }
 
 TEST_F(URLFetcherFileTest, CanTakeOwnershipOfFile) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Get a small file.
   static const char kFileToFetch[] = "simple.html";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch),
       temp_dir.path().AppendASCII(kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   MessageLoop::current()->RunAllPending();
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 
 TEST_F(URLFetcherFileTest, OverwriteExistingFile) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Create a file before trying to fetch.
   static const char kFileToFetch[] = "simple.html";
   static const char kData[] = "abcdefghijklmnopqrstuvwxyz";
   file_path_ = temp_dir.path().AppendASCII(kFileToFetch);
   const int data_size = arraysize(kData);
   ASSERT_EQ(file_util::WriteFile(file_path_, kData, data_size), data_size);
   ASSERT_TRUE(file_util::PathExists(file_path_));
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   ASSERT_FALSE(file_util::ContentsEqual(file_path_, expected_file_));
 
   // Get a small file.
   CreateFetcherForFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch),
       file_path_);
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 }
 
 TEST_F(URLFetcherFileTest, TryToOverwriteDirectory) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   // Create a directory before trying to fetch.
   static const char kFileToFetch[] = "simple.html";
   file_path_ = temp_dir.path().AppendASCII(kFileToFetch);
   ASSERT_TRUE(file_util::CreateDirectory(file_path_));
   ASSERT_TRUE(file_util::PathExists(file_path_));
 
   // Get a small file.
   expected_file_error_ = base::PLATFORM_FILE_ERROR_ACCESS_DENIED;
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch),
       file_path_);
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   MessageLoop::current()->RunAllPending();
 }
 
 TEST_F(URLFetcherFileTest, SmallGetToTempFile) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   // Get a small file.
   static const char kFileToFetch[] = "simple.html";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForTempFile(
       test_server.GetURL(std::string(kTestServerFilePrefix) + kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 TEST_F(URLFetcherFileTest, LargeGetToTempFile) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   // Get a file large enough to require more than one read into
   // URLFetcher::Core's IOBuffer.
   static const char kFileToFetch[] = "animate1.gif";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForTempFile(test_server.GetURL(
       std::string(kTestServerFilePrefix) + kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 }
 
 TEST_F(URLFetcherFileTest, CanTakeOwnershipOfTempFile) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   // Get a small file.
   static const char kFileToFetch[] = "simple.html";
-  expected_file_ = test_server.document_root().AppendASCII(kFileToFetch);
+  expected_file_ = GetDocumentRoot(test_server).AppendASCII(kFileToFetch);
   CreateFetcherForTempFile(test_server.GetURL(
       std::string(kTestServerFilePrefix) + kFileToFetch));
 
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   MessageLoop::current()->RunAllPending();
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 }  // namespace
 
 }  // namespace net