module.exports should default to an empty object.

Per the spec in modules.md the exports property should default to an
empty object. We lazy allocate it so that modules that just replace it
don't create the empty object and then throw it away.

R=abarth@chromium.org

Review URL: https://codereview.chromium.org/872043003

engine/bindings/core/v8/ScriptController.cpp

@@ -295,7 +295,7 @@ void ScriptController::executeModuleScript(AbstractModule& module, const String&
                     scriptModule.formalDependencies.append(name);
                     v8::Handle<v8::Value> actual;
                     if (Module* childModule = child->module())
-                        actual = childModule->exports().v8Value();
+                        actual = childModule->exports(scriptState).v8Value();
                     if (actual.IsEmpty())
                         actual = v8::Undefined(m_isolate);
                     scriptModule.resolvedDependencies.append(actual);

engine/bindings/core/v8/ScriptValue.h

@@ -60,6 +60,11 @@ public:
         ASSERT(isEmpty() || m_scriptState);
     }
 
+    static ScriptValue createEmptyObject(ScriptState* scriptState)
+    {
+        return ScriptValue(scriptState, v8::Object::New(scriptState->isolate()));
+    }
+
     ScriptState* scriptState() const
     {
         return m_scriptState.get();

engine/bindings/templates/attributes.cpp

@@ -249,6 +249,9 @@ v8::Local<v8::Value> v8Value, const v8::PropertyCallbackInfo<void>& info
           attribute.is_setter_call_with_execution_context %}
     ExecutionContext* executionContext = currentExecutionContext(info.GetIsolate());
     {% endif %}
+    {% if attribute.is_call_with_script_state %}
+    ScriptState* scriptState = ScriptState::current(info.GetIsolate());
+    {% endif %}
     {# Set #}
     {{attribute.cpp_setter}};
     {# Post-set #}

engine/core/app/AbstractModule.cpp

@@ -41,7 +41,7 @@ void AbstractModule::OnModuleLoadComplete(ModuleLoader* loader,
                                           Module* module) {
   RefPtr<ScriptPromiseResolver> resolver = loaders_.take(loader);
   ScriptState::Scope scope(resolver->scriptState());
-  resolver->resolve(module->exports().v8Value());
+  resolver->resolve(module->exports(resolver->scriptState()).v8Value());
 }
 
 } // namespace blink

engine/core/app/Module.cpp

@@ -29,4 +29,14 @@ Application* Module::GetApplication() {
   return application();
 }
 
+void Module::setExports(ScriptState*, const ScriptValue& exports) {
+  exports_ = exports;
+}
+
+const ScriptValue& Module::exports(ScriptState* scriptState) const {
+  if (exports_.isEmpty())
+    exports_ = ScriptValue::createEmptyObject(scriptState);
+  return exports_;
+}
+
 } // namespace blink

engine/core/app/Module.h

@@ -24,8 +24,8 @@ public:
 
   Application* application() const { return application_.get(); }
 
-  void setExports(const ScriptValue& exports) { exports_ = exports; }
-  const ScriptValue& exports() const { return exports_; }
+  void setExports(ScriptState*, const ScriptValue& exports);
+  const ScriptValue& exports(ScriptState*) const;
 
 private:
   Module(ExecutionContext* context,
@@ -37,7 +37,7 @@ private:
   Application* GetApplication() override;
 
   RefPtr<Application> application_;
-  ScriptValue exports_;
+  mutable ScriptValue exports_;
 };
 
 } // namespace blink

engine/core/app/Module.idl

@@ -7,5 +7,5 @@
     Constructor(Application application, Document document, DOMString url),
 ] interface Module : AbstractModule {
     readonly attribute Application application;
-    attribute any exports;
+    [CallWith=ScriptState] attribute any exports;
 };

examples/city-list/city-list.sky

@@ -7,11 +7,6 @@
 <import src="city-data-service.sky" as="CityDataService" />
 <import src="city-sequence.sky" as="CitySequence" />
 
-<script>
-// TODO(esprehn): exports should start as the empty object.
-module.exports = {};
-</script>
-
 <sky-element name="state-header">
 <template>
   <style>

tests/modules/import-without-export.sky

@@ -3,7 +3,9 @@
 <import src="../resources/dump-as-text.sky" />
 <div id="result">FAIL - Script did not execute</div>
 <script>
-document.getElementById("result").textContent =
-    hello === undefined ? "PASS" : "FAIL";
+var result = "FAIL";
+if (typeof hello === "object" && Object.keys(hello).length === 0)
+  result = "PASS";
+document.getElementById("result").textContent = result;
 </script>
 </html>

tests/modules/modules-expected.txt

@@ -1,5 +1,6 @@
-Running 1 tests
+Running 2 tests
 ok 1 Module should be constructable
-1 tests
-1 pass
+ok 2 Module should have an empty object by default for exports
+2 tests
+2 pass
 0 fail

tests/modules/modules.sky

@@ -13,6 +13,13 @@ describe('Module', function() {
     assert.equal(module.document, doc2);
     assert.equal(module.url, "http://www.example.com/module");
   });
+
+  it('should have an empty object by default for exports', function() {
+    var app = new Application(new Document(), "http://www.example.com/app");
+    var module = new Module(app, new Document(), "http://www.example.com/module");
+    assert.isObject(module.exports);
+    assert.lengthOf(Object.keys(module.exports), 0);
+  });
 });
 </script>
 </html>