Revert 31875 to see whether it fixes reliability bot.

BUG=25677
TEST=None

Review URL: http://codereview.chromium.org/397014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32074 0039d316-1c4b-4281-b951-d872f2087c98

base/file_util.cc

@@ -267,20 +267,6 @@ MemoryMappedFile::~MemoryMappedFile() {
   CloseHandles();
 }
 
-bool MemoryMappedFile::Initialize(base::PlatformFile file) {
-  if (IsValid())
-    return false;
-
-  file_ = file;
-
-  if (!MapFileToMemoryInternal()) {
-    CloseHandles();
-    return false;
-  }
-
-  return true;
-}
-
 bool MemoryMappedFile::Initialize(const FilePath& file_name) {
   if (IsValid())
     return false;
@@ -293,19 +279,6 @@ bool MemoryMappedFile::Initialize(const FilePath& file_name) {
   return true;
 }
 
-bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) {
-  file_ = base::CreatePlatformFile(file_name,
-      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
-      NULL);
-
-  if (file_ == base::kInvalidPlatformFileValue) {
-    LOG(ERROR) << "Couldn't open " << file_name.value();
-    return false;
-  }
-
-  return MapFileToMemoryInternal();
-}
-
 bool MemoryMappedFile::IsValid() {
   return data_ != NULL;
 }

base/file_util.h

@@ -24,7 +24,6 @@
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
-#include "base/platform_file.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
 #include "base/time.h"
@@ -491,9 +490,10 @@ class MemoryMappedFile {
   // the file does not exist, or the memory mapping fails, it will return false.
   // Later we may want to allow the user to specify access.
   bool Initialize(const FilePath& file_name);
-  // As above, but works with an already-opened file. MemoryMappedFile will take
-  // ownership of |file| and close it when done.
-  bool Initialize(base::PlatformFile file);
+#if defined(OS_POSIX)
+  // As above, but works with an alreay-opened file.
+  bool Initialize(const base::FileDescriptor& fd);
+#endif
 
   const uint8* data() const { return data_; }
   size_t length() const { return length_; }
@@ -502,19 +502,23 @@ class MemoryMappedFile {
   bool IsValid();
 
  private:
-  // Open the given file and pass it to MapFileToMemoryInternal().
-  bool MapFileToMemory(const FilePath& file_name);
-
   // Map the file to memory, set data_ to that memory address. Return true on
   // success, false on any kind of failure. This is a helper for Initialize().
+  bool MapFileToMemory(const FilePath& file_name);
+
+#if defined(OS_POSIX)
   bool MapFileToMemoryInternal();
+#endif
 
   // Closes all open handles. Later we may want to make this public.
   void CloseHandles();
 
-  base::PlatformFile file_;
 #if defined(OS_WIN)
+  HANDLE file_;
   HANDLE file_mapping_;
+#elif defined(OS_POSIX)
+  // The file descriptor.
+  base::FileDescriptor file_;
 #endif
   uint8* data_;
   size_t length_;

base/file_util_posix.cc

@@ -667,36 +667,60 @@ bool FileEnumerator::ReadDirectory(std::vector<DirectoryEntryInfo>* entries,
 // MemoryMappedFile
 
 MemoryMappedFile::MemoryMappedFile()
-    : file_(base::kInvalidPlatformFileValue),
-      data_(NULL),
+    : data_(NULL),
       length_(0) {
 }
 
+bool MemoryMappedFile::Initialize(const base::FileDescriptor& fd) {
+  if (IsValid())
+    return false;
+
+  file_ = fd;
+
+  if (!MapFileToMemoryInternal()) {
+    CloseHandles();
+    return false;
+  }
+
+  return true;
+}
+
+bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) {
+  file_ = base::FileDescriptor(open(file_name.value().c_str(), O_RDONLY), true);
+
+  if (file_.fd == -1) {
+    LOG(ERROR) << "Couldn't open " << file_name.value();
+    return false;
+  }
+
+  return MapFileToMemoryInternal();
+}
+
 bool MemoryMappedFile::MapFileToMemoryInternal() {
   struct stat file_stat;
-  if (fstat(file_, &file_stat) == base::kInvalidPlatformFileValue) {
-    LOG(ERROR) << "Couldn't fstat " << file_ << ", errno " << errno;
+  if (fstat(file_.fd, &file_stat) == -1) {
+    LOG(ERROR) << "Couldn't fstat " << file_.fd << ", errno " << errno;
     return false;
   }
   length_ = file_stat.st_size;
 
   data_ = static_cast<uint8*>(
-      mmap(NULL, length_, PROT_READ, MAP_SHARED, file_, 0));
+      mmap(NULL, length_, PROT_READ, MAP_SHARED, file_.fd, 0));
   if (data_ == MAP_FAILED)
-    LOG(ERROR) << "Couldn't mmap " << file_ << ", errno " << errno;
+    LOG(ERROR) << "Couldn't mmap " << file_.fd << ", errno " << errno;
 
   return data_ != MAP_FAILED;
 }
 
 void MemoryMappedFile::CloseHandles() {
   if (data_ != NULL)
     munmap(data_, length_);
-  if (file_ != base::kInvalidPlatformFileValue)
-    close(file_);
+  if (file_.auto_close && file_.fd != -1)
+    close(file_.fd);
 
   data_ = NULL;
   length_ = 0;
-  file_ = base::kInvalidPlatformFileValue;
+  file_ = base::FileDescriptor();
 }
 
 } // namespace file_util

base/file_util_win.cc

@@ -773,7 +773,10 @@ MemoryMappedFile::MemoryMappedFile()
       length_(INVALID_FILE_SIZE) {
 }
 
-bool MemoryMappedFile::MapFileToMemoryInternal() {
+bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) {
+  file_ = ::CreateFile(file_name.value().c_str(), GENERIC_READ,
+                       FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                       FILE_ATTRIBUTE_NORMAL, NULL);
   if (file_ == INVALID_HANDLE_VALUE)
     return false;
 

build/common.gypi

@@ -45,14 +45,12 @@
           'target_arch%':
             '<!(uname -m | sed -e "s/i.86/ia32/;s/x86_64/x64/;s/arm.*/arm/")',
 
+          # For now, only Linux spellchecks in the renderer.
+          'spellchecker_in_renderer%': 1,
         }, {  # OS!="linux"
           'target_arch%': 'ia32',
-        }],
-        [ 'OS=="mac"', {
-          # For now, only Linux and Windows use spellcheck in the renderer.
+
           'spellchecker_in_renderer%': 0,
-        }, {  # OS!="mac"
-          'spellchecker_in_renderer%': 1,
         }],
       ],
 
