Remove knowledge of nacl from content.

Content should not have knowledge of NaCl. Moving per process NaCl debug
stub port information to NaClBrowser.

More explicitly distiquish between the case where the debug port is not yet
known versus when it is unused.

Switch debug port to being a per-process value (as it is).

This relands r270907 with a fix to make it work with disable_nacl=1
and therefore ASAN.

BUG=None
TEST=trybots
R=jam@chromium.org,sehr@chromium.org

Review URL: https://codereview.chromium.org/286143010

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

chrome/browser/task_manager/child_process_resource_provider.cc

@@ -31,12 +31,10 @@ class ChildProcessResource : public Resource {
   ChildProcessResource(int process_type,
                        const base::string16& name,
                        base::ProcessHandle handle,
-                       int unique_process_id,
-                       int nacl_debug_stub_port);
+                       int unique_process_id);
   virtual ~ChildProcessResource();
 
   // Resource methods:
-  virtual int GetNaClDebugStubPort() const OVERRIDE;
   virtual base::string16 GetTitle() const OVERRIDE;
   virtual base::string16 GetProfileName() const OVERRIDE;
   virtual gfx::ImageSkia GetIcon() const OVERRIDE;
@@ -59,7 +57,6 @@ class ChildProcessResource : public Resource {
   base::ProcessHandle handle_;
   int pid_;
   int unique_process_id_;
-  int nacl_debug_stub_port_;
   mutable base::string16 title_;
   bool network_usage_support_;
 
@@ -77,13 +74,11 @@ ChildProcessResource::ChildProcessResource(
     int process_type,
     const base::string16& name,
     base::ProcessHandle handle,
-    int unique_process_id,
-    int nacl_debug_stub_port)
+    int unique_process_id)
     : process_type_(process_type),
       name_(name),
       handle_(handle),
       unique_process_id_(unique_process_id),
