Skip to content

GitLab

T
tangled
Commit f925f56d authored by dpapad's avatar dpapad Committed by Commit Bot
Browse files
Options

Migrate chrome://memory-internals to JS modules. 

Also performing several other small cleanups
 1) Eliminate usage of CallJavascriptFunctionUnsafe()
 2) Eliminate global setSaveDumpMessage in favor of a WebUI event.
 3) Eliminate global returnProcessList in favor of a Promise.
 4) Remove bad calls to AllowJavascript(). This method should not
    be called within async C++ callbacks.
 5) Add JS type checking.
 6) Move "Refresh" and "Save" button creation in HTML, no need to
    do this in JS.
 7) Remove unnecessary flattenhtml="true" in grd file.

Bug: 1028829
Change-Id: I7841982bef7957b7178171ab50211f93deb1d672
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956446
Auto-Submit: dpapad <dpapad@chromium.org>
Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Reviewed-by: default avatarJohn Lee <johntlee@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759075}
parent 4fdc53d7
Show whitespace changes
Inline Side-by-side
Showing with 100 additions and 85 deletions
chrome/browser/dev_ui_browser_resources.grd
View file @ f925f56d
...@@ -50,7 +50,7 @@ This file specifies browser resources for developer-facing chrome:// pages ...@@ -50,7 +50,7 @@ This file specifies browser resources for developer-facing chrome:// pages
<include name="IDR_MEDIA_HISTORY_HTML" file="resources\media\media_history.html" flattenhtml="true" type="BINDATA" compress="gzip" allowexternalscript="true" /> <include name="IDR_MEDIA_HISTORY_HTML" file="resources\media\media_history.html" flattenhtml="true" type="BINDATA" compress="gzip" allowexternalscript="true" />
<include name="IDR_MEDIA_HISTORY_JS" file="resources\media\media_history.js" flattenhtml="true" type="BINDATA" compress="gzip" /> <include name="IDR_MEDIA_HISTORY_JS" file="resources\media\media_history.js" flattenhtml="true" type="BINDATA" compress="gzip" />
<include name="IDR_MEDIA_HISTORY_STORE_MOJOM_LITE_JS" file="${root_gen_dir}\chrome\browser\media\history\media_history_store.mojom-lite.js" use_base_dir="false" type="BINDATA" compress="gzip" /> <include name="IDR_MEDIA_HISTORY_STORE_MOJOM_LITE_JS" file="${root_gen_dir}\chrome\browser\media\history\media_history_store.mojom-lite.js" use_base_dir="false" type="BINDATA" compress="gzip" />
<include name="IDR_MEMORY_INTERNALS_HTML" file="resources\memory_internals\memory_internals.html" flattenhtml="true" type="BINDATA" compress="gzip" /> <include name="IDR_MEMORY_INTERNALS_HTML" file="resources\memory_internals\memory_internals.html" type="BINDATA" compress="gzip" />
<include name="IDR_MEMORY_INTERNALS_JS" file="resources\memory_internals\memory_internals.js" type="BINDATA" compress="gzip" /> <include name="IDR_MEMORY_INTERNALS_JS" file="resources\memory_internals\memory_internals.js" type="BINDATA" compress="gzip" />
<include name="IDR_NOTIFICATIONS_INTERNALS_HTML" file="resources\notifications_internals\notifications_internals.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" /> <include name="IDR_NOTIFICATIONS_INTERNALS_HTML" file="resources\notifications_internals\notifications_internals.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" compress="gzip" />
<include name="IDR_NOTIFICATIONS_INTERNALS_CSS" file="resources\notifications_internals\notifications_internals.css" type="BINDATA" compress="gzip" /> <include name="IDR_NOTIFICATIONS_INTERNALS_CSS" file="resources\notifications_internals\notifications_internals.css" type="BINDATA" compress="gzip" />
......
chrome/browser/resources/BUILD.gn
View file @ f925f56d
...@@ -18,6 +18,7 @@ if (closure_compile) { ...@@ -18,6 +18,7 @@ if (closure_compile) {
"engagement:closure_compile", "engagement:closure_compile",
"interventions_internals:closure_compile", "interventions_internals:closure_compile",
"media:closure_compile", "media:closure_compile",
"memory_internals:closure_compile",
"reset_password:closure_compile", "reset_password:closure_compile",
] ]
if (is_linux || is_win || is_mac) { if (is_linux || is_win || is_mac) {
......
chrome/browser/resources/memory_internals/BUILD.gn 0 → 100644
View file @ f925f56d
# Copyright 2020 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.
import("//third_party/closure_compiler/compile_js.gni")
js_type_check("closure_compile") {
uses_js_modules = true
deps = [ ":memory_internals" ]
}
js_library("memory_internals") {
deps = [
"//ui/webui/resources/js:cr.m",
"//ui/webui/resources/js:util.m",
]
}
chrome/browser/resources/memory_internals/memory_internals.html
View file @ f925f56d
<!doctype html> <!doctype html>
<html><head> <html>
<title>Memory Internals</title> <head>
<style type="text/css"> <meta charset="utf-8">
<title>Memory Internals</title>
<style type="text/css">
body { body {
font-family: sans-serif; font-family: sans-serif;
} }
...@@ -13,7 +15,7 @@ ...@@ -13,7 +15,7 @@
td { td {
padding: 5px; padding: 5px;
} }
.message { #message {
font-style: italic; font-style: italic;
margin-bottom: 1em; margin-bottom: 1em;
padding: 5px; padding: 5px;
...@@ -21,18 +23,20 @@ ...@@ -21,18 +23,20 @@
.commands { .commands {
padding: 5px; padding: 5px;
} }
</style> </style>
</head><body> </head>
<body>
<h3>Experimental memory internals</h3>
<div id="message"></div>
<script src="chrome://resources/js/util.js"></script> <div class="commands">
<script src="chrome://resources/js/cr.js"></script> <button id="refresh">↺ Refresh process list</button>
<script src="chrome://memory-internals/memory_internals.js"></script> <button id="save">⇩ Save dump</button>
<div id="save-dump-text"></div>
</div>
<h3>Experimental memory internals</h3> <div id="proclist">loading...</div>
<script type="module" src="memory_internals.js"></script>
<div class="message" id="message"></div> </body>
</html>
<div id="proclist">loading...</div>
</body></html>
chrome/browser/resources/memory_internals/memory_internals.js
View file @ f925f56d
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
'use strict'; import {addWebUIListener, sendWithPromise} from 'chrome://resources/js/cr.m.js';
import {$} from 'chrome://resources/js/util.m.js';
function requestProcessList() { function requestProcessList() {
chrome.send('requestProcessList'); sendWithPromise('requestProcessList').then(onProcessListReceived);
} }
function saveDump() { function saveDump() {
...@@ -34,11 +35,7 @@ function addListRow(table, celltype, cols) { ...@@ -34,11 +35,7 @@ function addListRow(table, celltype, cols) {
table.appendChild(tr); table.appendChild(tr);
} }
function setSaveDumpMessage(data) { function onProcessListReceived(data) {
$('save_dump_text').innerText = data;
}
function returnProcessList(data) {
$('message').innerText = data['message']; $('message').innerText = data['message'];
const proclist = $('proclist'); const proclist = $('proclist');
...@@ -47,25 +44,7 @@ function returnProcessList(data) { ...@@ -47,25 +44,7 @@ function returnProcessList(data) {
const processes = data['processes']; const processes = data['processes'];
if (processes.length == 0) { if (processes.length == 0) {
return; return;
} // No processes to dump, don't make the table and refresh button. } // No processes to dump, don't make the table.
// Add the refresh and save-dump buttons.
const commandsDiv = document.createElement('div');
commandsDiv.className = 'commands';
const refreshButton = document.createElement('button');
refreshButton.innerText = '\u21ba Refresh process list';
refreshButton.onclick = () => requestProcessList();
commandsDiv.appendChild(refreshButton);
const saveDumpButton = document.createElement('button');
saveDumpButton.innerText = '\u21e9 Save dump';
saveDumpButton.onclick = () => saveDump();
commandsDiv.appendChild(saveDumpButton);
const saveDumpText = document.createElement('div');
saveDumpText.id = 'save_dump_text';
commandsDiv.appendChild(saveDumpText);
proclist.appendChild(commandsDiv);
const table = document.createElement('table'); const table = document.createElement('table');
...@@ -81,13 +60,11 @@ function returnProcessList(data) { ...@@ -81,13 +60,11 @@ function returnProcessList(data) {
const description = document.createTextNode(proc[1]); const description = document.createTextNode(proc[1]);
const profiled = proc[2]; const profiled = proc[2];
let button = null; const button = document.createElement('button');
if (profiled) { if (profiled) {
button = document.createElement('button');
button.innerText = '\uD83D\uDC1E Report'; button.innerText = '\uD83D\uDC1E Report';
button.onclick = () => reportProcess(procId); button.onclick = () => reportProcess(procId);
} else { } else {
button = document.createElement('button');
button.innerText = '\u2600 Start profiling'; button.innerText = '\u2600 Start profiling';
button.onclick = () => startProfiling(procId); button.onclick = () => startProfiling(procId);
} }
...@@ -99,11 +76,20 @@ function returnProcessList(data) { ...@@ -99,11 +76,20 @@ function returnProcessList(data) {
} }
// Get data and have it displayed upon loading. // Get data and have it displayed upon loading.
document.addEventListener('DOMContentLoaded', requestProcessList); document.addEventListener('DOMContentLoaded', () => {
$('refresh').onclick = requestProcessList;
$('save').onclick = saveDump;
addWebUIListener('save-dump-progress', progress => {
$('save-dump-text').innerText = progress;
});
requestProcessList();
});
/* For manual testing. /* For manual testing.
function fakeResults() { function fakeResults() {
returnProcessList([ onProcessListReceived([
[ 11234, "Process 11234 [Browser]" ], [ 11234, "Process 11234 [Browser]" ],
[ 11235, "Process 11235 [Renderer]" ], [ 11235, "Process 11235 [Renderer]" ],
[ 11236, "Process 11236 [Renderer]" ]]); [ 11236, "Process 11236 [Renderer]" ]]);
......
chrome/browser/ui/webui/memory_internals_ui.cc
View file @ f925f56d
...@@ -149,9 +149,12 @@ class MemoryInternalsDOMHandler : public content::WebUIMessageHandler, ...@@ -149,9 +149,12 @@ class MemoryInternalsDOMHandler : public content::WebUIMessageHandler,
// Hops to the IO thread to enumerate child processes, and back to the UI // Hops to the IO thread to enumerate child processes, and back to the UI
// thread to fill in the renderer processes. // thread to fill in the renderer processes.
static void GetChildProcessesOnIOThread( static void GetChildProcessesOnIOThread(
base::WeakPtr<MemoryInternalsDOMHandler> dom_handler); base::WeakPtr<MemoryInternalsDOMHandler> dom_handler,
void GetProfiledPids(std::vector<base::Value> children); const std::string& callback_id);
void ReturnProcessListOnUIThread(std::vector<base::Value> children, void GetProfiledPids(const std::string& callback_id,
std::vector<base::Value> children);
void ReturnProcessListOnUIThread(const std::string& callback_id,
std::vector<base::Value> children,
std::vector<base::ProcessId> profiled_pids); std::vector<base::ProcessId> profiled_pids);
// SelectFileDialog::Listener implementation: // SelectFileDialog::Listener implementation:
...@@ -201,22 +204,26 @@ void MemoryInternalsDOMHandler::RegisterMessages() { ...@@ -201,22 +204,26 @@ void MemoryInternalsDOMHandler::RegisterMessages() {
void MemoryInternalsDOMHandler::HandleRequestProcessList( void MemoryInternalsDOMHandler::HandleRequestProcessList(
const base::ListValue* args) { const base::ListValue* args) {
AllowJavascript();
std::string callback_id = args->GetList()[0].GetString();
// This is called on the UI thread, the child process iterator must run on // This is called on the UI thread, the child process iterator must run on
// the IO thread, while the render process iterator must run on the UI thread. // the IO thread, while the render process iterator must run on the UI thread.
base::PostTask( base::PostTask(
FROM_HERE, {content::BrowserThread::IO}, FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&MemoryInternalsDOMHandler::GetChildProcessesOnIOThread, base::BindOnce(&MemoryInternalsDOMHandler::GetChildProcessesOnIOThread,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr(), std::move(callback_id)));
} }
void MemoryInternalsDOMHandler::HandleSaveDump(const base::ListValue* args) { void MemoryInternalsDOMHandler::HandleSaveDump(const base::ListValue* args) {
base::FilePath default_file = base::FilePath().AppendASCII( base::FilePath default_file = base::FilePath().AppendASCII(
base::StringPrintf("trace_with_heap_dump.json.gz")); base::StringPrintf("trace_with_heap_dump.json.gz"));
AllowJavascript();
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
base::Value result("Saving..."); base::Value result("Saving...");
AllowJavascript(); FireWebUIListener("save-dump-progress", result);
CallJavascriptFunction("setSaveDumpMessage", result);
// On Android write to the user data dir. // On Android write to the user data dir.
// TODO(bug 757115) Does it make sense to show the Android file picker here // TODO(bug 757115) Does it make sense to show the Android file picker here
...@@ -273,7 +280,8 @@ void MemoryInternalsDOMHandler::HandleStartProfiling( ...@@ -273,7 +280,8 @@ void MemoryInternalsDOMHandler::HandleStartProfiling(
} }
void MemoryInternalsDOMHandler::GetChildProcessesOnIOThread( void MemoryInternalsDOMHandler::GetChildProcessesOnIOThread(
base::WeakPtr<MemoryInternalsDOMHandler> dom_handler) { base::WeakPtr<MemoryInternalsDOMHandler> dom_handler,
const std::string& callback_id) {
std::vector<base::Value> result; std::vector<base::Value> result;
// The only non-renderer child processes that currently support out-of-process // The only non-renderer child processes that currently support out-of-process
...@@ -291,10 +299,11 @@ void MemoryInternalsDOMHandler::GetChildProcessesOnIOThread( ...@@ -291,10 +299,11 @@ void MemoryInternalsDOMHandler::GetChildProcessesOnIOThread(
base::PostTask(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&MemoryInternalsDOMHandler::GetProfiledPids, base::BindOnce(&MemoryInternalsDOMHandler::GetProfiledPids,
dom_handler, std::move(result))); dom_handler, callback_id, std::move(result)));
} }
void MemoryInternalsDOMHandler::GetProfiledPids( void MemoryInternalsDOMHandler::GetProfiledPids(
const std::string& callback_id,
std::vector<base::Value> children) { std::vector<base::Value> children) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
heap_profiling::Supervisor* supervisor = heap_profiling::Supervisor* supervisor =
...@@ -305,17 +314,18 @@ void MemoryInternalsDOMHandler::GetProfiledPids( ...@@ -305,17 +314,18 @@ void MemoryInternalsDOMHandler::GetProfiledPids(
base::PostTask( base::PostTask(
FROM_HERE, {content::BrowserThread::UI}, FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&MemoryInternalsDOMHandler::ReturnProcessListOnUIThread, base::BindOnce(&MemoryInternalsDOMHandler::ReturnProcessListOnUIThread,
weak_factory_.GetWeakPtr(), std::move(children), weak_factory_.GetWeakPtr(), callback_id,
std::vector<base::ProcessId>())); std::move(children), std::vector<base::ProcessId>()));
return; return;
} }
supervisor->GetProfiledPids( supervisor->GetProfiledPids(base::BindOnce(
base::BindOnce(&MemoryInternalsDOMHandler::ReturnProcessListOnUIThread, &MemoryInternalsDOMHandler::ReturnProcessListOnUIThread,
weak_factory_.GetWeakPtr(), std::move(children))); weak_factory_.GetWeakPtr(), callback_id, std::move(children)));
} }
void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread( void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread(
const std::string& callback_id,
std::vector<base::Value> children, std::vector<base::Value> children,
std::vector<base::ProcessId> profiled_pids) { std::vector<base::ProcessId> profiled_pids) {
// This function will be called with the child processes that are not // This function will be called with the child processes that are not
...@@ -364,16 +374,14 @@ void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread( ...@@ -364,16 +374,14 @@ void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread(
result.SetKey("message", base::Value(GetMessageString())); result.SetKey("message", base::Value(GetMessageString()));
result.SetKey("processes", base::Value(std::move(process_list))); result.SetKey("processes", base::Value(std::move(process_list)));
AllowJavascript(); ResolveJavascriptCallback(base::Value(callback_id), result);
CallJavascriptFunction("returnProcessList", result);
} }
void MemoryInternalsDOMHandler::FileSelected(const base::FilePath& path, void MemoryInternalsDOMHandler::FileSelected(const base::FilePath& path,
int index, int index,
void* params) { void* params) {
base::Value result("Saving..."); base::Value result("Saving...");
AllowJavascript(); FireWebUIListener("save-dump-progress", result);
CallJavascriptFunction("setSaveDumpMessage", result);
ProfilingProcessHost::GetInstance()->SaveTraceWithHeapDumpToFile( ProfilingProcessHost::GetInstance()->SaveTraceWithHeapDumpToFile(
path, path,
...@@ -389,8 +397,7 @@ void MemoryInternalsDOMHandler::FileSelectionCanceled(void* params) { ...@@ -389,8 +397,7 @@ void MemoryInternalsDOMHandler::FileSelectionCanceled(void* params) {
void MemoryInternalsDOMHandler::SaveTraceFinished(bool success) { void MemoryInternalsDOMHandler::SaveTraceFinished(bool success) {
base::Value result(success ? "Save successful." : "Save failure."); base::Value result(success ? "Save successful." : "Save failure.");
AllowJavascript(); FireWebUIListener("save-dump-progress", result);
CallJavascriptFunction("setSaveDumpMessage", result);
} }
} // namespace } // namespace
......
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