@@ -145,7 +143,7 @@
 
     # Whether pepper APIs are enabled.
     'enable_pepper%': 0,
-
+    
     # Whether usage of OpenMAX is enabled.
     'enable_openmax%': 0,
 

chrome/browser/profile.cc

@@ -1339,8 +1339,8 @@ void ProfileImpl::ReinitializeSpellCheckHost(bool force) {
 }
 
 void ProfileImpl::SpellCheckHostInitialized() {
-  spellcheck_host_ready_ = spellcheck_host_ &&
-      spellcheck_host_->bdict_file() != base::kInvalidPlatformFileValue;
+  spellcheck_host_ready_ =
+      spellcheck_host_ && spellcheck_host_->bdict_fd().fd != -1;
   NotificationService::current()->Notify(
       NotificationType::SPELLCHECK_HOST_REINITIALIZED,
           Source<Profile>(this), NotificationService::NoDetails());

chrome/browser/renderer_host/browser_render_process_host.cc

@@ -57,7 +57,6 @@
 #include "grit/generated_resources.h"
 #include "ipc/ipc_logging.h"
 #include "ipc/ipc_message.h"
-#include "ipc/ipc_platform_file.h"
 #include "ipc/ipc_switches.h"
 
 #if defined(OS_WIN)
@@ -1137,23 +1136,13 @@ void BrowserRenderProcessHost::InitSpellChecker() {
   SpellCheckHost* spellcheck_host = profile()->GetSpellCheckHost();
   if (spellcheck_host) {
     PrefService* prefs = profile()->GetPrefs();
-    IPC::PlatformFileForTransit file;
-#if defined(OS_POSIX)
-    file = base::FileDescriptor(spellcheck_host->bdict_file(), false);
-#elif defined(OS_WIN)
-    ::DuplicateHandle(::GetCurrentProcess(), spellcheck_host->bdict_file(),
-                      GetHandle(), &file, 0, false, DUPLICATE_SAME_ACCESS);
-#endif
     Send(new ViewMsg_SpellChecker_Init(
-        file,
-        spellcheck_host->custom_words(),
+        spellcheck_host->bdict_fd(), spellcheck_host->custom_words(),
         spellcheck_host->language(),
         prefs->GetBoolean(prefs::kEnableAutoSpellCorrect)));
   } else {
     Send(new ViewMsg_SpellChecker_Init(
-        IPC::PlatformFileForTransit(),
-        std::vector<std::string>(),
-        std::string(),
+        base::FileDescriptor(), std::vector<std::string>(), std::string(),
         false));
   }
 }

chrome/browser/spellcheck_host.cc

@@ -121,18 +121,6 @@ FilePath GetVersionedFileName(const std::string& input_language,
   return dict_dir.AppendASCII(versioned_bdict_file_name);
 }
 
-FilePath GetFirstChoiceFilePath(const std::string& language) {
-  FilePath dict_dir;
-  PathService::Get(chrome::DIR_APP_DICTIONARIES, &dict_dir);
-  return GetVersionedFileName(language, dict_dir);
-}
-
-FilePath GetFallbackFilePath(const FilePath& first_choice) {
-  FilePath dict_dir;
-  PathService::Get(chrome::DIR_USER_DATA, &dict_dir);
-  return dict_dir.Append(first_choice.BaseName());
-}
-
 }  // namespace
 
 // Constructed on UI thread.
