Apply script preprocessor to Web page scripts only.

Source/bindings/v8/ScriptController.cpp

 /*
  * Copyright (C) 2008, 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Apple 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.
@@ skipping 48 matching lines @@
 #include "core/html/HTMLPlugInElement.h"
 #include "core/inspector/InspectorInstrumentation.h"
 #include "core/inspector/ScriptCallStack.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/page/ContentSecurityPolicy.h"
 #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"
 #include "core/platform/Widget.h"
 #include "core/platform/chromium/TraceEvent.h"
 #include "core/plugins/PluginView.h"
 #include "weborigin/SecurityOrigin.h"
 #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"
 
 namespace WebCore {
 
 bool ScriptController::canAccessFromCurrentOrigin(Frame *frame)
 {
     return !v8::Context::InContext() || BindingSecurity::shouldAllowAccessToFrame(frame);
 }
@@ skipping 572 matching lines @@
 
     String scriptResult;
     if (!result.getString(scriptResult))
         return true;
 
     // We're still in a frame, so there should be a DocumentLoader.
     ASSERT(m_frame->document()->loader());
 
     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())
         loader->replaceDocument(scriptResult, ownerDocument.get());
     return true;
 }
 
 ScriptValue ScriptController::executeScriptInMainWorld(const ScriptSourceCode& sourceCode)
 {
     String sourceURL = sourceCode.url();
     const String* savedSourceURL = m_sourceURL;
     m_sourceURL = &sourceURL;
 
     v8::HandleScope handleScope;
     v8::Handle<v8::Context> v8Context = ScriptController::mainWorldContext(m_frame);
     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;
 
     if (object.IsEmpty())
         return ScriptValue();
 
     return ScriptValue(object);
 }
 
 void ScriptController::executeScriptInIsolatedWorld(int worldID, const Vector<ScriptSourceCode>& sources, int extensionGroup, Vector<ScriptValue>* results)
@@ skipping 23 matching lines @@
 
         v8Results = evaluateHandleScope.Close(resultArray);
     }
 
     if (results && !v8Results.IsEmpty()) {
         for (size_t i = 0; i < v8Results->Length(); ++i)
             results->append(ScriptValue(v8Results->Get(i)));
     }
 }
 
+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