Apply script preprocessor to Web page scripts only.

Source/bindings/v8/ScriptController.cpp

Index: Source/bindings/v8/ScriptController.cpp
diff --git a/Source/bindings/v8/ScriptController.cpp b/Source/bindings/v8/ScriptController.cpp
index d0085c4491eaa9b9bec14c81ea736845f5adc1c2..8359cfcb69ede41aacb350648a9784086090c452 100644
--- a/Source/bindings/v8/ScriptController.cpp
+++ b/Source/bindings/v8/ScriptController.cpp
@@ -66,6 +66,7 @@
 #include "core/page/DOMWindow.h"
 #include "core/page/Frame.h"
 #include "core/page/Page.h"
+#include "core/page/PageConsole.h"
 #include "core/page/Settings.h"
 #include "core/platform/HistogramSupport.h"
 #include "core/platform/NotImplemented.h"
@@ -76,6 +77,7 @@
 #include "wtf/CurrentTime.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/StringExtras.h"
+#include "wtf/TemporaryChange.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/TextPosition.h"
@@ -668,7 +670,7 @@ bool ScriptController::executeScriptIfJavaScriptURL(const KURL& url)
 
     if (!locationChangeBefore && m_frame->navigationScheduler()->locationChangePending())
         return true;
-        
+
     // DocumentWriter::replaceDocument can cause the DocumentLoader to get deref'ed and possible destroyed,
     // so protect it with a RefPtr.
     if (RefPtr<DocumentLoader> loader = m_frame->document()->loader())
@@ -687,9 +689,12 @@ ScriptValue ScriptController::executeScriptInMainWorld(const ScriptSourceCode& s
     if (v8Context.IsEmpty())
         return ScriptValue();
 
+    String processedString = m_frame->script()->preprocess(sourceCode.source(), sourceURL);
+    ScriptSourceCode processedSourceCode(processedString, sourceCode.url(), sourceCode.startPosition());
+
     v8::Context::Scope scope(v8Context);
     RefPtr<Frame> protect(m_frame);
-    v8::Local<v8::Value> object = compileAndRunScript(sourceCode);
+    v8::Local<v8::Value> object = compileAndRunScript(processedSourceCode);
 
     m_sourceURL = savedSourceURL;
 
@@ -733,4 +738,119 @@ void ScriptController::executeScriptInIsolatedWorld(int worldID, const Vector<Sc
     }
 }
 