@@ -141,26 +129,29 @@ SpellCheckHost::SpellCheckHost(Observer* observer,
                                URLRequestContextGetter* request_context_getter)
     : observer_(observer),
       language_(language),
-      file_(base::kInvalidPlatformFileValue),
       tried_to_download_(false),
       request_context_getter_(request_context_getter) {
   DCHECK(observer_);
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
+  // TODO(estade): for Windows, we need to fall back to DIR_USER_DATA if
+  // DIR_APP_DICTIONARIES is not writeable.
+  FilePath dict_dir;
+  PathService::Get(chrome::DIR_APP_DICTIONARIES, &dict_dir);
+  bdict_file_ = GetVersionedFileName(language, dict_dir);
+
   FilePath personal_file_directory;
   PathService::Get(chrome::DIR_USER_DATA, &personal_file_directory);
   custom_dictionary_file_ =
       personal_file_directory.Append(chrome::kCustomDictionaryFileName);
 
-  bdict_file_path_ = GetFirstChoiceFilePath(language);
-
   ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &SpellCheckHost::InitializeDictionaryLocation));
+      NewRunnableMethod(this, &SpellCheckHost::Initialize));
 }
 
 SpellCheckHost::~SpellCheckHost() {
-  if (file_ != base::kInvalidPlatformFileValue)
-    base::ClosePlatformFile(file_);
+  if (fd_.fd != -1)
+    close(fd_.fd);
 }
 
 void SpellCheckHost::UnsetObserver() {
@@ -181,39 +172,24 @@ void SpellCheckHost::AddWord(const std::string& word) {
       Source<SpellCheckHost>(this), NotificationService::NoDetails());
 }
 
-void SpellCheckHost::InitializeDictionaryLocation() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-
-#if defined(OS_WIN)
-  // Check if the dictionary exists in the fallback location. If so, use it
-  // rather than downloading anew.
-  FilePath fallback = GetFallbackFilePath(bdict_file_path_);
-  if (!file_util::PathExists(bdict_file_path_) &&
-      file_util::PathExists(fallback)) {
-    bdict_file_path_ = fallback;
-  }
-#endif
-
-  Initialize();
-}
-
 void SpellCheckHost::Initialize() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
   if (!observer_)
     return;
 
-  file_ = base::CreatePlatformFile(bdict_file_path_,
-       base::PLATFORM_FILE_READ | base::PLATFORM_FILE_OPEN,
-       NULL);
+  // We set |auto_close| to false because we don't want IPC to close the fd.
+  // We will close it manually in the destructor.
+  fd_ = base::FileDescriptor(open(bdict_file_.value().c_str(), O_RDONLY),
+                             false);
 
   // File didn't exist. Download it.
-  if (file_ == base::kInvalidPlatformFileValue && !tried_to_download_) {
+  if (fd_.fd == -1 && !tried_to_download_) {
     DownloadDictionary();
     return;
   }
 
-  if (file_ != base::kInvalidPlatformFileValue) {
+  if (fd_.fd != -1) {
     // Load custom dictionary.
     std::string contents;
     file_util::ReadFileToString(custom_dictionary_file_, &contents);
@@ -242,7 +218,7 @@ void SpellCheckHost::DownloadDictionary() {
   static const char kDownloadServerUrl[] =
       "http://cache.pack.google.com/edgedl/chrome/dict/";
   GURL url = GURL(std::string(kDownloadServerUrl) + WideToUTF8(
-      l10n_util::ToLower(bdict_file_path_.BaseName().ToWStringHack())));
+      l10n_util::ToLower(bdict_file_.BaseName().ToWStringHack())));
   fetcher_.reset(new URLFetcher(url, URLFetcher::GET, this));
   fetcher_->set_request_context(request_context_getter_.get());
   tried_to_download_ = true;
@@ -287,27 +263,15 @@ void SpellCheckHost::OnURLFetchComplete(const URLFetcher* source,
   }
 
   size_t bytes_written =
-      file_util::WriteFile(bdict_file_path_, data.data(), data.length());
+      file_util::WriteFile(bdict_file_, data.data(), data.length());
   if (bytes_written != data.length()) {
-    bool success = false;
-#if defined(OS_WIN)
-    bdict_file_path_ = GetFallbackFilePath(bdict_file_path_);
-    bytes_written =
-        file_util::WriteFile(GetFallbackFilePath(bdict_file_path_),
-                                                 data.data(), data.length());
-    if (bytes_written == data.length())
-      success = true;
-#endif
-
-    if (!success) {
-      LOG(ERROR) << "Failure to save dictionary.";
-      // To avoid trying to load a partially saved dictionary, shortcut the
-      // Initialize() call.
-      ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
-          NewRunnableMethod(this,
-              &SpellCheckHost::InformObserverOfInitialization));
-      return;
-    }
+    LOG(ERROR) << "Failure to save dictionary.";
+    // To avoid trying to load a partially saved dictionary, shortcut the
+    // Initialize() call.
+    ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
+        NewRunnableMethod(this,
+            &SpellCheckHost::InformObserverOfInitialization));
+    return;
   }
 
   Initialize();