From b71fee9e486faeb445f4cad32738b3ea04536654 Mon Sep 17 00:00:00 2001
From: Adam Barth <abarth@chromium.org>
Date: Mon, 10 Nov 2014 15:34:25 -0800
Subject: [PATCH] Move exports from Document to Module

This CL moves the |exports| from Document to the new |Module| interface,
matching the spec. Also, the |module| object available to scripts is now really
an instance of |Module|.

R=eseidel@chromium.org

Review URL: https://codereview.chromium.org/703593003
---
 engine/bindings/core/v8/ScriptController.cpp  | 31 ++++++++++---------
 engine/bindings/core/v8/ScriptController.h    |  4 +--
 engine/core/app/AbstractModule.cpp            |  2 ++
 engine/core/dom/Document.cpp                  |  1 +
 engine/core/dom/Document.h                    | 11 ++++---
 engine/core/dom/Document.idl                  |  1 -
 engine/core/frame/LocalDOMWindow.cpp          |  2 ++
 engine/core/frame/LocalDOMWindow.h            |  2 ++
 engine/core/html/imports/HTMLImportChild.cpp  |  6 ++++
 engine/core/html/imports/HTMLImportChild.h    |  3 ++
 engine/core/html/imports/HTMLImportLoader.cpp |  7 ++++-
 engine/core/html/imports/HTMLImportLoader.h   |  4 +++
 engine/core/html/parser/HTMLScriptRunner.cpp  |  3 +-
 .../instance-of-application-expected.txt      |  1 +
 tests/modules/instance-of-application.sky     |  8 +++++
 tests/modules/instance-of-module-expected.txt |  1 +
 tests/modules/instance-of-module.sky          |  8 +++++
 .../resources/instance-of-module-module.sky   |  3 ++
 18 files changed, 73 insertions(+), 25 deletions(-)
 create mode 100644 tests/modules/instance-of-application-expected.txt
 create mode 100644 tests/modules/instance-of-application.sky
 create mode 100644 tests/modules/instance-of-module-expected.txt
 create mode 100644 tests/modules/instance-of-module.sky
 create mode 100644 tests/modules/resources/instance-of-module-module.sky

diff --git a/engine/bindings/core/v8/ScriptController.cpp b/engine/bindings/core/v8/ScriptController.cpp
index 1f5fec302af..eb1bdca0d60 100644
--- a/engine/bindings/core/v8/ScriptController.cpp
+++ b/engine/bindings/core/v8/ScriptController.cpp
@@ -45,6 +45,7 @@
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "bindings/core/v8/V8Window.h"
 #include "bindings/core/v8/WindowProxy.h"
+#include "core/app/Module.h"
 #include "core/dom/Document.h"
 #include "core/dom/Node.h"
 #include "core/events/Event.h"
@@ -328,7 +329,7 @@ void ScriptController::executeScriptInIsolatedWorld(int worldID, const Vector<Sc
     }
 }
 