+class ScriptController::ScriptPreprocessor {

[abarth-chromium, 2013/07/09 23:14:10]

Please put this class in a separate file.  We prefer to have one class per file.
 There isn't really any reason for this class to be an inner class of
ScriptController.

+    WTF_MAKE_NONCOPYABLE(ScriptPreprocessor);
+public:
+    ScriptPreprocessor(const String& preprocessorScript, ScriptController* controller, PageConsole* console)
+        : m_controller(controller)
+        , m_isPreprocessing(false)
+    {
+        v8::TryCatch tryCatch;
+        tryCatch.SetVerbose(true);
+        ScriptSourceCode preprocessor(preprocessorScript);
+        Vector<ScriptSourceCode> sources;
+        sources.append(preprocessor);
+        Vector<ScriptValue> scriptResults;
+        controller->executeScriptInIsolatedWorld(ScriptPreprocessorIsolatedWorldId, sources, 0, &scriptResults);
+
+        ASSERT(scriptResults.size() == 1);

[abarth-chromium, 2013/07/09 23:14:10]

Please handle this error instead of just ASSERTing the result.

[johnjbarton, 2013/07/10 23:24:23]

Done.

+        ScriptValue preprocessorFunction = scriptResults[0];
+        if (!preprocessorFunction.isFunction()) {
+            console->addMessage(JSMessageSource, ErrorMessageLevel, "The preprocessor must compile to a function.");
+            return;

[abarth-chromium, 2013/07/09 23:14:10]

You're already set up to handle an error here.  Why not handle the case where
scriptResult.size() is an unexpected value?

[johnjbarton, 2013/07/10 23:24:23]

Done.

+        }
+
+        v8::Local<v8::Context> context = isolatedWorldContext();
+        v8::Isolate* isolate = context->GetIsolate();
+        m_preprocessorFunction.set(isolate, v8::Handle<v8::Function>::Cast(preprocessorFunction.v8Value()));
+    }
+
+    v8::Local<v8::Context> isolatedWorldContext()
+    {
+        RefPtr<DOMWrapperWorld> world = DOMWrapperWorld::ensureIsolatedWorld(ScriptPreprocessorIsolatedWorldId, 0);

[abarth-chromium, 2013/07/09 23:14:10]

What is the 0 in the second position here?

+        V8WindowShell* isolatedWorldShell = m_controller->windowShell(world.get());

[johnjbarton, 2013/07/10 23:24:23]

This groups the isolatedWorld with other worlds that will use the same V8
extensions. I replaced the 0 with the const value
DOMWrapperWorld::mainWorldExtensionGroup since we do not want any V8 extensions
not also used by the page. I also changed the similar 0 value in the
executeScriptInIsolatedWorld() call.

+        return isolatedWorldShell->context();
+    }
+
+    String preprocessSourceCode(const String& sourceCode, const String& sourceName)
+    {
+        if (m_preprocessorFunction.isEmpty())
+            return sourceCode;
+
+        v8::Local<v8::Context> context = isolatedWorldContext();
+        v8::Isolate* isolate(context->GetIsolate());
+        v8::HandleScope handleScope(isolate);
+        v8::Context::Scope contextScope(context);
+
+        v8::Handle<v8::String> sourceCodeString = v8String(sourceCode, isolate);
+        v8::Handle<v8::String> sourceNameString = v8String(sourceName, isolate);
+        v8::Handle<v8::Value> argv[] = { sourceCodeString, sourceNameString };
+
+        v8::TryCatch tryCatch;
+        tryCatch.SetVerbose(true);
+        TemporaryChange<bool> isPreprocessing(m_isPreprocessing, true);
+        v8::Handle<v8::Value> resultValue =
+            V8ScriptRunner::callAsFunction(m_preprocessorFunction.newLocal(isolate), context->Global(), WTF_ARRAY_LENGTH(argv), argv);

[abarth-chromium, 2013/07/09 23:14:10]

You can combine these lines.  There's no line limit here.

[johnjbarton, 2013/07/10 23:24:23]

Done.

+
+        if (!resultValue.IsEmpty() && resultValue->IsString())
+            return toWebCoreStringWithNullCheck(resultValue);
+
+        return sourceCode;
+    }
+
+    bool hasPreprocessorFunction()
+    {
+        return !m_preprocessorFunction.isEmpty();
+    }
+
+    bool isPreprocessing()
+    {
+        return m_isPreprocessing;
+    }
+
+private:
+    String m_preprocessorBody;
+    ScopedPersistent<v8::Function> m_preprocessorFunction;
+    ScriptController* m_controller;
+    bool m_isPreprocessing;
+};
+
+bool ScriptController::hasScriptPreprocessor()
+{
+    return m_scriptPreprocessor && m_scriptPreprocessor->hasPreprocessorFunction();
+}
+
+bool ScriptController::isPreprocessingScript()
+{
+    return hasScriptPreprocessor() && m_scriptPreprocessor->isPreprocessing();
+}
+
+void ScriptController::setScriptPreprocessor(const String& preprocessorBody)
+{
+    // We delay the creation of the preprocess until just before the first JS from the
+    // Web page to ensure that the debugger's console initialization code has completed.
+    m_preprocessorSource = preprocessorBody;
+}
+
+void ScriptController::clearScriptPreprocessor()
+{
+    m_scriptPreprocessor.clear();
+    m_preprocessorSource = "";

[abarth-chromium, 2013/07/09 23:14:10]

m_preprocessorSource = String();

That will get you back to exactly the state you started in (i.e., having a null
string---not an empty string).

[johnjbarton, 2013/07/10 23:24:23]

Done.

+}
+
+String ScriptController::preprocess(const String& scriptSource, const String& scriptName)
+{
+    if (m_preprocessorSource.isEmpty())
+        return scriptSource;
+
+    if (!m_scriptPreprocessor)
+        m_scriptPreprocessor = adoptPtr(new ScriptPreprocessor(m_preprocessorSource, this, this->m_frame->page()->console()));

[abarth-chromium, 2013/07/09 23:14:10]

this->m_frame   ->    m_frame

The "this->" isn't needed.

What if page() is zero?

[johnjbarton, 2013/07/10 23:24:23]

Done
I added a test for this case.

+
+    if (!m_scriptPreprocessor->hasPreprocessorFunction())
+        return scriptSource;
+
+    return m_scriptPreprocessor->preprocessSourceCode(scriptSource, scriptName);
+}
+
+
 } // namespace WebCore