ChildProcessLauncher: Don't access context_ private members directly.

content/browser/child_process_launcher.cc

Index: content/browser/child_process_launcher.cc
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index c4e372c1b4b0219205d202e033a06d31708a725c..7893d23ab6627b0134f0d51396838ffa53962ba6 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -78,20 +78,28 @@ class ChildProcessLauncher::Context
   // 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();
@@ -218,6 +226,32 @@ void ChildProcessLauncher::Context::ResetClient() {
   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(
@@ -521,51 +555,27 @@ ChildProcessLauncher::~ChildProcessLauncher() {
 }
 
 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
@@ -573,10 +583,10 @@ base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
   // 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) {