Show more different NaCl loading errors.

chrome/browser/nacl_host/nacl_process_host.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/nacl_host/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 271 matching lines @@
     const base::FilePath& manifest_path) {
   nacl_host_message_filter_ = nacl_host_message_filter;
   reply_msg_ = reply_msg;
   manifest_path_ = manifest_path;
 
   // Start getting the IRT open asynchronously while we launch the NaCl process.
   // We'll make sure this actually finished in StartWithLaunchedProcess, below.
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   nacl_browser->EnsureAllResourcesAvailable();
   if (!nacl_browser->IsOk()) {
-    LOG(ERROR) << "NaCl process launch failed: could not find all the "
-        "resources needed to launch the process";
+    SendErrorToRenderer("could not find all the resources needed"
+                        " to launch the process");
     delete this;
     return;
   }
 
   // Rather than creating a socket pair in the renderer, and passing
   // one side through the browser to sel_ldr, socket pairs are created
   // in the browser and then passed to the renderer and sel_ldr.
   //
   // This is mainly for the benefit of Windows, where sockets cannot
   // be passed in messages, but are copied via DuplicateHandle().
   // This means the sandboxed renderer cannot send handles to the
   // browser process.
 
   NaClHandle pair[2];
   // Create a connected socket
   if (NaClSocketPair(pair) == -1) {
-    LOG(ERROR) << "NaCl process launch failed: could not create a socket pair";
+    SendErrorToRenderer("internal error: could not create a channel"

[Mark Seaborn, 2013/07/17 18:21:49]

This error message is actually misleading.  NaClSocketPair() does not itself
create a channel between NaCl and renderer.  I disagree with Nick somewhat here.
 "NaClSocketPair() failed" would be a more straightforward error here.  These
are all unexpected failure cases.  Their purpose is to be understandable to
developers of NaCl and little more.

[halyavin, 2013/07/18 09:32:22]

Done.

+                        " between NaCl and renderer");
     delete this;
     return;
   }
   internal_->socket_for_renderer = pair[0];
   internal_->socket_for_sel_ldr = pair[1];
   SetCloseOnExec(pair[0]);
   SetCloseOnExec(pair[1]);
 
   // Launch the process
   if (!LaunchSelLdr()) {
@@ skipping 65 matching lines @@
   if (!script.empty()) {
     cmd_line.AppendArg("--command");
     cmd_line.AppendArgNative(script.value());
   }
   return base::LaunchProcess(cmd_line, base::LaunchOptions(), NULL);
 }
 
 bool NaClProcessHost::LaunchSelLdr() {
   std::string channel_id = process_->GetHost()->CreateChannel();
   if (channel_id.empty()) {
-    LOG(ERROR) << "NaCl process launch failed: could not create channel";
+    SendErrorToRenderer("could not create IPC channel to NaCl process");

[Mark Seaborn, 2013/07/17 18:21:49]

Also misleading since it does not connect the IPC channel.  How about
"CreateChannel() failed"?

[halyavin, 2013/07/18 09:32:22]

Done.

     return false;
   }
 
   CommandLine::StringType nacl_loader_prefix;
 #if defined(OS_POSIX)
   nacl_loader_prefix = CommandLine::ForCurrentProcess()->GetSwitchValueNative(
       switches::kNaClLoaderCmdPrefix);
 #endif  // defined(OS_POSIX)
 
   // Build command line for nacl.
@@ skipping 14 matching lines @@
 #endif
 
   base::FilePath exe_path = ChildProcessHost::GetChildPath(flags);
   if (exe_path.empty())
     return false;
 
 #if defined(OS_WIN)
   // On Windows 64-bit NaCl loader is called nacl64.exe instead of chrome.exe
   if (RunningOnWOW64()) {
     if (!NaClBrowser::GetInstance()->GetNaCl64ExePath(&exe_path)) {
+      SendErrorToRenderer("could not get path to nacl64.exe");
       return false;
     }
   }
 #endif
 
   scoped_ptr<CommandLine> cmd_line(new CommandLine(exe_path));
   nacl::CopyNaClCommandLineArguments(cmd_line.get());
 
   cmd_line->AppendSwitchASCII(switches::kProcessType,
                               switches::kNaClLoaderProcess);
   cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id);
   if (NaClBrowser::GetDelegate()->DialogsAreSuppressed())
     cmd_line->AppendSwitch(switches::kNoErrorDialogs);
 
   if (!nacl_loader_prefix.empty())
     cmd_line->PrependWrapper(nacl_loader_prefix);
 
   // On Windows we might need to start the broker process to launch a new loader
 #if defined(OS_WIN)
   if (RunningOnWOW64()) {
     if (!NaClBrokerService::GetInstance()->LaunchLoader(
             weak_factory_.GetWeakPtr(), channel_id)) {
-      LOG(ERROR) << "NaCl process launch failed: broker service did not launch "
-          "process";
+      SendErrorToRenderer("broker service did not launch process");
       return false;
     }
   } else {
     process_->Launch(new NaClSandboxedProcessLauncherDelegate,
                      cmd_line.release());
   }
 #elif defined(OS_POSIX)
   process_->Launch(nacl_loader_prefix.empty(),  // use_zygote
                    base::EnvironmentVector(),
                    cmd_line.release());
@@ skipping 24 matching lines @@
 
 void NaClProcessHost::OnProcessLaunched() {
   if (!StartWithLaunchedProcess())
     delete this;
 }
 
 // Called when the NaClBrowser singleton has been fully initialized.
 void NaClProcessHost::OnResourcesReady() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   if (!nacl_browser->IsReady()) {
-    LOG(ERROR) << "NaCl process launch failed: could not acquire shared "
-        "resources needed by NaCl";
+    SendErrorToRenderer("could not acquire shared resources needed by NaCl");
     delete this;
   } else if (!SendStart()) {
     delete this;
   }
 }
 
 bool NaClProcessHost::ReplyToRenderer(
     const IPC::ChannelHandle& channel_handle) {
 #if defined(OS_WIN)
   // If we are on 64-bit Windows, the NaCl process's sandbox is
   // managed by a different process from the renderer's sandbox.  We
   // need to inform the renderer's sandbox about the NaCl process so
   // that the renderer can send handles to the NaCl process using
   // BrokerDuplicateHandle().
   if (RunningOnWOW64()) {
     if (!content::BrokerAddTargetPeer(process_->GetData().handle)) {
-      LOG(ERROR) << "Failed to add NaCl process PID";
+      SendErrorToRenderer("failed to register NaCl process in sandbox system");

[Mark Seaborn, 2013/07/17 18:21:49]

How about "BrokerAddTargetPeer() failed"?

[halyavin, 2013/07/18 09:32:22]

Done.

       return false;
     }
   }
 #endif
 
   nacl::FileDescriptor handle_for_renderer;
 #if defined(OS_WIN)
   // Copy the handle into the renderer process.
   HANDLE handle_in_renderer;
   if (!DuplicateHandle(base::GetCurrentProcessHandle(),
                        reinterpret_cast<HANDLE>(
                            internal_->socket_for_renderer),
                        nacl_host_message_filter_->PeerHandle(),
                        &handle_in_renderer,
                        0,  // Unused given DUPLICATE_SAME_ACCESS.
                        FALSE,
                        DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
-    LOG(ERROR) << "DuplicateHandle() failed";
+    SendErrorToRenderer("DuplicateHandle() failed");
     return false;
   }
   handle_for_renderer = reinterpret_cast<nacl::FileDescriptor>(
       handle_in_renderer);
 #else
   // No need to dup the imc_handle - we don't pass it anywhere else so
   // it cannot be closed.
   nacl::FileDescriptor imc_handle;
   imc_handle.fd = internal_->socket_for_renderer;
   imc_handle.auto_close = true;
   handle_for_renderer = imc_handle;
 #endif
 
   const ChildProcessData& data = process_->GetData();
-  NaClHostMsg_LaunchNaCl::WriteReplyParams(
-      reply_msg_, handle_for_renderer,
-      channel_handle, base::GetProcId(data.handle), data.id);
-  nacl_host_message_filter_->Send(reply_msg_);
-  nacl_host_message_filter_ = NULL;
-  reply_msg_ = NULL;
+  SendMessageToRenderer(
+      nacl::NaClLaunchResult(handle_for_renderer,
+                             channel_handle,
+                             base::GetProcId(data.handle),
+                             data.id),
+      std::string() /* error_message */);
   internal_->socket_for_renderer = NACL_INVALID_HANDLE;
   return true;
 }
 
+void NaClProcessHost::SendErrorToRenderer(const std::string& error_message) {
+  LOG(ERROR) << "NaCl process launch failed: " << error_message;
+  SendMessageToRenderer(nacl::NaClLaunchResult(), error_message);
+}
+
+void NaClProcessHost::SendMessageToRenderer(
+    const nacl::NaClLaunchResult& result,
+    const std::string& error_message) {
+  DCHECK(nacl_host_message_filter_);
+  DCHECK(reply_msg_);
+  if (nacl_host_message_filter_ != NULL && reply_msg_ != NULL) {
+    NaClHostMsg_LaunchNaCl::WriteReplyParams(
+        reply_msg_, result, error_message);
+    nacl_host_message_filter_->Send(reply_msg_);
+    nacl_host_message_filter_ = NULL;
+    reply_msg_ = NULL;
+  }
+}
+
 // TCP port we chose for NaCl debug stub. It can be any other number.
 static const int kDebugStubPort = 4014;
 
 #if defined(OS_POSIX)
 SocketDescriptor NaClProcessHost::GetDebugStubSocketHandle() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   SocketDescriptor s;
   // We allocate currently unused TCP port for debug stub tests. The port
   // number is passed to the test via debug stub port listener.
   if (nacl_browser->HasGdbDebugStubPortListener()) {
@@ skipping 178 matching lines @@
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
 
   if (nacl_browser->IsReady()) {
     return SendStart();
   } else if (nacl_browser->IsOk()) {
     nacl_browser->WaitForResources(
         base::Bind(&NaClProcessHost::OnResourcesReady,
                    weak_factory_.GetWeakPtr()));
     return true;
   } else {
-    LOG(ERROR) << "NaCl process failed to launch: previously failed to acquire "
-        "shared resources";
+    SendErrorToRenderer("previously failed to acquire shared resources");
     return false;
   }
 }
 
 void NaClProcessHost::OnQueryKnownToValidate(const std::string& signature,
                                              bool* result) {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   *result = nacl_browser->QueryKnownToValidate(signature, off_the_record_);
 }
 
@@ skipping 142 matching lines @@
   } else {
     NaClStartDebugExceptionHandlerThread(
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif