Skip to content

GitLab

T
tangled
Commit 9f991843 authored by sigbjornf@opera.com's avatar sigbjornf@opera.com
Browse files
Options

Propagate nested importScripts() error events outwards. 

If importScripts() evaluation fails, we report the exception wrt the
URL and location of the failing script. Extend that to handle the
nested case, propagating any inner importScripts-induced error event
outwards rather than construct a new one wrt the outer script URL.

R=haraken@chromium.org
BUG=394615

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

git-svn-id: svn://svn.chromium.org/blink/trunk@178432 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent d718e5c7
Hide whitespace changes
Inline Side-by-side
Showing with 130 additions and 42 deletions
third_party/WebKit/LayoutTests/fast/workers/worker-nested-importScripts-error-expected.txt 0 → 100644
View file @ 9f991843
This tests that errors from nested importScripts have the expected provenance.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Starting worker: resources/importScripts-1.js
PASS event.type is "error"
PASS event.filename.indexOf('invalidScript.js') >= 0 is true
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/workers/worker-nested-importScripts-error.html 0 → 100644
View file @ 9f991843
<!DOCTYPE html>
<html>
<head>
<script src="../../resources/js-test.js"></script>
</head>
<body>
<script>
description("This tests that errors from nested importScripts have the expected provenance.");
self.jsTestIsAsync = true;
var worker = startWorker("resources/importScripts-1.js");
var event;
worker.onerror = function (e) {
event = e;
shouldBeEqualToString("event.type", "error");
shouldBeTrue("event.filename.indexOf('invalidScript.js') >= 0");
finishJSTest();
};
</script>
</body>
</html>
third_party/WebKit/Source/bindings/core/v8/WorkerScriptController.cpp
View file @ 9f991843
...@@ -58,11 +58,58 @@ ...@@ -58,11 +58,58 @@
namespace WebCore { namespace WebCore {
class WorkerScriptController::WorkerGlobalScopeExecutionState FINAL {
STACK_ALLOCATED();
public:
explicit WorkerGlobalScopeExecutionState(WorkerScriptController* controller)
: hadException(false)
, lineNumber(0)
, columnNumber(0)
, m_controller(controller)
, m_outerState(controller->m_globalScopeExecutionState)
{
m_controller->m_globalScopeExecutionState = this;
}
~WorkerGlobalScopeExecutionState()
{
m_controller->m_globalScopeExecutionState = m_outerState;
}
void trace(Visitor* visitor)
{
visitor->trace(m_errorEventFromImportedScript);
}
bool hadException;
String errorMessage;
int lineNumber;
int columnNumber;
String sourceURL;
ScriptValue exception;
RefPtrWillBeMember<ErrorEvent> m_errorEventFromImportedScript;
// A WorkerGlobalScopeExecutionState context is stack allocated by
// WorkerScriptController::evaluate(), with the contoller using it
// during script evaluation. To handle nested evaluate() uses,
// WorkerGlobalScopeExecutionStates are chained together;
// |m_outerState| keeps a pointer to the context object one level out
// (or 0, if outermost.) Upon return from evaluate(), the
// WorkerScriptController's WorkerGlobalScopeExecutionState is popped
// and the previous one restored (see above dtor.)
//
// With Oilpan, |m_outerState| isn't traced. It'll be "up the stack"
// and its fields will be traced when scanning the stack.
WorkerScriptController* m_controller;
WorkerGlobalScopeExecutionState* m_outerState;
};
WorkerScriptController::WorkerScriptController(WorkerGlobalScope& workerGlobalScope) WorkerScriptController::WorkerScriptController(WorkerGlobalScope& workerGlobalScope)
: m_isolate(v8::Isolate::New()) : m_isolate(v8::Isolate::New())
, m_workerGlobalScope(workerGlobalScope) , m_workerGlobalScope(workerGlobalScope)
, m_executionForbidden(false) , m_executionForbidden(false)
, m_executionScheduledToTerminate(false) , m_executionScheduledToTerminate(false)
, m_globalScopeExecutionState(0)
{ {
m_isolate->Enter(); m_isolate->Enter();
V8Initializer::initializeWorker(m_isolate); V8Initializer::initializeWorker(m_isolate);
...@@ -153,7 +200,7 @@ bool WorkerScriptController::initializeContextIfNeeded() ...@@ -153,7 +200,7 @@ bool WorkerScriptController::initializeContextIfNeeded()
return true; return true;
} }
ScriptValue WorkerScriptController::evaluate(const String& script, const String& fileName, const TextPosition& scriptStartPosition, WorkerGlobalScopeExecutionState* state) ScriptValue WorkerScriptController::evaluate(const String& script, const String& fileName, const TextPosition& scriptStartPosition)
{ {
if (!initializeContextIfNeeded()) if (!initializeContextIfNeeded())
return ScriptValue(); return ScriptValue();
...@@ -179,16 +226,16 @@ ScriptValue WorkerScriptController::evaluate(const String& script, const String& ...@@ -179,16 +226,16 @@ ScriptValue WorkerScriptController::evaluate(const String& script, const String&
if (block.HasCaught()) { if (block.HasCaught()) {
v8::Local<v8::Message> message = block.Message(); v8::Local<v8::Message> message = block.Message();
state->hadException = true; m_globalScopeExecutionState->hadException = true;
state->errorMessage = toCoreString(message->Get()); m_globalScopeExecutionState->errorMessage = toCoreString(message->Get());
state->lineNumber = message->GetLineNumber(); m_globalScopeExecutionState->lineNumber = message->GetLineNumber();
state->columnNumber = message->GetStartColumn() + 1; m_globalScopeExecutionState->columnNumber = message->GetStartColumn() + 1;
TOSTRING_DEFAULT(V8StringResource<>, sourceURL, message->GetScriptOrigin().ResourceName(), ScriptValue()); TOSTRING_DEFAULT(V8StringResource<>, sourceURL, message->GetScriptOrigin().ResourceName(), ScriptValue());
state->sourceURL = sourceURL; m_globalScopeExecutionState->sourceURL = sourceURL;
state->exception = ScriptValue(m_scriptState.get(), block.Exception()); m_globalScopeExecutionState->exception = ScriptValue(m_scriptState.get(), block.Exception());
block.Reset(); block.Reset();
} else { } else {
state->hadException = false; m_globalScopeExecutionState->hadException = false;
} }
if (result.IsEmpty() || result->IsUndefined()) if (result.IsEmpty() || result->IsUndefined())
...@@ -202,21 +249,27 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, RefPtr ...@@ -202,21 +249,27 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, RefPtr
if (isExecutionForbidden()) if (isExecutionForbidden())
return; return;
WorkerGlobalScopeExecutionState state; WorkerGlobalScopeExecutionState state(this);
evaluate(sourceCode.source(), sourceCode.url().string(), sourceCode.startPosition(), &state); evaluate(sourceCode.source(), sourceCode.url().string(), sourceCode.startPosition());
if (state.hadException) { if (state.hadException) {
if (errorEvent) { if (errorEvent) {
*errorEvent = m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin) ? if (state.m_errorEventFromImportedScript) {
ErrorEvent::createSanitizedError(m_world.get()) : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get()); // Propagate inner error event outwards.
*errorEvent = state.m_errorEventFromImportedScript.release();
return;
}
if (m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin))
*errorEvent = ErrorEvent::createSanitizedError(m_world.get());
else
*errorEvent = ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get());
V8ErrorHandler::storeExceptionOnErrorEventWrapper(errorEvent->get(), state.exception.v8Value(), m_scriptState->context()->Global(), m_isolate); V8ErrorHandler::storeExceptionOnErrorEventWrapper(errorEvent->get(), state.exception.v8Value(), m_scriptState->context()->Global(), m_isolate);
} else { } else {
ASSERT(!m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin)); ASSERT(!m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin));
RefPtrWillBeRawPtr<ErrorEvent> event = nullptr; RefPtrWillBeRawPtr<ErrorEvent> event = nullptr;
if (m_errorEventFromImportedScript) { if (state.m_errorEventFromImportedScript)
event = m_errorEventFromImportedScript.release(); event = state.m_errorEventFromImportedScript.release();
} else { else
event = ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get()); event = ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get());
}
m_workerGlobalScope.reportException(event, nullptr, NotSharableCrossOrigin); m_workerGlobalScope.reportException(event, nullptr, NotSharableCrossOrigin);
} }
} }
...@@ -258,10 +311,12 @@ void WorkerScriptController::disableEval(const String& errorMessage) ...@@ -258,10 +311,12 @@ void WorkerScriptController::disableEval(const String& errorMessage)
m_disableEvalPending = errorMessage; m_disableEvalPending = errorMessage;
} }
void WorkerScriptController::rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent> errorEvent) void WorkerScriptController::rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent> errorEvent, ExceptionState& exceptionState)
{ {
m_errorEventFromImportedScript = errorEvent; const String& errorMessage = errorEvent->message();
throwError(V8ThrowException::createError(v8GeneralError, m_errorEventFromImportedScript->message(), m_isolate), m_isolate); if (m_globalScopeExecutionState)
m_globalScopeExecutionState->m_errorEventFromImportedScript = errorEvent;
exceptionState.rethrowV8Exception(V8ThrowException::createError(v8GeneralError, errorMessage, m_isolate));
} }
} // namespace WebCore } // namespace WebCore
third_party/WebKit/Source/bindings/core/v8/WorkerScriptController.h
View file @ 9f991843
...@@ -41,25 +41,11 @@ ...@@ -41,25 +41,11 @@
namespace WebCore { namespace WebCore {
class ErrorEvent; class ErrorEvent;
class ExceptionState;
class ScriptSourceCode; class ScriptSourceCode;
class ScriptValue; class ScriptValue;
class WorkerGlobalScope; class WorkerGlobalScope;
class WorkerGlobalScopeExecutionState;
struct WorkerGlobalScopeExecutionState {
WorkerGlobalScopeExecutionState()
: hadException(false)
, lineNumber(0)
, columnNumber(0)
{
}
bool hadException;
String errorMessage;
int lineNumber;
int columnNumber;
String sourceURL;
ScriptValue exception;
};
class WorkerScriptController { class WorkerScriptController {
public: public:
...@@ -72,7 +58,7 @@ public: ...@@ -72,7 +58,7 @@ public:
void evaluate(const ScriptSourceCode&, RefPtrWillBeRawPtr<ErrorEvent>* = 0); void evaluate(const ScriptSourceCode&, RefPtrWillBeRawPtr<ErrorEvent>* = 0);
void rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent>); void rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent>, ExceptionState&);
// Async request to terminate a future JS execution. Eventually causes termination // Async request to terminate a future JS execution. Eventually causes termination
// exception raised during JS execution, if the worker thread happens to run JS. // exception raised during JS execution, if the worker thread happens to run JS.
...@@ -89,9 +75,6 @@ public: ...@@ -89,9 +75,6 @@ public:
void disableEval(const String&); void disableEval(const String&);
// Evaluate a script file in the current execution environment.
ScriptValue evaluate(const String& script, const String& fileName, const TextPosition& scriptStartPosition, WorkerGlobalScopeExecutionState*);
v8::Isolate* isolate() const { return m_isolate; } v8::Isolate* isolate() const { return m_isolate; }
DOMWrapperWorld& world() const { return *m_world; } DOMWrapperWorld& world() const { return *m_world; }
ScriptState* scriptState() { return m_scriptState.get(); } ScriptState* scriptState() { return m_scriptState.get(); }
...@@ -103,8 +86,12 @@ public: ...@@ -103,8 +86,12 @@ public:
// until real work has been done. // until real work has been done.
bool idleNotification() { return v8::V8::IdleNotification(); } bool idleNotification() { return v8::V8::IdleNotification(); }
private: private:
class WorkerGlobalScopeExecutionState;
// Evaluate a script file in the current execution environment.
ScriptValue evaluate(const String& script, const String& fileName, const TextPosition& scriptStartPosition);
v8::Isolate* m_isolate; v8::Isolate* m_isolate;
WorkerGlobalScope& m_workerGlobalScope; WorkerGlobalScope& m_workerGlobalScope;
RefPtr<ScriptState> m_scriptState; RefPtr<ScriptState> m_scriptState;
...@@ -113,7 +100,15 @@ private: ...@@ -113,7 +100,15 @@ private:
bool m_executionForbidden; bool m_executionForbidden;
bool m_executionScheduledToTerminate; bool m_executionScheduledToTerminate;
mutable Mutex m_scheduledTerminationMutex; mutable Mutex m_scheduledTerminationMutex;
RefPtrWillBePersistent<ErrorEvent> m_errorEventFromImportedScript;
// |m_globalScopeExecutionState| refers to a stack object
// that evaluate() allocates; evaluate() ensuring that the
// pointer reference to it is removed upon returning. Hence
// kept as a bare pointer here, and not a Persistent with
// Oilpan enabled; stack scanning will visit the object and
// trace its on-heap fields.
GC_PLUGIN_IGNORE("394615")
WorkerGlobalScopeExecutionState* m_globalScopeExecutionState;
OwnPtr<V8IsolateInterruptor> m_interruptor; OwnPtr<V8IsolateInterruptor> m_interruptor;
}; };
......
third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp
View file @ 9f991843
...@@ -264,7 +264,7 @@ void WorkerGlobalScope::importScripts(const Vector<String>& urls, ExceptionState ...@@ -264,7 +264,7 @@ void WorkerGlobalScope::importScripts(const Vector<String>& urls, ExceptionState
RefPtrWillBeRawPtr<ErrorEvent> errorEvent = nullptr; RefPtrWillBeRawPtr<ErrorEvent> errorEvent = nullptr;
m_script->evaluate(ScriptSourceCode(scriptLoader->script(), scriptLoader->responseURL()), &errorEvent); m_script->evaluate(ScriptSourceCode(scriptLoader->script(), scriptLoader->responseURL()), &errorEvent);
if (errorEvent) { if (errorEvent) {
m_script->rethrowExceptionFromImportedScript(errorEvent.release()); m_script->rethrowExceptionFromImportedScript(errorEvent.release(), exceptionState);
return; return;
} }
} }
......
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