Skip to content

Commit

Permalink
Show PaintWorkletGlobalScope in dev-tool's console list
Browse files Browse the repository at this point in the history
Currently, if we open a webpage that calls registerPaint API and open the
dev-tool, we would see "top" under the console list. The reason is that
when the MainThreadDebugger::ContextCreated is called, the
|human_readable_name| is set to an empty string.

This CL changes it such that when the InitializeContextIfNeeded is called
from PaintWorkletGlobalScope::Create, we pass a string to set the
|human_readable_name| when it is not the main world, and that string
will show up in the dev-tool's console list.

Bug: 728591
Change-Id: I588d40c03d670331fdabee456411c34f75a7fb02
Reviewed-on: https://chromium-review.googlesource.com/543616
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483286}
  • Loading branch information
xidachen authored and Commit Bot committed Jun 29, 2017
1 parent 958bfd4 commit 5902839
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Tests console execution context selector for paintworklet.

Console context selector:
* top
____Paint Worklet

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<html>
<head>
<script src="../../http/tests/inspector/inspector-test.js"></script>
<script src="../../http/tests/inspector/console-test.js"></script>
<script src="../../http/tests/inspector/debugger-test.js"></script>

<script id="code" type="text/worklet">
registerPaint('foo', class { paint() { } });
</script>
<script>
function setup() {
var blob = new Blob([code.textContent], {type: 'text/javascript'});
return paintWorklet.addModule(URL.createObjectURL(blob));
}

async function test()
{
await new Promise(f => InspectorTest.startDebuggerTest(f, true));
await InspectorTest.evaluateInPageAsync('setup()');

var consoleView = Console.ConsoleView.instance();
var selector = consoleView._consoleContextSelector;
InspectorTest.addResult('Console context selector:');
for (var executionContext of selector._items) {
var selected = UI.context.flavor(SDK.ExecutionContext) === executionContext;
var text = '____'.repeat(selector._depthFor(executionContext)) + selector.titleFor(executionContext);
var disabled = !selector.isItemSelectable(executionContext);
InspectorTest.addResult(`${selected ? '*' : ' '} ${text} ${disabled ? '[disabled]' : ''}`);
}

InspectorTest.completeDebuggerTest();
}
</script>
</head>
<body onload="runTest()">
<p> Tests console execution context selector for paintworklet.
</p>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ PassRefPtr<DOMWrapperWorld> ScriptController::CreateNewInspectorIsolatedWorld(
if (!world)
return nullptr;
if (!world_name.IsEmpty()) {
DOMWrapperWorld::SetIsolatedWorldHumanReadableName(world->GetWorldId(),
world_name);
DOMWrapperWorld::SetNonMainWorldHumanReadableName(world->GetWorldId(),
world_name);
}
// Make sure the execution context exists.
WindowProxy(*world);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ void WorkerOrWorkletScriptController::DisposeContextIfNeeded() {
script_state_->DisposePerContextData();
}

bool WorkerOrWorkletScriptController::InitializeContextIfNeeded() {
bool WorkerOrWorkletScriptController::InitializeContextIfNeeded(
const String& human_readable_name) {
v8::HandleScope handle_scope(isolate_);

if (IsContextInitialized())
Expand Down Expand Up @@ -229,6 +230,13 @@ bool WorkerOrWorkletScriptController::InitializeContextIfNeeded() {
debugger->ContextCreated(global_scope_->GetThread(), context);
}

// Set the human readable name for the world if the call passes an actual
// |human_readable name|.
if (!human_readable_name.IsEmpty()) {
world_->SetNonMainWorldHumanReadableName(world_->GetWorldId(),
human_readable_name);
}

return true;
}

Expand All @@ -241,7 +249,7 @@ ScriptValue WorkerOrWorkletScriptController::Evaluate(
TRACE_EVENT1("devtools.timeline", "EvaluateScript", "data",
InspectorEvaluateScriptEvent::Data(nullptr, file_name,
script_start_position));
if (!InitializeContextIfNeeded())
if (!InitializeContextIfNeeded(String()))
return ScriptValue();

ScriptState::Scope scope(script_state_.Get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CORE_EXPORT WorkerOrWorkletScriptController

// Used by WorkerThread. Returns true if the context is successfully
// initialized or already initialized.
bool InitializeContextIfNeeded();
bool InitializeContextIfNeeded(const String& human_readable_name);

// Used by WorkerGlobalScope:
void RethrowExceptionFromImportedScript(ErrorEvent*, ExceptionState&);
Expand Down
5 changes: 3 additions & 2 deletions third_party/WebKit/Source/core/frame/HostsUsingFeatures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ void HostsUsingFeatures::CountHostOrIsolatedWorldHumanReadableName(
document->HostsUsingFeaturesValue().Count(feature);
return;
}
if (Page* page = document->GetPage())
if (Page* page = document->GetPage()) {
page->GetHostsUsingFeatures().CountName(
feature, script_state->World().IsolatedWorldHumanReadableName());
feature, script_state->World().NonMainWorldHumanReadableName());
}
}

void HostsUsingFeatures::Value::Count(Feature feature) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,8 @@ void MainThreadDebugger::ContextCreated(ScriptState* script_state,
aux_data_builder.Append(IdentifiersFactory::FrameId(frame));
aux_data_builder.Append("\"}");
String aux_data = aux_data_builder.ToString();
String human_readable_name = world.IsIsolatedWorld()
? world.IsolatedWorldHumanReadableName()
: String();
String human_readable_name =
!world.IsMainWorld() ? world.NonMainWorldHumanReadableName() : String();
String origin_string = origin ? origin->ToRawString() : String();
v8_inspector::V8ContextInfo context_info(
script_state->GetContext(), ContextGroupId(frame),
Expand Down
3 changes: 2 additions & 1 deletion third_party/WebKit/Source/core/workers/WorkerThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,8 @@ void WorkerThread::InitializeOnWorkerThread(

// TODO(nhiroki): Handle a case where the script controller fails to
// initialize the context.
if (GlobalScope()->ScriptController()->InitializeContextIfNeeded()) {
if (GlobalScope()->ScriptController()->InitializeContextIfNeeded(
String())) {
worker_reporting_proxy_.DidInitializeWorkerContext();
v8::HandleScope handle_scope(GetIsolate());
Platform::Current()->WorkerContextCreated(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ PaintWorkletGlobalScope* PaintWorkletGlobalScope::Create(
new PaintWorkletGlobalScope(frame, url, user_agent,
std::move(security_origin), isolate,
pending_generator_registry);
paint_worklet_global_scope->ScriptController()->InitializeContextIfNeeded();
// TODO(xidachen): When we implement two PaintWorkletGlobalScope, we should
// change the last parameter.
paint_worklet_global_scope->ScriptController()->InitializeContextIfNeeded(
"Paint Worklet");
MainThreadDebugger::Instance()->ContextCreated(
paint_worklet_global_scope->ScriptController()->GetScriptState(),
paint_worklet_global_scope->GetFrame(),
Expand Down
12 changes: 8 additions & 4 deletions third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ static bool IsIsolatedWorldId(int world_id) {
return DOMWrapperWorld::kMainWorldId < world_id &&
world_id < DOMWrapperWorld::kIsolatedWorldIdLimit;
}

static bool IsMainWorldId(int world_id) {
return world_id == DOMWrapperWorld::kMainWorldId;
}
#endif

PassRefPtr<DOMWrapperWorld> DOMWrapperWorld::Create(v8::Isolate* isolate,
Expand Down Expand Up @@ -196,16 +200,16 @@ static IsolatedWorldHumanReadableNameMap& IsolatedWorldHumanReadableNames() {
return map;
}

String DOMWrapperWorld::IsolatedWorldHumanReadableName() {
DCHECK(this->IsIsolatedWorld());
String DOMWrapperWorld::NonMainWorldHumanReadableName() {
DCHECK(!this->IsMainWorld());
return IsolatedWorldHumanReadableNames().at(GetWorldId());
}

void DOMWrapperWorld::SetIsolatedWorldHumanReadableName(
void DOMWrapperWorld::SetNonMainWorldHumanReadableName(
int world_id,
const String& human_readable_name) {
#if DCHECK_IS_ON()
DCHECK(IsIsolatedWorldId(world_id));
DCHECK(!IsMainWorldId(world_id));
#endif
IsolatedWorldHumanReadableNames().Set(world_id, human_readable_name);
}
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ class PLATFORM_EXPORT DOMWrapperWorld : public RefCounted<DOMWrapperWorld> {

static DOMWrapperWorld& MainWorld();

static void SetIsolatedWorldHumanReadableName(int world_id, const String&);
String IsolatedWorldHumanReadableName();
static void SetNonMainWorldHumanReadableName(int world_id, const String&);
String NonMainWorldHumanReadableName();

// Associates an isolated world (see above for description) with a security
// origin. XMLHttpRequest instances used in that world will be considered
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,8 @@ void WebLocalFrameImpl::SetIsolatedWorldHumanReadableName(
int world_id,
const WebString& human_readable_name) {
DCHECK(GetFrame());
DOMWrapperWorld::SetIsolatedWorldHumanReadableName(world_id,
human_readable_name);
DOMWrapperWorld::SetNonMainWorldHumanReadableName(world_id,
human_readable_name);
}

void WebLocalFrameImpl::AddMessageToConsole(const WebConsoleMessage& message) {
Expand Down

0 comments on commit 5902839

Please sign in to comment.