-      nacl_debug_stub_port_(nacl_debug_stub_port),
       network_usage_support_(false) {
   // We cache the process id because it's not cheap to calculate, and it won't
   // be available when we get the plugin disconnected notification.
@@ -99,10 +94,6 @@ ChildProcessResource::~ChildProcessResource() {
 }
 
 // Resource methods:
-int ChildProcessResource::GetNaClDebugStubPort() const {
-  return nacl_debug_stub_port_;
-}
-
 base::string16 ChildProcessResource::GetTitle() const {
   if (title_.empty())
     title_ = GetLocalizedTitle();
@@ -318,8 +309,7 @@ void ChildProcessResourceProvider::AddToTaskManager(
           child_process_data.process_type,
           child_process_data.name,
           child_process_data.handle,
-          child_process_data.id,
-          child_process_data.nacl_debug_stub_port);
+          child_process_data.id);
   resources_[child_process_data.handle] = resource;
   pid_to_resources_[resource->process_id()] = resource;
   task_manager_->AddResource(resource);

chrome/browser/task_manager/resource_provider.cc

@@ -56,10 +56,6 @@ size_t Resource::GetV8MemoryUsed() const {
   return 0;
 }
 
-int Resource::GetNaClDebugStubPort() const {
-  return 0;
-}
-
 bool Resource::CanInspect() const {
   return false;
 }

chrome/browser/task_manager/resource_provider.h

@@ -77,8 +77,6 @@ class Resource {
   virtual size_t GetV8MemoryAllocated() const;
   virtual size_t GetV8MemoryUsed() const;
 
-  virtual int GetNaClDebugStubPort() const;
-
   // Returns true if this resource can be inspected using developer tools.
   virtual bool CanInspect() const;
 

chrome/browser/task_manager/task_manager.cc

@@ -31,6 +31,7 @@
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "components/nacl/browser/nacl_browser.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/gpu_data_manager_observer.h"
@@ -191,9 +192,7 @@ class TaskManagerModelGpuDataManagerObserver
 };
 
 TaskManagerModel::PerResourceValues::PerResourceValues()
-    : is_nacl_debug_stub_port_valid(false),
-      nacl_debug_stub_port(0),
-      is_title_valid(false),
+    : is_title_valid(false),
       is_profile_name_valid(false),
       network_usage(0),
       is_process_id_valid(false),
@@ -229,7 +228,9 @@ TaskManagerModel::PerProcessValues::PerProcessValues()
       gdi_handles_peak(0),
       is_user_handles_valid(0),
       user_handles(0),
-      user_handles_peak(0) {}
+      user_handles_peak(0),
+      is_nacl_debug_stub_port_valid(false),
+      nacl_debug_stub_port(0) {}
 
 TaskManagerModel::PerProcessValues::~PerProcessValues() {}
 
@@ -292,10 +293,10 @@ int TaskManagerModel::GroupCount() const {
 }
 
 int TaskManagerModel::GetNaClDebugStubPort(int index) const {
-  PerResourceValues& values(GetPerResourceValues(index));
+  base::ProcessHandle handle = GetResource(index)->GetProcess();
+  PerProcessValues& values(per_process_cache_[handle]);
   if (!values.is_nacl_debug_stub_port_valid) {
-    values.is_nacl_debug_stub_port_valid = true;
-    values.nacl_debug_stub_port = GetResource(index)->GetNaClDebugStubPort();
+    return nacl::kGdbDebugStubPortUnknown;
   }
   return values.nacl_debug_stub_port;
 }
@@ -417,7 +418,9 @@ const base::string16& TaskManagerModel::GetResourceProfileName(
 
 base::string16 TaskManagerModel::GetResourceNaClDebugStubPort(int index) const {
   int port = GetNaClDebugStubPort(index);
-  if (port == 0) {
+  if (port == nacl::kGdbDebugStubPortUnknown) {
+    return base::ASCIIToUTF16("Unknown");
+  } else if (port == nacl::kGdbDebugStubPortUnused) {
     return base::ASCIIToUTF16("N/A");
   } else {
     return base::IntToString16(port);
@@ -1179,7 +1182,12 @@ void TaskManagerModel::Refresh() {
   per_resource_cache_.clear();
   per_process_cache_.clear();
 
-  // Compute the CPU usage values.
+#if !defined(DISABLE_NACL)
+  nacl::NaClBrowser* nacl_browser = nacl::NaClBrowser::GetInstance();
+#endif  // !defined(DISABLE_NACL)
+
+  // Compute the CPU usage values and check if NaCl GDB debug stub port is
+  // known.
   // Note that we compute the CPU usage for all resources (instead of doing it
   // lazily) as process_util::GetCPUUsage() returns the CPU usage since the last
   // time it was called, and not calling it everytime would skew the value the
@@ -1189,6 +1197,16 @@ void TaskManagerModel::Refresh() {
        iter != resources_.end(); ++iter) {
     base::ProcessHandle process = (*iter)->GetProcess();
     PerProcessValues& values(per_process_cache_[process]);
+#if !defined(DISABLE_NACL)
+    // Debug stub port doesn't change once known.
+    if (!values.is_nacl_debug_stub_port_valid) {
+      values.nacl_debug_stub_port = nacl_browser->GetProcessGdbDebugStubPort(
+          (*iter)->GetUniqueChildProcessId());
+      if (values.nacl_debug_stub_port != nacl::kGdbDebugStubPortUnknown) {
+        values.is_nacl_debug_stub_port_valid = true;
+      }
+    }
+#endif  // !defined(DISABLE_NACL)
     if (values.is_cpu_usage_valid && values.is_idle_wakeups_valid)
       continue;
     MetricsMap::iterator metrics_iter = metrics_map_.find(process);

chrome/browser/task_manager/task_manager.h

@@ -351,9 +351,6 @@ class TaskManagerModel : public base::RefCountedThreadSafe<TaskManagerModel> {
     PerResourceValues();
     ~PerResourceValues();
 
-    bool is_nacl_debug_stub_port_valid;
-    int nacl_debug_stub_port;
-
     bool is_title_valid;
     base::string16 title;
 
@@ -413,6 +410,9 @@ class TaskManagerModel : public base::RefCountedThreadSafe<TaskManagerModel> {
     bool is_user_handles_valid;
     size_t user_handles;
     size_t user_handles_peak;
+
+    bool is_nacl_debug_stub_port_valid;
+    int nacl_debug_stub_port;
   };
 
   typedef std::vector<task_manager::Resource*> ResourceList;

components/nacl/browser/nacl_browser.cc

@@ -272,15 +272,15 @@ void NaClBrowser::OnIrtOpened(scoped_ptr<base::FileProxy> file_proxy,
   CheckWaiting();
 }
 
-void NaClBrowser::FireGdbDebugStubPortOpened(int port) {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(debug_stub_port_listener_, port));
-}
-
-bool NaClBrowser::HasGdbDebugStubPortListener() {
-  return !debug_stub_port_listener_.is_null();
+void NaClBrowser::SetProcessGdbDebugStubPort(int process_id, int port) {
+  gdb_debug_stub_port_map_[process_id] = port;
+  if (port != kGdbDebugStubPortUnknown &&
+      !debug_stub_port_listener_.is_null()) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(debug_stub_port_listener_, port));
+  }
 }
 
 void NaClBrowser::SetGdbDebugStubPortListener(
@@ -292,6 +292,14 @@ void NaClBrowser::ClearGdbDebugStubPortListener() {
   debug_stub_port_listener_.Reset();
 }
 
+int NaClBrowser::GetProcessGdbDebugStubPort(int process_id) {
+  GdbDebugStubPortMap::iterator i = gdb_debug_stub_port_map_.find(process_id);
+  if (i != gdb_debug_stub_port_map_.end()) {
+    return i->second;
+  }
+  return kGdbDebugStubPortUnused;
+}
+
 void NaClBrowser::InitValidationCacheFilePath() {
   // Determine where the validation cache resides in the file system.  It
   // exists in Chrome's cache directory and is not tied to any specific
@@ -534,6 +542,10 @@ void NaClBrowser::PersistValidationCache() {
   validation_cache_is_modified_ = false;
 }
 
+void NaClBrowser::OnProcessEnd(int process_id) {
+  gdb_debug_stub_port_map_.erase(process_id);
+}
+
 void NaClBrowser::OnProcessCrashed() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
   if (crash_times_.size() == kMaxCrashesPerInterval) {

components/nacl/browser/nacl_browser.h

@@ -25,6 +25,9 @@ class FileProxy;
 
 namespace nacl {
 
+static const int kGdbDebugStubPortUnknown = -1;
+static const int kGdbDebugStubPortUnused = 0;
+
 // Open an immutable executable file that can be mmapped.
 // This function should only be called on a thread that can perform file IO.
 base::File OpenNaClExecutableImpl(const base::FilePath& file_path);
@@ -65,11 +68,12 @@ class NaClBrowser {
   // debug stub server instead of a fixed one.
 
   // Notify listener that new debug stub TCP port is allocated.
-  void FireGdbDebugStubPortOpened(int port);
-  bool HasGdbDebugStubPortListener();
+  void SetProcessGdbDebugStubPort(int process_id, int port);
   void SetGdbDebugStubPortListener(base::Callback<void(int)> listener);
   void ClearGdbDebugStubPortListener();
 
+  int GetProcessGdbDebugStubPort(int process_id);
+
   bool ValidationCacheIsEnabled() const {
     return validation_cache_is_enabled_;
   }
@@ -117,6 +121,8 @@ class NaClBrowser {
   static void SetDelegate(NaClBrowserDelegate* delegate);
   static NaClBrowserDelegate* GetDelegate();
 
+  // Each time a NaCl process ends, the browser is notified.
+  void OnProcessEnd(int process_id);
   // Support for NaCl crash throttling.
   // Each time a NaCl module crashes, the browser is notified.
   void OnProcessCrashed();
@@ -171,6 +177,10 @@ class NaClBrowser {
   NaClResourceState validation_cache_state_;
   base::Callback<void(int)> debug_stub_port_listener_;
 
+  // Map from process id to debug stub port if any.
+  typedef std::map<int, int> GdbDebugStubPortMap;
+  GdbDebugStubPortMap gdb_debug_stub_port_map_;
+
   typedef base::HashingMRUCache<std::string, base::FilePath> PathCacheType;
   PathCacheType path_cache_;
 

components/nacl/browser/nacl_process_host.cc

@@ -312,6 +312,7 @@ NaClProcessHost::~NaClProcessHost() {
     } else {
       LOG(ERROR) << message;
     }
+    NaClBrowser::GetInstance()->OnProcessEnd(process_->GetData().id);
   }
 
   if (internal_->socket_for_renderer != NACL_INVALID_HANDLE) {
@@ -351,6 +352,7 @@ void NaClProcessHost::OnProcessCrashed(int exit_status) {
 // static
 void NaClProcessHost::EarlyStartup() {
   NaClBrowser::GetInstance()->EarlyStartup();
+  // Inform NaClBrowser that we exist and will have a debug port at some point.
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   // Open the IRT file early to make sure that it isn't replaced out from
   // under us by autoupdate.
@@ -491,6 +493,7 @@ void NaClProcessHost::OnChannelConnected(int32 peer_pid) {
 void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) {
   process_launched_by_broker_ = true;
   process_->SetHandle(handle);
+  SetDebugStubPort(nacl::kGdbDebugStubPortUnknown);
   if (!StartWithLaunchedProcess())
     delete this;
 }
@@ -766,13 +769,9 @@ void NaClProcessHost::SendMessageToRenderer(
   }
 }
 
-void NaClProcessHost::SetDebugStubPort(uint16_t port) {
+void NaClProcessHost::SetDebugStubPort(int port) {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
-  if (nacl_browser->HasGdbDebugStubPortListener()) {
-    nacl_browser->FireGdbDebugStubPortOpened(port);
-  }
-  // Set debug stub port on the process object.
-  process_->SetNaClDebugStubPort(port);
+  nacl_browser->SetProcessGdbDebugStubPort(process_->GetData().id, port);
 }
 
 #if defined(OS_POSIX)

components/nacl/browser/nacl_process_host.h

@@ -107,8 +107,8 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   bool LaunchNaClGdb();
 
   // Mark the process as using a particular GDB debug stub port and notify
-  // listeners.
-  void SetDebugStubPort(uint16_t port);
+  // listeners (if the port is not kGdbDebugStubPortUnknown).
+  void SetDebugStubPort(int port);
 
 #if defined(OS_POSIX)
   // Create bound TCP socket in the browser process so that the NaCl GDB debug

content/browser/browser_child_process_host_impl.cc

@@ -177,11 +177,6 @@ base::ProcessHandle BrowserChildProcessHostImpl::GetHandle() const {
   return child_process_->GetHandle();
 }
 
-void BrowserChildProcessHostImpl::SetNaClDebugStubPort(int port) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  data_.nacl_debug_stub_port = port;
-}
-
 void BrowserChildProcessHostImpl::SetName(const base::string16& name) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   data_.name = name;

content/browser/browser_child_process_host_impl.h

@@ -53,7 +53,6 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl
   virtual ChildProcessHost* GetHost() const OVERRIDE;
   virtual base::TerminationStatus GetTerminationStatus(
       bool known_dead, int* exit_code) OVERRIDE;
-  virtual void SetNaClDebugStubPort(int port) OVERRIDE;
   virtual void SetName(const base::string16& name) OVERRIDE;
   virtual void SetHandle(base::ProcessHandle handle) OVERRIDE;
 

content/public/browser/browser_child_process_host.h

@@ -71,9 +71,6 @@ class CONTENT_EXPORT BrowserChildProcessHost : public IPC::Sender {
   // this object.
   virtual void SetHandle(base::ProcessHandle handle) = 0;
 
-  // Set the nacl debug stub port of the process.
-  virtual void SetNaClDebugStubPort(int port) = 0;
-
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   // Returns a PortProvider used to get process metrics for child processes.
   static base::ProcessMetrics::PortProvider* GetPortProvider();

content/public/browser/child_process_data.h

@@ -28,11 +28,8 @@ struct ChildProcessData {
   // The handle to the process.
   base::ProcessHandle handle;
 
-  int nacl_debug_stub_port;
-
   explicit ChildProcessData(int process_type)
-      : process_type(process_type), id(0), handle(base::kNullProcessHandle),
-        nacl_debug_stub_port(0) {
+      : process_type(process_type), id(0), handle(base::kNullProcessHandle) {
   }
 };