Propagate nested importScripts() error events outwards.

Source/bindings/core/v8/WorkerScriptController.cpp

Index: Source/bindings/core/v8/WorkerScriptController.cpp
diff --git a/Source/bindings/core/v8/WorkerScriptController.cpp b/Source/bindings/core/v8/WorkerScriptController.cpp
index d7e3a93b43fd4276e6d5dec0044caf869bb75833..b73acc39213c3c526c4cc8d0d6ac7fb87157bf29 100644
--- a/Source/bindings/core/v8/WorkerScriptController.cpp
+++ b/Source/bindings/core/v8/WorkerScriptController.cpp
@@ -203,20 +203,27 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, RefPtr
         return;
 
     WorkerGlobalScopeExecutionState state;
+    m_errorEventFromImportedScript.clear();

[haraken, 2014/07/17 15:19:05]

Is it possible that m_errorEventFromImportedScript is not cleared at this point?
Maybe can we add ASSERT(!m_errorEventFromImportedScript)?

[sof, 2014/07/17 15:22:17]

It won't be cleared whenever a previous (not nested) importScripts() calls
rethrowExceptionFromImportedScript().

[haraken, 2014/07/17 15:47:24]

Then does it mean that a persistent handle to the error event is kept alive
until the next script is evaluated by the WorkerScriptController? If yes, that
wouldn't be desirable.

[sof, 2014/07/17 15:53:19]

Yes, or via a RefPtr non-Oilpan if that exception ends up being caught.

[haraken, 2014/07/17 15:58:54]

I'm a bit afraid that it will cause leaks. Why can't we clear
m_errorEventFromImportedScript at the end of WorkerScriptController::evaluate()?
I wonder why we need to keep it alive after WorkerScriptController::evaluate()
finishes.

(Probably I'm not understanding how things are working.)

[sof, 2014/07/17 18:13:11]

I'll have another look if we can tidy this up here, but this CL actually
improves the situation ErrorEvent leak-wise.

It's brought about by the need to sanitize uncaught errors when reporting them
out of the global scope and onto the worker object.

[sof, 2014/07/17 22:05:31]

Reworked a bit to let these ErrorEvents be stack allocated instead.

The ExecutionState stack objects are now chained to handle nested invocation --
please take a look. 

     evaluate(sourceCode.source(), sourceCode.url().string(), sourceCode.startPosition(), &state);
     if (state.hadException) {
         if (errorEvent) {
-            *errorEvent = m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin) ?
-                ErrorEvent::createSanitizedError(m_world.get()) : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get());
+            if (m_errorEventFromImportedScript) {
+                // Propagate inner error event outwards.
+                *errorEvent = 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);
         } else {
             ASSERT(!m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin));
             RefPtrWillBeRawPtr<ErrorEvent> event = nullptr;
-            if (m_errorEventFromImportedScript) {
+            if (m_errorEventFromImportedScript)
                 event = m_errorEventFromImportedScript.release();
-            } else {
+            else
                 event = ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, m_world.get());
-            }
             m_workerGlobalScope.reportException(event, nullptr, NotSharableCrossOrigin);
         }
     }
@@ -258,10 +265,10 @@ void WorkerScriptController::disableEval(const String& errorMessage)
     m_disableEvalPending = errorMessage;
 }
 
-void WorkerScriptController::rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent> errorEvent)
+void WorkerScriptController::rethrowExceptionFromImportedScript(PassRefPtrWillBeRawPtr<ErrorEvent> errorEvent, ExceptionState& exceptionState)
 {
     m_errorEventFromImportedScript = errorEvent;
-    throwError(V8ThrowException::createError(v8GeneralError, m_errorEventFromImportedScript->message(), m_isolate), m_isolate);
+    exceptionState.rethrowV8Exception(V8ThrowException::createError(v8GeneralError, m_errorEventFromImportedScript->message(), m_isolate));
 }
 
 } // namespace WebCore