Commit 46afb301 authored by afakhry's avatar afakhry Committed by Commit bot

Fix various TaskManager bugs and add new enhancements

The following items are included in this CL.

1- Javascipt column for child processes that don't use V8 memory should always show "N/A".
2- Idle wakeups should show up only once per process.
3- NaCl debug ports column shows "Disabled" when the enable-nacl-debug flag is disabled.
4- NaCl modules should show the name of the app/extension instead of the URL.
5- Network usage can now be reported per process.

R=nick@chromium.org
BUG=551580

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

Cr-Commit-Position: refs/heads/master@{#361152}
parent 2914df72
......@@ -3624,6 +3624,9 @@ Even if you have downloaded files from this website before, the website might ha
<message name="IDS_TASK_MANAGER_UNKNOWN_VALUE_TEXT" desc="The value displayed for an unknown NaCl debug stub port in the task manager.">
Unknown
</message>
<message name="IDS_TASK_MANAGER_DISABLED_NACL_DBG_TEXT" desc="The value displayed for NaCl debug stub port in the task manager when the flag #enable-nacl-debug is disabled.">
Disabled
</message>
<message name="IDS_TASK_MANAGER_HANDLES_CELL_TEXT" desc="The value displayed in the user/gdi cells.">
<ph name="NUM_HANDLES">$1<ex>300</ex></ph> (<ph name="NUM_KILOBYTES_LIVE">$2<ex>500</ex></ph> peak)
</message>
......
......@@ -6,7 +6,10 @@
#include "base/i18n/rtl.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/process_resource_usage.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/task_management/task_manager_observer.h"
#include "chrome/grit/generated_resources.h"
#include "components/nacl/common/nacl_process_type.h"
......@@ -15,6 +18,8 @@
#include "content/public/browser/child_process_data.h"
#include "content/public/common/process_type.h"
#include "content/public/common/service_registry.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension_set.h"
#include "grit/theme_resources.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
......@@ -72,9 +77,26 @@ base::string16 GetLocalizedTitle(const base::string16& title,
result_title);
case PROCESS_TYPE_NACL_BROKER:
return l10n_util::GetStringUTF16(IDS_TASK_MANAGER_NACL_BROKER_PREFIX);
case PROCESS_TYPE_NACL_LOADER:
case PROCESS_TYPE_NACL_LOADER: {
auto* profile_manager = g_browser_process->profile_manager();
if (profile_manager) {
// TODO(afakhry): Fix the below looping by plumbing a way to get the
// profile or the profile path from the child process host if any.
auto loaded_profiles = profile_manager->GetLoadedProfiles();
for (auto* profile : loaded_profiles) {
auto& enabled_extensions =
extensions::ExtensionRegistry::Get(profile)->enabled_extensions();
auto extension =
enabled_extensions.GetExtensionOrAppByURL(GURL(result_title));
if (extension) {
result_title = base::UTF8ToUTF16(extension->name());
break;
}
}
}
return l10n_util::GetStringFUTF16(IDS_TASK_MANAGER_NACL_PREFIX,
result_title);
}
// These types don't need display names or get them from elsewhere.
case content::PROCESS_TYPE_BROWSER:
case content::PROCESS_TYPE_RENDERER:
......@@ -127,6 +149,18 @@ ProcessResourceUsage* CreateProcessResourcesSampler(
return new ProcessResourceUsage(service.Pass());
}
bool UsesV8Memory(int process_type) {
switch (process_type) {
case content::PROCESS_TYPE_UTILITY:
case content::PROCESS_TYPE_BROWSER:
case content::PROCESS_TYPE_RENDERER:
return true;
default:
return false;
}
}
} // namespace
ChildProcessTask::ChildProcessTask(const content::ChildProcessData& data)
......@@ -137,7 +171,8 @@ ChildProcessTask::ChildProcessTask(const content::ChildProcessData& data)
v8_memory_allocated_(-1),
v8_memory_used_(-1),
unique_child_process_id_(data.id),
process_type_(data.process_type) {
process_type_(data.process_type),
uses_v8_memory_(UsesV8Memory(process_type_)) {
}
ChildProcessTask::~ChildProcessTask() {
......@@ -150,6 +185,9 @@ void ChildProcessTask::Refresh(const base::TimeDelta& update_interval,
if ((refresh_flags & REFRESH_TYPE_V8_MEMORY) == 0)
return;
if (!uses_v8_memory_)
return;
// The child process resources refresh is performed asynchronously, we will
// invoke it and record the current values (which might be invalid at the
// moment. We can safely ignore that and count on future refresh cycles
......@@ -190,7 +228,7 @@ int ChildProcessTask::GetChildProcessUniqueID() const {
}
bool ChildProcessTask::ReportsV8Memory() const {
return process_resources_sampler_->ReportsV8MemoryStats();
return uses_v8_memory_ && process_resources_sampler_->ReportsV8MemoryStats();
}
int64 ChildProcessTask::GetV8MemoryAllocated() const {
......
......@@ -51,6 +51,10 @@ class ChildProcessTask : public Task {
// |NaClTrustedProcessType|.
const int process_type_;
// Depending on the |process_type_|, determines whether this task uses V8
// memory or not.
const bool uses_v8_memory_;
DISALLOW_COPY_AND_ASSIGN(ChildProcessTask);
};
......
......@@ -4,6 +4,10 @@
#include "chrome/browser/task_management/providers/task.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_info_cache.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/task_management/task_manager_observer.h"
namespace task_management {
......@@ -31,6 +35,19 @@ Task::Task(const base::string16& title,
Task::~Task() {
}
// static
base::string16 Task::GetProfileNameFromProfile(Profile* profile) {
DCHECK(profile);
ProfileInfoCache& cache =
g_browser_process->profile_manager()->GetProfileInfoCache();
size_t index =
cache.GetIndexOfProfileWithPath(profile->GetOriginalProfile()->GetPath());
if (index != std::string::npos)
return cache.GetNameOfProfileAtIndex(index);
return base::string16();
}
void Task::Activate() {
}
......
......@@ -12,6 +12,8 @@
#include "third_party/WebKit/public/web/WebCache.h"
#include "ui/gfx/image/image_skia.h"
class Profile;
namespace task_management {
// Defines a task that corresponds to a tab, an app, an extension, ... etc. It
......@@ -43,6 +45,9 @@ class Task {
base::ProcessHandle handle);
virtual ~Task();
// Gets the name of the given |profile| from the ProfileInfoCache.
static base::string16 GetProfileNameFromProfile(Profile* profile);
// Activates this TaskManager's task by bringing its container to the front
// (if possible).
virtual void Activate();
......@@ -69,8 +74,7 @@ class Task {
// value is whatever unique IDs of their hosts in the browser process.
virtual int GetChildProcessUniqueID() const = 0;
// The name of the profile associated with the browser context of the render
// view host that this task represents (if this task represents a renderer).
// The name of the profile owning this task.
virtual base::string16 GetProfileName() const;
// Getting the Sqlite used memory (in bytes). Not all tasks reports Sqlite
......
......@@ -11,8 +11,6 @@
#include "chrome/browser/favicon/favicon_utils.h"
#include "chrome/browser/process_resource_usage.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_info_cache.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/task_management/task_manager_observer.h"
#include "chrome/grit/generated_resources.h"
#include "content/public/browser/render_process_host.h"
......@@ -44,15 +42,7 @@ base::string16 GetRendererProfileName(
const content::RenderProcessHost* render_process_host) {
Profile* profile =
Profile::FromBrowserContext(render_process_host->GetBrowserContext());
DCHECK(profile);
ProfileInfoCache& cache =
g_browser_process->profile_manager()->GetProfileInfoCache();
size_t index =
cache.GetIndexOfProfileWithPath(profile->GetOriginalProfile()->GetPath());
if (index != std::string::npos)
return cache.GetNameOfProfileAtIndex(index);
return base::string16();
return Task::GetProfileNameFromProfile(profile);
}
inline bool IsRendererResourceSamplingDisabled(int64 flags) {
......
......@@ -63,6 +63,7 @@ TaskGroup::TaskGroup(
cpu_usage_(0.0),
memory_usage_(),
gpu_memory_(-1),
per_process_network_usage_(-1),
#if defined(OS_WIN)
gdi_current_handles_(-1),
gdi_peak_handles_(-1),
......@@ -112,9 +113,18 @@ void TaskGroup::Refresh(
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// First refresh the enabled non-expensive resources usages on the UI thread.
// 1- Refresh all the tasks.
for (auto& task_pair : tasks_)
task_pair.second->Refresh(update_interval, refresh_flags);
// 1- Refresh all the tasks as well as the total network usage (if enabled).
const bool network_usage_refresh_enabled =
IsResourceRefreshEnabled(REFRESH_TYPE_NETWORK_USAGE, refresh_flags);
per_process_network_usage_ = network_usage_refresh_enabled ? 0 : -1;
for (auto& task_pair : tasks_) {
Task* task = task_pair.second;
task->Refresh(update_interval, refresh_flags);
if (network_usage_refresh_enabled && task->ReportsNetworkUsage()) {
per_process_network_usage_ += task->network_usage();
}
}
// 2- Refresh GPU memory (if enabled).
if (IsResourceRefreshEnabled(REFRESH_TYPE_GPU_MEMORY, refresh_flags))
......
......@@ -54,6 +54,7 @@ class TaskGroup {
int64 physical_bytes() const { return memory_usage_.physical_bytes; }
int64 gpu_memory() const { return gpu_memory_; }
bool gpu_memory_has_duplicates() const { return gpu_memory_has_duplicates_; }
int64 per_process_network_usage() const { return per_process_network_usage_; }
#if defined(OS_WIN)
int64 gdi_current_handles() const { return gdi_current_handles_; }
......@@ -97,6 +98,9 @@ class TaskGroup {
double cpu_usage_;
MemoryUsageStats memory_usage_;
int64 gpu_memory_;
// The network usage in bytes per second as the sum of all network usages of
// the individual tasks sharing the same process.
int64 per_process_network_usage_;
#if defined(OS_WIN)
// Windows GDI and USER Handles.
int64 gdi_current_handles_;
......
......@@ -92,7 +92,7 @@ int TaskManagerImpl::GetNaClDebugStubPort(TaskId task_id) const {
#if !defined(DISABLE_NACL)
return GetTaskGroupByTaskId(task_id)->nacl_debug_stub_port();
#else
return -1;
return -2;
#endif // !defined(DISABLE_NACL)
}
......@@ -151,6 +151,10 @@ int64 TaskManagerImpl::GetNetworkUsage(TaskId task_id) const {
return GetTaskByTaskId(task_id)->network_usage();
}
int64 TaskManagerImpl::GetProcessTotalNetworkUsage(TaskId task_id) const {
return GetTaskGroupByTaskId(task_id)->per_process_network_usage();
}
int64 TaskManagerImpl::GetSqliteMemoryUsed(TaskId task_id) const {
return GetTaskByTaskId(task_id)->GetSqliteMemoryUsed();
}
......
......@@ -51,6 +51,7 @@ class TaskManagerImpl :
const base::ProcessId& GetProcessId(TaskId task_id) const override;
Task::Type GetType(TaskId task_id) const override;
int64 GetNetworkUsage(TaskId task_id) const override;
int64 GetProcessTotalNetworkUsage(TaskId task_id) const override;
int64 GetSqliteMemoryUsed(TaskId task_id) const override;
bool GetV8Memory(TaskId task_id,
int64* allocated,
......
......@@ -66,8 +66,9 @@ class TaskManagerInterface {
// refresh cycle. A value of -1 means no valid value is currently available.
virtual int GetIdleWakeupsPerSecond(TaskId task_id) const = 0;
// Returns the NaCl GDB debug stub port. A value of -1 means no valid value is
// currently available.
// Returns the NaCl GDB debug stub port. A value of
// |nacl::kGdbDebugStubPortUnknown| means no valid value is currently
// available. A value of -2 means NaCl is not enabled for this build.
virtual int GetNaClDebugStubPort(TaskId task_id) const = 0;
// On Windows, gets the current and peak number of GDI and USER handles in
......@@ -104,6 +105,13 @@ class TaskManagerInterface {
// usage.
virtual int64 GetNetworkUsage(TaskId task_id) const = 0;
// Returns the total network usage (in bytes per second) during the current
// refresh cycle for the process on which the task with |task_id| is running.
// This is the sum of all the network usage of the individual tasks (that
// can be gotten by the above GetNetworkUsage()). A value of -1 means network
// usage calculation refresh is currently not available.
virtual int64 GetProcessTotalNetworkUsage(TaskId task_id) const = 0;
// Returns the Sqlite used memory (in bytes) for the task with |task_id|.
// A value of -1 means no valid value is currently available.
virtual int64 GetSqliteMemoryUsed(TaskId task_id) const = 0;
......
......@@ -58,6 +58,9 @@ class TestTaskManager : public TaskManagerInterface {
}
Task::Type GetType(TaskId task_id) const override { return Task::UNKNOWN; }
int64 GetNetworkUsage(TaskId task_id) const override { return -1; }
int64 GetProcessTotalNetworkUsage(TaskId task_id) const override {
return -1;
}
int64 GetSqliteMemoryUsed(TaskId task_id) const override { return -1; }
bool GetV8Memory(TaskId task_id,
int64* allocated,
......
......@@ -4,6 +4,7 @@
#include "chrome/browser/ui/task_manager/task_manager_table_model.h"
#include "base/command_line.h"
#include "base/i18n/number_formatting.h"
#include "base/i18n/rtl.h"
#include "base/prefs/scoped_user_pref_update.h"
......@@ -17,6 +18,7 @@
#include "chrome/common/pref_names.h"
#include "chrome/grit/generated_resources.h"
#include "components/nacl/browser/nacl_browser.h"
#include "components/nacl/common/nacl_switches.h"
#include "content/public/common/result_codes.h"
#include "third_party/WebKit/public/web/WebCache.h"
#include "ui/base/l10n/l10n_util.h"
......@@ -47,6 +49,7 @@ bool IsSharedByGroup(int column_id) {
case IDS_TASK_MANAGER_SHARED_MEM_COLUMN:
case IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN:
case IDS_TASK_MANAGER_CPU_COLUMN:
case IDS_TASK_MANAGER_NET_COLUMN:
case IDS_TASK_MANAGER_PROCESS_ID_COLUMN:
case IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN:
case IDS_TASK_MANAGER_VIDEO_MEMORY_COLUMN:
......@@ -55,6 +58,7 @@ bool IsSharedByGroup(int column_id) {
case IDS_TASK_MANAGER_WEBCORE_SCRIPTS_CACHE_COLUMN:
case IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN:
case IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN:
case IDS_TASK_MANAGER_IDLE_WAKEUPS_COLUMN:
return true;
default:
return false;
......@@ -87,7 +91,9 @@ class TaskManagerValuesStringifier {
zero_string_(base::ASCIIToUTF16("0")),
asterisk_string_(base::ASCIIToUTF16("*")),
unknown_string_(l10n_util::GetStringUTF16(
IDS_TASK_MANAGER_UNKNOWN_VALUE_TEXT)) {
IDS_TASK_MANAGER_UNKNOWN_VALUE_TEXT)),
disabled_nacl_debugging_string_(l10n_util::GetStringUTF16(
IDS_TASK_MANAGER_DISABLED_NACL_DBG_TEXT)) {
}
~TaskManagerValuesStringifier() {}
......@@ -128,7 +134,7 @@ class TaskManagerValuesStringifier {
}
base::string16 GetNaClPortText(int nacl_port) {
if (nacl_port == nacl::kGdbDebugStubPortUnused)
if (nacl_port == nacl::kGdbDebugStubPortUnused || nacl_port == -2)
return n_a_string_;
if (nacl_port == nacl::kGdbDebugStubPortUnknown)
......@@ -175,6 +181,9 @@ class TaskManagerValuesStringifier {
const base::string16& zero_string() const { return zero_string_; }
const base::string16& asterisk_string() const { return asterisk_string_; }
const base::string16& unknown_string() const { return unknown_string_; }
const base::string16& disabled_nacl_debugging_string() const {
return disabled_nacl_debugging_string_;
}
private:
// The localized string "N/A".
......@@ -190,6 +199,10 @@ class TaskManagerValuesStringifier {
// The string "Unknown".
const base::string16 unknown_string_;
// The string to show on the NaCl debug port column cells when the flag
// #enable-nacl-debug is disabled.
const base::string16 disabled_nacl_debugging_string_;
DISALLOW_COPY_AND_ASSIGN(TaskManagerValuesStringifier);
};
......@@ -218,7 +231,13 @@ TaskManagerTableModel::TaskManagerTableModel(int64_t refresh_flags,
table_view_delegate_(delegate),
columns_settings_(new base::DictionaryValue),
table_model_observer_(nullptr),
stringifier_(new TaskManagerValuesStringifier) {
stringifier_(new TaskManagerValuesStringifier),
#if !defined(DISABLE_NACL)
is_nacl_debugging_flag_enabled_(base::CommandLine::ForCurrentProcess()->
HasSwitch(switches::kEnableNaClDebug)) {
#else
is_nacl_debugging_flag_enabled_(false) {
#endif // !defined(DISABLE_NACL)
DCHECK(delegate);
}
......@@ -242,7 +261,7 @@ base::string16 TaskManagerTableModel::GetText(int row, int column) {
case IDS_TASK_MANAGER_NET_COLUMN:
return stringifier_->GetNetworkUsageText(
observed_task_manager()->GetNetworkUsage(tasks_[row]));
observed_task_manager()->GetProcessTotalNetworkUsage(tasks_[row]));
case IDS_TASK_MANAGER_CPU_COLUMN:
return stringifier_->GetCpuUsageText(
......@@ -325,6 +344,9 @@ base::string16 TaskManagerTableModel::GetText(int row, int column) {
}
case IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN:
if (!is_nacl_debugging_flag_enabled_)
return stringifier_->disabled_nacl_debugging_string();
return stringifier_->GetNaClPortText(
observed_task_manager()->GetNaClDebugStubPort(tasks_[row]));
......
......@@ -131,6 +131,9 @@ class TaskManagerTableModel
// values to string16.
scoped_ptr<TaskManagerValuesStringifier> stringifier_;
// The status of the flag #enable-nacl-debug.
bool is_nacl_debugging_flag_enabled_;
DISALLOW_COPY_AND_ASSIGN(TaskManagerTableModel);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment