ImportantFileWriter: check return value of PostTask...

chrome/browser/extensions/test_extension_prefs.cc

 // Copyright (c) 2011 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/extensions/test_extension_prefs.h"
 
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/scoped_ptr.h"
@@ skipping 47 matching lines @@
 TestExtensionPrefs::~TestExtensionPrefs() {}
 
 void TestExtensionPrefs::RecreateExtensionPrefs() {
   // We persist and reload the PrefService's PrefStores because this process
   // deletes all empty dictionaries. The ExtensionPrefs implementation
   // needs to be able to handle this situation.
   if (pref_service_.get()) {
     // The PrefService writes its persistent file on the file thread, so we
     // need to wait for any pending I/O to complete before creating a new
     // PrefService.
-    MessageLoop file_loop;
-    BrowserThread file_thread(BrowserThread::FILE, &file_loop);
     pref_service_->SavePersistentPrefs();
-    file_loop.RunAllPending();
+    MessageLoop::current()->RunAllPending();

[asargent_no_longer_on_chrome, 2011/03/09 18:13:01]

The code before was waiting for the file loop to run all pending tasks, but
you've changed it to MessageLoop::current(), which I think will return the UI
thread's MessageLoop. Doesn't this create a race condition such that where we
recreate the pref service below this block, the I/O on the file thread might not
be finished in time?

[Paweł Hajdan Jr., 2011/03/09 18:22:14]

There can be only one MessageLoop per thread. My interpretation of the previous
code is that file_loop was totally equivalent to MessageLoop::current() after
the declaration MessageLoop file_loop.

[asargent_no_longer_on_chrome, 2011/03/09 18:33:25]

But it was registering file_loop as the message loop for the file thread just
after creating it. Are you saying that doesn't work?

[Paweł Hajdan Jr., 2011/03/09 20:40:03]

After a more detailed look I think MessageLoop::current() was _correct_ but
potentially fragile. I've switched to waiting for an event signaled on FILE
thread. This will work both in case FILE thread is a "mock" thread running on
the same message loop, or a "real" thread with its own message loop.

Thank you for catching this, I think it is an important issue.

   }
 
   extension_pref_value_map_.reset(new ExtensionPrefValueMap);
   PrefServiceMockBuilder builder;
   builder.WithUserFilePrefs(preferences_file_);
   builder.WithExtensionPrefs(
       new ExtensionPrefStore(extension_pref_value_map_.get(), false));
   pref_service_.reset(builder.Create());
   ExtensionPrefs::RegisterUserPrefs(pref_service_.get());
 
@@ skipping 30 matching lines @@
 
 std::string TestExtensionPrefs::AddExtensionAndReturnId(std::string name) {
   scoped_refptr<Extension> extension(AddExtension(name));
   return extension->id();
 }
 
 PrefService* TestExtensionPrefs::CreateIncognitoPrefService() const {
   return pref_service_->CreateIncognitoPrefService(
       new ExtensionPrefStore(extension_pref_value_map_.get(), true));
 }