Revert changes that moved spellchecker to renderer for 249 branch since there were some crashes and pulling in the fixes is hard.

This reverts:

------------------------------------------------------------------------
r31875 | estade@chromium.org | 2009-11-12 17:36:50 -0800 (Thu, 12 Nov 2009) | 4 lines

Use renderer spellchecker for windows.

BUG=25677
Review URL: http://codereview.chromium.org/372075
------------------------------------------------------------------------
r31529 | estade@chromium.org | 2009-11-09 17:33:05 -0800 (Mon, 09 Nov 2009) | 8 lines

Couple of prospective fix for memory test flakiness: addref/release the url request context getter appropriately; fix a leak.

Passing as a straight pointer without addreffing was copied from the previous spellchecker impl. Using .release() instead of = NULL was a plain old mistake.

BUG=none
TEST=memory test flakiness goes away hopefully?

Review URL: http://codereview.chromium.org/379015
------------------------------------------------------------------------
r31199 | estade@chromium.org | 2009-11-05 19:05:46 -0800 (Thu, 05 Nov 2009) | 28 lines

Move the spellchecker to the renderer.

The motivation is that this removes the sync IPC on every call to the spellchecker. Also, currently we spellcheck in the IO thread, which frequently needs to go to disk (in particular, the entire spellcheck dictionary starts paged out), so this will block just the single renderer when that happens, rather than the whole IO thread.

This breaks the SpellChecker class into two new classes.
1) On the browser side, we have SpellCheckHost. This class handles browser-wide tasks, such as keeping the custom words list in sync with the on-disk custom words dictionary, downloading missing dictionaries, etc. On Posix, it also opens the bdic file since the renderer isn't allowed to open files. SpellCheckHost is created and destroyed on the UI thread. It is initialized on the file thread.
2) On the renderer side, SpellChecker2. This class will one day be renamed SpellChecker. It handles actual checking of the words, memory maps the dictionary file, loads hunspell, etc. There is one SpellChecker2 per RenderThread (hence one per render process).

My intention is for this patch to move Linux to this new approach, and follow up with ports for Windows (which will involve passing a dictionary file name rather than a file descriptor through to the renderer) and Mac (which will involve adding sync ViewHost IPC callsfor when the platform spellchecker is enabled). Note that anyone using the platform spellchecker rather than Hunspell will get no benefit out of this refactor.

There should be no loss of functionality for Linux (or any other platform) in this patch. The following should all still work:
- dictionary is loaded lazily
- hunspell is initialized lazily, per renderer
- language changes work.
- Dynamic downloading of new dictionaries
- auto spell correct works (as well as toggling it).
- disabling spellcheck works.
- custom words work (including adding in one renderer and immediately having it take effect in other renderers, for certain values of "immediate")

TODO:
- move spellchecker unit tests to test SpellCheck2
- add sync IPC for platform spellchecker; port to Mac
- add dictionary location fallback; port to Windows
- remove SpellChecker classes from browser/

BUG=25677

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



git-svn-id: svn://svn.chromium.org/chrome/branches/249/src@32436 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 23 insertions, 48 deletions

diff --git a/base/file_util.cc b/base/file_util.cc
index 8d1b517..4cde773 100644
--- a/base/file_util.cc
+++ b/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;
 }
diff --git a/base/file_util.h b/base/file_util.h
index 782a6a2..9a590a4 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -24,15 +24,10 @@
 
 #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"
 
-#if defined(OS_POSIX)
-#include "base/file_descriptor_posix.h"
-#endif
-
 namespace base {
 class Time;
 }
@@ -491,9 +486,6 @@ 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);
 
   const uint8* data() const { return data_; }
   size_t length() const { return length_; }
@@ -502,19 +494,19 @@ 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 MapFileToMemoryInternal();
+  bool MapFileToMemory(const FilePath& file_name);
 
   // 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.
+  int file_;
 #endif
   uint8* data_;
   size_t length_;
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index a269d3f..6c5b86a 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -667,15 +667,22 @@ bool FileEnumerator::ReadDirectory(std::vector<DirectoryEntryInfo>* entries,
 // MemoryMappedFile
 
 MemoryMappedFile::MemoryMappedFile()
-    : file_(base::kInvalidPlatformFileValue),
+    : file_(-1),
       data_(NULL),
       length_(0) {
 }
 
-bool MemoryMappedFile::MapFileToMemoryInternal() {
+bool MemoryMappedFile::MapFileToMemory(const FilePath& file_name) {
+  file_ = open(file_name.value().c_str(), O_RDONLY);
+
+  if (file_ == -1) {
+    LOG(ERROR) << "Couldn't open " << file_name.value();
+    return false;
+  }
+
   struct stat file_stat;
-  if (fstat(file_, &file_stat) == base::kInvalidPlatformFileValue) {
-    LOG(ERROR) << "Couldn't fstat " << file_ << ", errno " << errno;
+  if (fstat(file_, &file_stat) == -1) {
+    LOG(ERROR) << "Couldn't fstat " << file_name.value() << ", errno " << errno;
     return false;
   }
   length_ = file_stat.st_size;
@@ -683,7 +690,7 @@ bool MemoryMappedFile::MapFileToMemoryInternal() {
   data_ = static_cast<uint8*>(
       mmap(NULL, length_, PROT_READ, MAP_SHARED, file_, 0));
   if (data_ == MAP_FAILED)
-    LOG(ERROR) << "Couldn't mmap " << file_ << ", errno " << errno;
+    LOG(ERROR) << "Couldn't mmap " << file_name.value() << ", errno " << errno;
 
   return data_ != MAP_FAILED;
 }
@@ -691,12 +698,12 @@ bool MemoryMappedFile::MapFileToMemoryInternal() {
 void MemoryMappedFile::CloseHandles() {
   if (data_ != NULL)
     munmap(data_, length_);
-  if (file_ != base::kInvalidPlatformFileValue)
+  if (file_ != -1)
     close(file_);
 
   data_ = NULL;
   length_ = 0;
-  file_ = base::kInvalidPlatformFileValue;
+  file_ = -1;
 }
 
 } // namespace file_util
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 3105d37..d6edc6f 100644
--- a/base/file_util_win.cc
+++ b/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;