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 2af23acf31070007d73d28b16445f2ec849cb717..48b9ae07872168855bb18e568af77e4c3f05f12e 100644
--- a/Source/bindings/v8/ScriptController.cpp
+++ b/Source/bindings/v8/ScriptController.cpp
@@ -38,6 +38,7 @@
 #include "bindings/v8/BindingSecurity.h"
 #include "bindings/v8/NPV8Object.h"
 #include "bindings/v8/ScriptCallStackFactory.h"
+#include "bindings/v8/ScriptPreprocessor.h"
 #include "bindings/v8/ScriptSourceCode.h"
 #include "bindings/v8/ScriptValue.h"
 #include "bindings/v8/V8Binding.h"
@@ -515,6 +516,7 @@ void ScriptController::clearWindowShell()
     for (IsolatedWorldMap::iterator iter = m_isolatedWorlds.begin(); iter != m_isolatedWorlds.end(); ++iter)
         iter->value->clearForNavigation();
     V8GCController::hintForCollectGarbage();
+    clearScriptPreprocessor();
     HistogramSupport::histogramCustomCounts("WebCore.ScriptController.clearWindowShell", (currentTime() - start) * 1000, 0, 10000, 50);
 }
 
@@ -646,7 +648,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())
@@ -665,9 +667,12 @@ ScriptValue ScriptController::executeScriptInMainWorld(const ScriptSourceCode& s
     if (v8Context.IsEmpty())
         return ScriptValue();
 
+    String processedString = m_frame->script()->preprocess(sourceCode.source(), sourceURL);

[haraken, 2013/07/11 01:13:59]

What's the difference between preprocess() and m_frame->script()->preprocess() ?

[johnjbarton, 2013/07/11 19:14:42]

Done.

+    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;
 
@@ -711,4 +716,45 @@ void ScriptController::executeScriptInIsolatedWorld(int worldID, const Vector<Sc
     }
 }
 
+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 = String();
+}
+
+String ScriptController::preprocess(const String& scriptSource, const String& scriptName)

[haraken, 2013/07/11 01:13:59]

Would you add a comment here about the exact condition under which the
preprocessing is conducted?

[johnjbarton, 2013/07/11 19:14:42]

Done. (The comment was also in the .h file).

+{
+    if (m_preprocessorSource.isEmpty())
+        return scriptSource;
+
+    if (!m_frame->page())
+        return scriptSource;
+
+    if (!m_scriptPreprocessor)
+        m_scriptPreprocessor = adoptPtr(new ScriptPreprocessor(m_preprocessorSource, this, m_frame->page()->console()));
+
+    if (!m_scriptPreprocessor->hasPreprocessorFunction())
+        return scriptSource;
+
+    return m_scriptPreprocessor->preprocessSourceCode(scriptSource, scriptName);
+}

[haraken, 2013/07/11 01:13:59]

I'm not happy with having a lot of proxy methods to ScriptPreprocessor. Also the
methods are not well organized. (Do we really need all of hasScriptPreprocess,
isPreprocessing, m_preprocessor.isEmpty, m_scriptPreprocessor etc ?) It's very
confusing when we should check what. Please clean up the methods.

[johnjbarton, 2013/07/11 19:14:42]

Two objectives lead to proxy methods:
  1) encapsulate the ScriptPreprocessor to avoid distributing the logic for its
lazy creation (lazy creation motivated by avoiding entangling preprocessing and
console script injection).
  2) conditional access to the ScriptPreprocessor by ScriptDebugServer for
eval() processing. 

I think #1 is important. 

To avoid #2 we would have to call into the debugger-specific functions of
ScriptDebugServer from ScriptPreprocessor. I implemented this version for your
evaluation.

Of course my logic could be flawed or there may be another solution.

+
+
 } // namespace WebCore