Query NaCl processes' GDB debug port on the correct thread.

chrome/browser/task_manager/sampling/task_group.cc

 // Copyright 2015 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/task_manager/sampling/task_group.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 13 matching lines @@
 
 // A mask for the refresh types that are done in the background thread.
 const int kBackgroundRefreshTypesMask =
     REFRESH_TYPE_CPU | REFRESH_TYPE_MEMORY | REFRESH_TYPE_IDLE_WAKEUPS |
 #if defined(OS_WIN)
         REFRESH_TYPE_START_TIME | REFRESH_TYPE_CPU_TIME |
 #endif  // defined(OS_WIN)
 #if defined(OS_LINUX)
         REFRESH_TYPE_FD_COUNT |
 #endif  // defined(OS_LINUX)
         REFRESH_TYPE_NACL ||

[Wez, 2017/01/23 19:21:02]

Ick, I think this indentation got messed-up by git cl format in my previous CL
for NaCL debug stub port - noticed a similar issue on another CL - will look
into it.

[Wez, 2017/01/23 19:44:27]

OK, worse than that, the indentation is missed up because of the || in the term
I added in https://codereview.chromium.org/2634573003, which is of course wrong
:(

I'm surprised that this didn't break the Task Manager, since it would have left
it waiting for _everything_ to refresh in the background?

     REFRESH_TYPE_PRIORITY;
 
 #if defined(OS_WIN)
 // Gets the GDI and USER Handles on Windows at one shot.
 void GetWindowsHandles(base::ProcessHandle handle,
                        int64_t* out_gdi_current,
                        int64_t* out_gdi_peak,
                        int64_t* out_user_current,
                        int64_t* out_user_peak) {
   *out_gdi_current = 0;
@@ skipping 38 matching lines @@
       gpu_memory_(-1),
       memory_state_(base::MemoryState::UNKNOWN),
       per_process_network_usage_(-1),
 #if defined(OS_WIN)
       gdi_current_handles_(-1),
       gdi_peak_handles_(-1),
       user_current_handles_(-1),
       user_peak_handles_(-1),
 #endif  // defined(OS_WIN)
 #if !defined(DISABLE_NACL)
-      nacl_debug_stub_port_(-1),
+      nacl_debug_stub_port_(nacl::kGdbDebugStubPortUnknown),
 #endif  // !defined(DISABLE_NACL)
       idle_wakeups_per_second_(-1),
 #if defined(OS_LINUX)
       open_fd_count_(-1),
 #endif  // defined(OS_LINUX)
       gpu_memory_has_duplicates_(false),
       is_backgrounded_(false),
       weak_ptr_factory_(this) {
   scoped_refptr<TaskGroupSampler> sampler(
       new TaskGroupSampler(base::Process::Open(proc_id),
@@ skipping 74 matching lines @@
 #endif  // defined(OS_WIN)
 
 // 4- Refresh the NACL debug stub port (if enabled). This calls out to
 //    NaClBrowser on the browser's IO thread, completing asynchronously.
 #if !defined(DISABLE_NACL)
   if (TaskManagerObserver::IsResourceRefreshEnabled(REFRESH_TYPE_NACL,
                                                     refresh_flags) &&
       !tasks_.empty()) {
     RefreshNaClDebugStubPort(tasks_[0]->GetChildProcessUniqueID());
   }
 #endif  // !defined(DISABLE_NACL)

[Wez, 2017/01/23 19:21:02]

nit: REFRESH_TYPE_NACL exists even in DISABLE_NACL, so should this code be
either asserting that it's not set, or explicitly ignoring it in those builds? 
(Same applies to REFRESH_TYPE_HANDLES, above).

[afakhry, 2017/02/09 01:26:59]

We just used to ignore it since it used to be performed synchronously and we
didn't need to wait for it in order to fire that all background calculations
were performed. 

Now that it is performed on the background, it's still should be OK (provided
that you do my comment on task_group.cc:34 on the last patch) even if the user
requested REFRESH_TYPE_NACL while DISABLE_NACL. 

[Wez, 2017/02/10 02:51:56]

Acknowledged.

 
   int64_t shared_refresh_flags =
       refresh_flags & shared_sampler_->GetSupportedFlags();
 
   // 5- Refresh resources via SharedSampler if the current platform
   // implementation supports that. The actual work is done on the worker thread.
   // At the moment this is supported only on OS_WIN.
   if (shared_refresh_flags != 0) {
     shared_sampler_->Refresh(process_id_, shared_refresh_flags);
     refresh_flags &= ~shared_refresh_flags;
@@ skipping 52 matching lines @@
 #if defined(OS_WIN)
   GetWindowsHandles(process_handle_,
                     &gdi_current_handles_,
                     &gdi_peak_handles_,
                     &user_current_handles_,
                     &user_peak_handles_);
 #endif  // defined(OS_WIN)
 }
 
 #if !defined(DISABLE_NACL)
+static int GetNaClDebugStubPortOnIoThread(int process_id) {
+  return nacl::NaClBrowser::GetInstance()->GetProcessGdbDebugStubPort(
+      process_id);
+}
+
 void TaskGroup::RefreshNaClDebugStubPort(int child_process_unique_id) {
-  nacl::NaClBrowser* nacl_browser = nacl::NaClBrowser::GetInstance();
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&nacl::NaClBrowser::GetProcessGdbDebugStubPort,
-                 base::Unretained(nacl_browser), child_process_unique_id),
+      base::Bind(&GetNaClDebugStubPortOnIoThread, child_process_unique_id),
       base::Bind(&TaskGroup::OnRefreshNaClDebugStubPortDone,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TaskGroup::OnRefreshNaClDebugStubPortDone(int nacl_debug_stub_port) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   nacl_debug_stub_port_ = nacl_debug_stub_port;
   OnBackgroundRefreshTypeFinished(REFRESH_TYPE_NACL);
 }
@@ skipping 67 matching lines @@
 
 void TaskGroup::OnBackgroundRefreshTypeFinished(int64_t finished_refresh_type) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   current_on_bg_done_flags_ |= finished_refresh_type;
   if (AreBackgroundCalculationsDone())
     on_background_calculations_done_.Run();
 }
 
 }  // namespace task_manager