-void ScriptController::executeModuleScript(Document& document, const String& source, const TextPosition& textPosition)
+void ScriptController::executeModuleScript(AbstractModule& module, const String& source, const TextPosition& textPosition)
 {
     v8::HandleScope handleScope(m_isolate);
     v8::Handle<v8::Context> context = toV8Context(m_frame, DOMWrapperWorld::mainWorld());
@@ -343,31 +344,31 @@ void ScriptController::executeModuleScript(Document& document, const String& sou
     v8::TryCatch tryCatch;
     tryCatch.SetVerbose(true);
 
-    V8ScriptModule module;
-    module.resourceName = document.url().string();
-    module.textPosition = textPosition;
-    // FIXME: This should be the actual module object instead of the document.
-    module.moduleObject = toV8(&document, context->Global(), m_isolate);
-    module.source = source;
+    V8ScriptModule scriptModule;
+    scriptModule.resourceName = module.url();
+    scriptModule.textPosition = textPosition;
+    scriptModule.moduleObject = toV8(&module, context->Global(), m_isolate);
+    scriptModule.source = source;
 
-    if (HTMLImport* parent = document.import()) {
-        for (HTMLImport* child = parent->firstChild(); child; child = child->next()) {
-            if (Element* link = static_cast<HTMLImportChild*>(child)->link()) {
+    if (HTMLImport* parent = module.document()->import()) {
+        for (HTMLImportChild* child = static_cast<HTMLImportChild*>(parent->firstChild());
+             child; child = static_cast<HTMLImportChild*>(child->next())) {
+            if (Element* link = child->link()) {
                 String name = link->getAttribute(HTMLNames::asAttr);
                 if (!name.isEmpty()) {
-                    module.formalDependencies.append(name);
+                    scriptModule.formalDependencies.append(name);
                     v8::Handle<v8::Value> actual;
-                    if (child->document())
-                        actual = child->document()->exports().v8Value();
+                    if (Module* childModule = child->module())
+                        actual = childModule->exports().v8Value();
                     if (actual.IsEmpty())
                         actual = v8::Undefined(m_isolate);
-                    module.resolvedDependencies.append(actual);
+                    scriptModule.resolvedDependencies.append(actual);
                 }
             }
         }
     }
 
-    V8ScriptRunner::runModule(m_isolate, m_frame->document(), module);
+    V8ScriptRunner::runModule(m_isolate, m_frame->document(), scriptModule);
 }
 
 } // namespace blink
diff --git a/engine/bindings/core/v8/ScriptController.h b/engine/bindings/core/v8/ScriptController.h
index 929268ff908..13c039c1cf9 100644
--- a/engine/bindings/core/v8/ScriptController.h
+++ b/engine/bindings/core/v8/ScriptController.h
@@ -44,12 +44,12 @@
 namespace blink {
 
 class DOMWrapperWorld;
-class Document;
 class ExecutionContext;
 class Event;
 class HTMLDocument;
 class KURL;
 class LocalFrame;
+class AbstractModule;
 class ScriptState;
 class ScriptSourceCode;
 class WindowProxy;
@@ -72,7 +72,7 @@ public:
     v8::Local<v8::Value> executeScriptInMainWorldAndReturnValue(const ScriptSourceCode&);
     v8::Local<v8::Value> executeScriptAndReturnValue(v8::Handle<v8::Context>, const ScriptSourceCode&);
 
-    void executeModuleScript(Document& document, const String& source, const TextPosition& textPosition);
+    void executeModuleScript(AbstractModule&, const String& source, const TextPosition& textPosition);
 
     // Executes JavaScript in an isolated world. The script gets its own global scope,
     // its own prototypes for intrinsic JavaScript objects (String, Array, and so-on),
diff --git a/engine/core/app/AbstractModule.cpp b/engine/core/app/AbstractModule.cpp
index 23d7a11866e..c99fa4b61b4 100644
--- a/engine/core/app/AbstractModule.cpp
+++ b/engine/core/app/AbstractModule.cpp
@@ -13,9 +13,11 @@ AbstractModule::AbstractModule(ExecutionContext* context,
   : ContextLifecycleObserver(context),
     document_(document),
     url_(url) {
+  document_->setModule(this);
 }
 
 AbstractModule::~AbstractModule() {
+  document_->setModule(nullptr);
 }
 
 ExecutionContext* AbstractModule::executionContext() const {
diff --git a/engine/core/dom/Document.cpp b/engine/core/dom/Document.cpp
index a8ae9d087e4..e259da84706 100644
--- a/engine/core/dom/Document.cpp
+++ b/engine/core/dom/Document.cpp
@@ -281,6 +281,7 @@ void DocumentVisibilityObserver::setObservedDocument(Document& document)
 Document::Document(const DocumentInit& initializer, DocumentClassFlags documentClasses)
     : ContainerNode(0, CreateDocument)
     , TreeScope(*this)
+    , m_module(nullptr)
     , m_hasNodesWithPlaceholderStyle(false)
     , m_evaluateMediaQueriesOnStyleRecalc(false)
     , m_pendingSheetLayout(NoLayoutWithPendingSheets)
diff --git a/engine/core/dom/Document.h b/engine/core/dom/Document.h
index 3c7cd76af5e..c75a8dea531 100644
--- a/engine/core/dom/Document.h
+++ b/engine/core/dom/Document.h
@@ -60,6 +60,7 @@
 
 namespace blink {
 
+class AbstractModule;
 class AnimationTimeline;
 class Attr;
 class CSSStyleDeclaration;
@@ -196,6 +197,9 @@ public:
     Element* activeElement() const;
     bool hasFocus() const;
 
+    AbstractModule* module() const { return m_module; }
+    void setModule(AbstractModule* module) { m_module = module; }
+
     // DOM methods & attributes for Document
 
     Length viewportDefaultMinWidth() const { return m_viewportDefaultMinWidth; }
@@ -342,9 +346,6 @@ public:
     const KURL& url() const { return m_url; }
     void setURL(const KURL&);
 
-    ScriptValue exports() const { return m_exports; }
-    void setExports(ScriptValue exports) { m_exports = exports; }
-
     // To understand how these concepts relate to one another, please see the
     // comments surrounding their declaration.
     const KURL& baseURL() const { return m_baseURL; }
@@ -713,6 +714,8 @@ private:
 
     DocumentLifecycle m_lifecycle;
 
+    AbstractModule* m_module;
+
     bool m_hasNodesWithPlaceholderStyle;
     bool m_evaluateMediaQueriesOnStyleRecalc;
 
@@ -765,8 +768,6 @@ private:
 
     TextLinkColors m_textLinkColors;
 
-    ScriptValue m_exports;
-
     ReadyState m_readyState;
     bool m_isParsing;
 
diff --git a/engine/core/dom/Document.idl b/engine/core/dom/Document.idl
index 63ad00a4492..c4921e3b36b 100644
--- a/engine/core/dom/Document.idl
+++ b/engine/core/dom/Document.idl
@@ -96,7 +96,6 @@ typedef (CanvasRenderingContext2D or WebGLRenderingContext) RenderingContext;
     readonly attribute boolean hidden;
 
     readonly attribute HTMLScriptElement currentScript;
-    attribute any exports;
 };
 
 Document implements ParentNode;
diff --git a/engine/core/frame/LocalDOMWindow.cpp b/engine/core/frame/LocalDOMWindow.cpp
index 3dfdc787222..f76ba60fe06 100644
--- a/engine/core/frame/LocalDOMWindow.cpp
+++ b/engine/core/frame/LocalDOMWindow.cpp
@@ -34,6 +34,7 @@
 #include "bindings/core/v8/ScriptCallStackFactory.h"
 #include "bindings/core/v8/ScriptController.h"
 #include "bindings/core/v8/SerializedScriptValue.h"
+#include "core/app/Application.h"
 #include "core/css/CSSComputedStyleDeclaration.h"
 #include "core/css/CSSRuleList.h"
 #include "core/css/DOMWindowCSS.h"
@@ -234,6 +235,7 @@ PassRefPtr<Document> LocalDOMWindow::installNewDocument(const DocumentInit& init
     clearDocument();
 
     m_document = HTMLDocument::create(init);
+    m_application = Application::create(m_document.get(), m_document.get(), m_document->url().string());
     m_eventQueue = DOMWindowEventQueue::create(m_document.get());
     m_document->attach();
 
diff --git a/engine/core/frame/LocalDOMWindow.h b/engine/core/frame/LocalDOMWindow.h
index 6b825d7f88f..de295760799 100644
--- a/engine/core/frame/LocalDOMWindow.h
+++ b/engine/core/frame/LocalDOMWindow.h
@@ -40,6 +40,7 @@
 
 namespace blink {
 
+class Application;
 class CSSRuleList;
 class CSSStyleDeclaration;
 class Console;
@@ -234,6 +235,7 @@ private:
 
     void removeAllEventListenersInternal(BroadcastListenerRemoval);
 
+    RefPtr<Application> m_application;
     RefPtr<Document> m_document;
 
 #if ENABLE(ASSERT)
diff --git a/engine/core/html/imports/HTMLImportChild.cpp b/engine/core/html/imports/HTMLImportChild.cpp
index 023d1f07090..6813e650055 100644
--- a/engine/core/html/imports/HTMLImportChild.cpp
+++ b/engine/core/html/imports/HTMLImportChild.cpp
@@ -112,6 +112,12 @@ void HTMLImportChild::importDestroyed()
 }
 #endif
 
+Module* HTMLImportChild::module() const
+{
+    ASSERT(m_loader);
+    return m_loader->module();
+}
+
 Document* HTMLImportChild::document() const
 {
     ASSERT(m_loader);
diff --git a/engine/core/html/imports/HTMLImportChild.h b/engine/core/html/imports/HTMLImportChild.h
index a7fd65a349e..fd63467e42f 100644
--- a/engine/core/html/imports/HTMLImportChild.h
+++ b/engine/core/html/imports/HTMLImportChild.h
@@ -42,6 +42,7 @@ class CustomElementMicrotaskImportStep;
 class HTMLImportLoader;
 class HTMLImportChildClient;
 class Element;
+class Module;
 
 //
 // An import tree node subclas to encapsulate imported document
@@ -65,6 +66,8 @@ public:
     WeakPtr<HTMLImportChild> weakPtr() { return m_weakFactory.createWeakPtr(); }
 #endif
 
+    Module* module() const;
+
     // HTMLImport
     virtual Document* document() const override;
     virtual bool isDone() const override;
diff --git a/engine/core/html/imports/HTMLImportLoader.cpp b/engine/core/html/imports/HTMLImportLoader.cpp
index aeafea991c7..84ae5695b3a 100644
--- a/engine/core/html/imports/HTMLImportLoader.cpp
+++ b/engine/core/html/imports/HTMLImportLoader.cpp
@@ -31,6 +31,7 @@
 #include "config.h"
 #include "core/html/imports/HTMLImportLoader.h"
 
+#include "core/app/Module.h"
 #include "core/dom/Document.h"
 #include "core/dom/DocumentParser.h"
 #include "core/dom/StyleEngine.h"
@@ -64,6 +65,7 @@ void HTMLImportLoader::importDestroyed()
 void HTMLImportLoader::clear()
 {
     m_controller = nullptr;
+    m_module.clear();
     if (m_document) {
         m_document->setImportsController(0);
         m_document->cancelParsing();
@@ -94,10 +96,13 @@ void HTMLImportLoader::OnReceivedResponse(mojo::URLResponsePtr response)
 HTMLImportLoader::State HTMLImportLoader::startWritingAndParsing(mojo::URLResponsePtr response)
 {
     ASSERT(!m_imports.isEmpty());
+    WeakPtr<Document> contextDocument = m_controller->master()->contextDocument();
+    ASSERT(contextDocument.get());
     KURL url(ParsedURLString, String::fromUTF8(response->url));
-    DocumentInit init = DocumentInit(url, 0, m_controller->master()->contextDocument(), m_controller)
+    DocumentInit init = DocumentInit(url, 0, contextDocument, m_controller)
         .withRegistrationContext(m_controller->master()->registrationContext());
     m_document = HTMLDocument::create(init);
+    m_module = Module::create(contextDocument.get(), nullptr, m_document.get(), url.string());
     m_document->startParsing();
     m_document->parser()->parse(response->body.Pass());
     return StateLoading;
diff --git a/engine/core/html/imports/HTMLImportLoader.h b/engine/core/html/imports/HTMLImportLoader.h
index a799a6027e1..a96cc4e6c67 100644
--- a/engine/core/html/imports/HTMLImportLoader.h
+++ b/engine/core/html/imports/HTMLImportLoader.h
@@ -44,6 +44,7 @@ class CustomElementSyncMicrotaskQueue;
 class Document;
 class HTMLImportChild;
 class HTMLImportsController;
+class Module;
 
 class HTMLImportLoader final : public MojoFetcher::Client {
 public:
@@ -63,6 +64,8 @@ public:
     virtual ~HTMLImportLoader();
 
     Document* document() const { return m_document.get(); }
+    Module* module() const { return m_module.get(); }
+
     void addImport(HTMLImportChild*);
 #if !ENABLE(OILPAN)
     void removeImport(HTMLImportChild*);
@@ -112,6 +115,7 @@ private:
     RawPtr<HTMLImportsController> m_controller;
     Vector<RawPtr<HTMLImportChild> > m_imports;
     State m_state;
+    RefPtr<Module> m_module;
     RefPtr<Document> m_document;
     RefPtr<CustomElementSyncMicrotaskQueue> m_microtaskQueue;
 
diff --git a/engine/core/html/parser/HTMLScriptRunner.cpp b/engine/core/html/parser/HTMLScriptRunner.cpp
index 9b636293594..35d7260cf63 100644
--- a/engine/core/html/parser/HTMLScriptRunner.cpp
+++ b/engine/core/html/parser/HTMLScriptRunner.cpp
@@ -59,7 +59,8 @@ void HTMLScriptRunner::executeScript(PassRefPtr<HTMLScriptElement> element, Text
     TemporaryChange<bool> executingScript(m_isExecutingScript, true);
 
     contextDocument->pushCurrentScript(element);
-    frame->script().executeModuleScript(sourceDocument, source, textPosition);
+    ASSERT(sourceDocument.module());
+    frame->script().executeModuleScript(*sourceDocument.module(), source, textPosition);
     contextDocument->popCurrentScript();
 }
 
diff --git a/tests/modules/instance-of-application-expected.txt b/tests/modules/instance-of-application-expected.txt
new file mode 100644
index 00000000000..7ef22e9a431
--- /dev/null
+++ b/tests/modules/instance-of-application-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/tests/modules/instance-of-application.sky b/tests/modules/instance-of-application.sky
new file mode 100644
index 00000000000..9fb7074b8da
--- /dev/null
+++ b/tests/modules/instance-of-application.sky
@@ -0,0 +1,8 @@
+<html>
+<import src="../resources/dump-as-text.sky" />
+<div id="result">FAIL</div>
+<script>
+document.getElementById("result").textContent =
+    module instanceof Application ? 'PASS': 'FAIL: ' + module;
+</script>
+</html>
diff --git a/tests/modules/instance-of-module-expected.txt b/tests/modules/instance-of-module-expected.txt
new file mode 100644
index 00000000000..7ef22e9a431
--- /dev/null
+++ b/tests/modules/instance-of-module-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/tests/modules/instance-of-module.sky b/tests/modules/instance-of-module.sky
new file mode 100644
index 00000000000..5cb30d0811c
--- /dev/null
+++ b/tests/modules/instance-of-module.sky
@@ -0,0 +1,8 @@
+<html>
+<import src="../resources/dump-as-text.sky" />
+<import src="resources/instance-of-module-module.sky" as="result" />
+<div id="result">FAIL</div>
+<script>
+document.getElementById("result").textContent = result;
+</script>
+</html>
diff --git a/tests/modules/resources/instance-of-module-module.sky b/tests/modules/resources/instance-of-module-module.sky
new file mode 100644
index 00000000000..7e53b2df34e
--- /dev/null
+++ b/tests/modules/resources/instance-of-module-module.sky
@@ -0,0 +1,3 @@
+<script>
+module.exports = module instanceof Module ? 'PASS' : 'FAIL: ' + module;
+</script>