[Spellcheck] Unify dictionary handling.

chrome/browser/spellchecker/spellcheck_service.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 "chrome/browser/spellchecker/spellcheck_service.h"
 
 #include "base/platform_file.h"
 #include "base/prefs/pref_member.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_split.h"
@@ skipping 150 matching lines @@
 
   content::BrowserContext* context = process->GetBrowserContext();
   if (SpellcheckServiceFactory::GetForContext(context) != this)
     return;
 
   PrefService* prefs = user_prefs::UserPrefs::Get(context);
   IPC::PlatformFileForTransit file = IPC::InvalidPlatformFileForTransit();
 
   if (hunspell_dictionary_->GetDictionaryFile() !=
       base::kInvalidPlatformFileValue) {
-#if defined(OS_POSIX)
-    file = base::FileDescriptor(hunspell_dictionary_->GetDictionaryFile(),
-                                false);
-#elif defined(OS_WIN)
-    BOOL ok = ::DuplicateHandle(::GetCurrentProcess(),
-                                hunspell_dictionary_->GetDictionaryFile(),
-                                process->GetHandle(),
-                                &file,
-                                0,
-                                false,
-                                DUPLICATE_SAME_ACCESS);
-    DCHECK(ok) << ::GetLastError();
-#endif
+    file = IPC::GetFileHandleForProcess(
+        hunspell_dictionary_->GetDictionaryFile(), process->GetHandle(), false);

[please use gerrit instead, 2013/12/30 17:21:24]

DHCECK(file != base::kInvalidPlatformFileValue)
    << logging::GetLastSystemErrorCode();

[groby-ooo-7-16, 2014/01/02 17:48:44]

Which error is this supposed to catch? 

[please use gerrit instead, 2014/01/02 17:54:25]

This would catch ::DuplicateHandle() returning false, like in the existing code.
http://msdn.microsoft.com/en-us/library/windows/desktop/ms724251(v=vs.85).aspx
is not clear on why that function could return false.

[groby-ooo-7-16, 2014/01/02 19:15:28]

Technically, DCHECK should document invalid states. If DuplicateHandle can fail,
it's not an invalid state :)

Further, GetFileHandleForProcess handles DuplicateHandle failures correctly, and
the render process handles invalid file handles correctly. I don't think this
DCHECK has a point, so keeping it removed.

If we're concerned that this actually fails in the field, this should either be
a metric or (shudder!) a CHECK

   }
 
   process->Send(new SpellCheckMsg_Init(
       file,
       custom_dictionary_->GetWords(),
       hunspell_dictionary_->GetLanguage(),
       prefs->GetBoolean(prefs::kEnableAutoSpellCorrect)));
   process->Send(new SpellCheckMsg_EnableSpellCheck(
       prefs->GetBoolean(prefs::kEnableContinuousSpellcheck)));
 }
@@ skipping 126 matching lines @@
 }
 
 void SpellcheckService::UpdateFeedbackSenderState() {
   if (SpellingServiceClient::IsAvailable(
           context_, SpellingServiceClient::SPELLCHECK)) {
     feedback_sender_->StartFeedbackCollection();
   } else {
     feedback_sender_->StopFeedbackCollection();
   }
 }