ChildProcessLauncher: Don't access context_ private members directly.

content/browser/child_process_launcher.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 "content/browser/child_process_launcher.h"
 
 #include <utility>  // For std::pair.
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 60 matching lines @@
       // |this_object| is NOT thread safe. Only use it to post a task back.
       scoped_refptr<Context> this_object,
       BrowserThread::ID client_thread_id,
       const base::TimeTicks begin_launch_time,
       base::ProcessHandle handle);
 #endif
 
   // Resets the client (the client is going away).
   void ResetClient();
 
+  bool IsStarting() const { return starting_; }

[Charlie Reis, 2014/11/20 18:00:09]

nit: is_starting()

[rvargas (doing something else), 2014/11/20 22:36:14]

Done.

+
+  const base::Process& GetProcess() const { return process_; }

[Charlie Reis, 2014/11/20 18:00:09]

nit: process()

[rvargas (doing something else), 2014/11/20 22:36:14]

Done.

+
+  int exit_code() const { return exit_code_; }
+
+  base::TerminationStatus termination_status() const {
+    return termination_status_;
+  }
+
   void set_terminate_child_on_shutdown(bool terminate_on_shutdown) {
     terminate_child_on_shutdown_ = terminate_on_shutdown;
   }
 
-  void GetTerminationStatus() {
-    termination_status_ =
-        base::GetTerminationStatus(process_.Handle(), &exit_code_);
-  }
+  void UpdateTerminationStatus(bool known_dead);
+
+  void Close() { process_.Close(); }

[Charlie Reis, 2014/11/20 18:00:09]

Minor nit: Perhaps this should not be defined inline, since Process::Closed()
isn't?

[rvargas (doing something else), 2014/11/20 22:36:14]

It is not defined inline to signal it is fast or that should be optimized; it's 
just because it is a one line method.

 
   void SetProcessBackgrounded(bool background);
 
  private:
   friend class base::RefCountedThreadSafe<ChildProcessLauncher::Context>;
-  friend class ChildProcessLauncher;
 
   ~Context() {
     Terminate();
   }
 
   static void RecordHistograms(base::TimeTicks begin_launch_time);
   static void RecordLaunchHistograms(base::TimeDelta launch_time);
 
   // Performs the actual work of launching the process.
   // Runs on the PROCESS_LAUNCHER thread.
@@ skipping 106 matching lines @@
 }
 #endif
 
 void ChildProcessLauncher::Context::ResetClient() {
   // No need for locking as this function gets called on the same thread that
   // client_ would be used.
   CHECK(BrowserThread::CurrentlyOn(client_thread_id_));
   client_ = NULL;
 }
 
+void ChildProcessLauncher::Context::UpdateTerminationStatus(bool known_dead) {
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
+  if (zygote_) {
+    termination_status_ = ZygoteHostImpl::GetInstance()->
+        GetTerminationStatus(process_.Handle(), known_dead, &exit_code_);
+  } else if (known_dead) {
+    termination_status_ =
+        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
+  } else {
+#elif defined(OS_MACOSX)
+  if (known_dead) {
+    termination_status_ =
+        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
+  } else {
+#elif defined(OS_ANDROID)
+  if (IsChildProcessOomProtected(process_.Handle())) {
+    termination_status_ = base::TERMINATION_STATUS_OOM_PROTECTED;
+  } else {
+#else
+  {
+#endif
+    termination_status_ =
+      base::GetTerminationStatus(process_.Handle(), &exit_code_);
+  }
+}
+
 void ChildProcessLauncher::Context::SetProcessBackgrounded(bool background) {
   base::Process to_pass = process_.Duplicate();
   BrowserThread::PostTask(
       BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
       base::Bind(&Context::SetProcessBackgroundedInternal,
                  base::Passed(&to_pass), background));
 }
 
 // static
 void ChildProcessLauncher::Context::RecordHistograms(
@@ skipping 283 matching lines @@
       cmd_line,
       child_process_id,
       client);
 }
 
 ChildProcessLauncher::~ChildProcessLauncher() {
   context_->ResetClient();
 }
 
 bool ChildProcessLauncher::IsStarting() {
-  return context_->starting_;
+  return context_->IsStarting();
 }
 
 const base::Process& ChildProcessLauncher::GetProcess() const {
-  DCHECK(!context_->starting_);
-  return context_->process_;
+  DCHECK(!context_->IsStarting());
+  return context_->GetProcess();
 }
 
 base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
     bool known_dead,
     int* exit_code) {
-  if (!context_->process_.IsValid()) {
+  if (!context_->GetProcess().IsValid()) {
     // Process is already gone, so return the cached termination status.
     if (exit_code)
-      *exit_code = context_->exit_code_;
-    return context_->termination_status_;
-  }
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-  if (context_->zygote_) {
-    context_->termination_status_ = ZygoteHostImpl::GetInstance()->
-        GetTerminationStatus(context_->process_.Handle(), known_dead,
-                             &context_->exit_code_);
-  } else if (known_dead) {
-    context_->termination_status_ =
-        base::GetKnownDeadTerminationStatus(context_->process_.Handle(),
-                                            &context_->exit_code_);
-  } else {
-#elif defined(OS_MACOSX)
-  if (known_dead) {
-    context_->termination_status_ =
-        base::GetKnownDeadTerminationStatus(context_->process_.Handle(),
-                                            &context_->exit_code_);
-  } else {
-#elif defined(OS_ANDROID)
-  if (IsChildProcessOomProtected(context_->process_.Handle())) {
-    context_->termination_status_ = base::TERMINATION_STATUS_OOM_PROTECTED;
-  } else {
-#else
-  {
-#endif
-    context_->GetTerminationStatus();
+      *exit_code = context_->exit_code();
+    return context_->termination_status();
   }
 
+  context_->UpdateTerminationStatus(known_dead);
   if (exit_code)
-    *exit_code = context_->exit_code_;
+    *exit_code = context_->exit_code();
 
   // POSIX: If the process crashed, then the kernel closed the socket
   // for it and so the child has already died by the time we get
   // here. Since GetTerminationStatus called waitpid with WNOHANG,
   // it'll reap the process.  However, if GetTerminationStatus didn't
   // reap the child (because it was still running), we'll need to
   // Terminate via ProcessWatcher. So we can't close the handle here.
-  if (context_->termination_status_ != base::TERMINATION_STATUS_STILL_RUNNING)
-    context_->process_.Close();
+  if (context_->termination_status() != base::TERMINATION_STATUS_STILL_RUNNING)
+    context_->Close();
 
-  return context_->termination_status_;
+  return context_->termination_status();
 }
 
 void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
   context_->SetProcessBackgrounded(background);
 }
 
 void ChildProcessLauncher::SetTerminateChildOnShutdown(
     bool terminate_on_shutdown) {
   if (context_.get())
     context_->set_terminate_child_on_shutdown(terminate_on_shutdown);
 }
 
 }  // namespace content