Improve the initialization of shared resources for the NaCl process host.

This CL eliminates multithreaded access to a Singleton and paves the way for
NaCl validation cache persistance.

BUG= http://code.google.com/p/nativeclient/issues/detail?id=2515
TEST= none


Review URL: https://chromiumcodereview.appspot.com/10387068

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

4 files changed, 137 insertions, 59 deletions

diff --git a/chrome/browser/nacl_host/nacl_browser.cc b/chrome/browser/nacl_host/nacl_browser.cc
index e9e8ec2..84ca17c 100644
--- a/chrome/browser/nacl_host/nacl_browser.cc
+++ b/chrome/browser/nacl_host/nacl_browser.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/nacl_host/nacl_browser.h"
 
+#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/win/windows_version.h"
 #include "chrome/common/chrome_paths.h"
@@ -41,8 +42,11 @@ const FilePath::StringType NaClIrtName() {
 }  // namespace
 
 NaClBrowser::NaClBrowser()
-    : irt_platform_file_(base::kInvalidPlatformFileValue),
-      irt_filepath_() {
+    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
+      irt_platform_file_(base::kInvalidPlatformFileValue),
+      irt_filepath_(),
+      irt_state_(NaClResourceUninitialized),
+      ok_(true) {
   InitIrtFilePath();
 }
 
@@ -65,10 +69,10 @@ void NaClBrowser::InitIrtFilePath() {
   } else {
     FilePath plugin_dir;
     if (!PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &plugin_dir)) {
-      DLOG(ERROR) << "Failed to locate the plugins directory";
+      DLOG(ERROR) << "Failed to locate the plugins directory, NaCl disabled.";
+      MarkAsFailed();
       return;
     }
-
     irt_filepath_ = plugin_dir.Append(NaClIrtName());
   }
 }
@@ -77,53 +81,86 @@ NaClBrowser* NaClBrowser::GetInstance() {
   return Singleton<NaClBrowser>::get();
 }
 
-bool NaClBrowser::IrtAvailable() const {
-  return irt_platform_file_ != base::kInvalidPlatformFileValue;
+bool NaClBrowser::IsReady() const {
+  return IsOk() && irt_state_ == NaClResourceReady;
+}
+
+bool NaClBrowser::IsOk() const {
+  return ok_;
 }
 
 base::PlatformFile NaClBrowser::IrtFile() const {
+  CHECK_EQ(irt_state_, NaClResourceReady);
   CHECK_NE(irt_platform_file_, base::kInvalidPlatformFileValue);
   return irt_platform_file_;
 }
 
-// Attempt to ensure the IRT will be available when we need it, but don't wait.
-bool NaClBrowser::EnsureIrtAvailable() {
-  if (IrtAvailable())
-    return true;
-
-  return content::BrowserThread::PostTask(
-      content::BrowserThread::FILE, FROM_HERE,
-      base::Bind(&NaClBrowser::DoOpenIrtLibraryFile));
-}
-
-// We really need the IRT to be available now, so make sure that it is.
-// When it's ready, we'll run the reply closure.
-bool NaClBrowser::MakeIrtAvailable(const base::Closure& reply) {
-  return content::BrowserThread::PostTaskAndReply(
-      content::BrowserThread::FILE, FROM_HERE,
-      base::Bind(&NaClBrowser::DoOpenIrtLibraryFile), reply);
+void NaClBrowser::EnsureAllResourcesAvailable() {
+  // Currently the only resource we need to initialize is the IRT.
+  // In the future we will need to load the validation cache from disk.
+  EnsureIrtAvailable();
 }
 
-const FilePath& NaClBrowser::GetIrtFilePath() {
-  return irt_filepath_;
+// Load the IRT async.
+void NaClBrowser::EnsureIrtAvailable() {
+  if (IsOk() && irt_state_ == NaClResourceUninitialized) {
+    irt_state_ = NaClResourceRequested;
+    // TODO(ncbray) use blocking pool.
+    if (!base::FileUtilProxy::CreateOrOpen(
+            content::BrowserThread::GetMessageLoopProxyForThread(
+                content::BrowserThread::FILE),
+            irt_filepath_,
+            base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
+            base::Bind(&NaClBrowser::OnIrtOpened,
+                       weak_factory_.GetWeakPtr()))) {
+      LOG(ERROR) << "Internal error, NaCl disabled.";
+      MarkAsFailed();
+    }
+  }
 }
 
-// This only ever runs on the BrowserThread::FILE thread.
-// If multiple tasks are posted, the later ones are no-ops.
-void NaClBrowser::OpenIrtLibraryFile() {
-  if (irt_platform_file_ != base::kInvalidPlatformFileValue)
-    // We've already run.
-    return;
-
-  base::PlatformFileError error_code;
-  irt_platform_file_ = base::CreatePlatformFile(irt_filepath_,
-                                                base::PLATFORM_FILE_OPEN |
-                                                base::PLATFORM_FILE_READ,
-                                                NULL,
-                                                &error_code);
-  if (error_code != base::PLATFORM_FILE_OK) {
+void NaClBrowser::OnIrtOpened(base::PlatformFileError error_code,
+                              base::PassPlatformFile file,
+                              bool created) {
+  DCHECK_EQ(irt_state_, NaClResourceRequested);
+  DCHECK(!created);
+  if (error_code == base::PLATFORM_FILE_OK) {
+    irt_platform_file_ = file.ReleaseValue();
+  } else {
     LOG(ERROR) << "Failed to open NaCl IRT file \""
                << irt_filepath_.LossyDisplayName()
                << "\": " << error_code;
+    MarkAsFailed();
   }
+  irt_state_ = NaClResourceReady;
+  CheckWaiting();
+}
+
+void NaClBrowser::CheckWaiting() {
+  if (!IsOk() || IsReady()) {
+    // Queue the waiting tasks into the message loop.  This helps avoid
+    // re-entrancy problems that could occur if the closure was invoked
+    // directly.  For example, this could result in use-after-free of the
+    // process host.
+    for (std::vector<base::Closure>::iterator iter = waiting_.begin();
+         iter != waiting_.end(); ++iter) {
+      MessageLoop::current()->PostTask(FROM_HERE, *iter);
+    }
+    waiting_.clear();
+  }
+}
+
+void NaClBrowser::MarkAsFailed() {
+  ok_ = false;
+  CheckWaiting();
+}
+
+void NaClBrowser::WaitForResources(const base::Closure& reply) {
+  waiting_.push_back(reply);
+  EnsureAllResourcesAvailable();
+  CheckWaiting();
+}
+
+const FilePath& NaClBrowser::GetIrtFilePath() {
+  return irt_filepath_;
 }
diff --git a/chrome/browser/nacl_host/nacl_browser.h b/chrome/browser/nacl_host/nacl_browser.h
index 9245963..4fac991b 100644
--- a/chrome/browser/nacl_host/nacl_browser.h
+++ b/chrome/browser/nacl_host/nacl_browser.h
@@ -7,7 +7,9 @@
 #pragma once
 
 #include "base/bind.h"
+#include "base/file_util_proxy.h"
 #include "base/memory/singleton.h"
+#include "base/memory/weak_ptr.h"
 #include "base/platform_file.h"
 #include "chrome/browser/nacl_host/nacl_validation_cache.h"
 
@@ -16,24 +18,43 @@ class NaClBrowser {
  public:
   static NaClBrowser* GetInstance();
 
-  bool IrtAvailable() const;
+  // Will it be possible to launch a NaCl process, eventually?
+  bool IsOk() const;
 
-  base::PlatformFile IrtFile() const;
+  // Are we ready to launch a NaCl process now?  Implies IsOk().
+  bool IsReady() const;
 
-  // Asynchronously attempt to get the IRT open.
-  bool EnsureIrtAvailable();
+  // Attempt to asynchronously acquire all resources needed to start a process.
+  // This method is idempotent - it is safe to call multiple times.
+  void EnsureAllResourcesAvailable();
 
-  // Make sure the IRT gets opened and follow up with the reply when it's ready.
-  bool MakeIrtAvailable(const base::Closure& reply);
+  // Enqueues reply() in the message loop when all the resources needed to start
+  // a process have been acquired.
+  void WaitForResources(const base::Closure& reply);
+
+  // Asynchronously attempt to get the IRT open.
+  // This is entailed by EnsureInitialized.  This method is exposed as part of
+  // the public interface, however, so the IRT can be explicitly opened as
+  // early as possible to prevent autoupdate issues.
+  void EnsureIrtAvailable();
 
   // Path to IRT. Available even before IRT is loaded.
   const FilePath& GetIrtFilePath();
 
+  // IRT file handle, only available when IsReady().
+  base::PlatformFile IrtFile() const;
+
   NaClValidationCache validation_cache;
 
  private:
   friend struct DefaultSingletonTraits<NaClBrowser>;
 
+  enum NaClResourceState {
+    NaClResourceUninitialized,
+    NaClResourceRequested,
+    NaClResourceReady
+  };
+
   NaClBrowser();
   ~NaClBrowser();
 
@@ -41,14 +62,27 @@ class NaClBrowser {
 
   void OpenIrtLibraryFile();
 
-  static void DoOpenIrtLibraryFile() {
-    GetInstance()->OpenIrtLibraryFile();
-  }
+  void OnIrtOpened(base::PlatformFileError error_code,
+                   base::PassPlatformFile file, bool created);
 
-  base::PlatformFile irt_platform_file_;
+  // Dispatch waiting tasks if we are ready, or if we know we'll never be ready.
+  void CheckWaiting();
+
+  // Indicate that it is impossible to launch a NaCl process.
+  void MarkAsFailed();
+
+  // Singletons get destroyed at shutdown.
+  base::WeakPtrFactory<NaClBrowser> weak_factory_;
 
+  base::PlatformFile irt_platform_file_;
   FilePath irt_filepath_;
 
+  NaClResourceState irt_state_;
+  bool ok_;
+
+  // A list of pending tasks to start NaCl processes.
+  std::vector<base::Closure> waiting_;
+
   DISALLOW_COPY_AND_ASSIGN(NaClBrowser);
 };
 
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 4c296be..ccb54a1 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -211,8 +211,10 @@ void NaClProcessHost::Launch(
 
   // Start getting the IRT open asynchronously while we launch the NaCl process.
   // We'll make sure this actually finished in StartWithLaunchedProcess, below.
-  if (!NaClBrowser::GetInstance()->EnsureIrtAvailable()) {
-    DLOG(ERROR) << "Cannot launch NaCl process after IRT file open failed";
+  NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
+  nacl_browser->EnsureAllResourcesAvailable();
+  if (!nacl_browser->IsOk()) {
+    DLOG(ERROR) << "Cannot launch NaCl process";
     delete this;
     return;
   }
@@ -538,10 +540,10 @@ void NaClProcessHost::OnProcessLaunched() {
     delete this;
 }
 
-// The asynchronous attempt to get the IRT file open has completed.
-void NaClProcessHost::IrtReady() {
+// Called when the NaClBrowser singleton has been fully initialized.
+void NaClProcessHost::OnResourcesReady() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
-  if (!nacl_browser->IrtAvailable() || !SendStart()) {
+  if (!nacl_browser->IsReady() || !SendStart()) {
     DLOG(ERROR) << "Cannot launch NaCl process";
     delete this;
   }
@@ -671,12 +673,17 @@ bool NaClProcessHost::StartWithLaunchedProcess() {
 #endif
 
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
-  if (nacl_browser->IrtAvailable())
-    return SendStart();  // The IRT is already open.  Away we go.
 
-  // We're waiting for the IRT to be open.
-  return nacl_browser->MakeIrtAvailable(
-      base::Bind(&NaClProcessHost::IrtReady, weak_factory_.GetWeakPtr()));
+  if (nacl_browser->IsReady()) {
+    return SendStart();
+  } else if (nacl_browser->IsOk()) {
+    nacl_browser->WaitForResources(
+        base::Bind(&NaClProcessHost::OnResourcesReady,
+                   weak_factory_.GetWeakPtr()));
+    return true;
+  } else {
+    return false;
+  }
 }
 
 void NaClProcessHost::OnQueryKnownToValidate(const std::string& signature,
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index 57e150d..412377c 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -82,7 +82,7 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   virtual void OnProcessCrashed(int exit_code) OVERRIDE;
   virtual void OnProcessLaunched() OVERRIDE;
 
-  void IrtReady();
+  void OnResourcesReady();
 
   // Sends the reply message to the renderer who is waiting for the plugin
   // to load. Returns true on success.