Propagate nested importScripts() error events outwards.

Source/bindings/core/v8/WorkerScriptController.cpp

 /*
  * Copyright (C) 2009, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 185 matching lines @@
 
     return ScriptValue(m_scriptState.get(), result);
 }
 
 void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, RefPtrWillBeRawPtr<ErrorEvent>* errorEvent)
 {
     if (isExecutionForbidden())
         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);
         }
     }
 }
 
 void WorkerScriptController::scheduleExecutionTermination()
 {
     // The mutex provides a memory barrier to ensure that once
     // termination is scheduled, isExecutionTerminating will
     // accurately reflect that state when called from another thread.
@@ skipping 21 matching lines @@
 {
     ASSERT(m_workerGlobalScope.isContextThread());
     return m_executionForbidden;
 }
 
 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