[WIP Prototype] ES6 https://html.spec.whatwg.org/#fetch-a-single-module-script implementation

third_party/WebKit/Source/bindings/core/v8/ScriptModule.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ScriptModule_h
+#define ScriptModule_h
+
+#include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/SharedPersistent.h"
+#include "core/CoreExport.h"
+#include "wtf/Allocator.h"
+#include "wtf/HashTableDeletedValueType.h"
+#include "wtf/HashTraits.h"
+#include "wtf/Vector.h"
+#include "wtf/text/WTFString.h"
+#include <v8.h>
+
+namespace blink {
+
+class ScriptModuleIdentifier;
+class ScriptState;
+
+// A ScriptModule wraps a handle to v8::Module for its carry outside V8

[dominicc (has gone to gerrit), 2017/01/11 03:23:47]

Grammar: "for its carry" doesn't make sense. Maybe something like:

ScriptModule wraps a handle to a v8::Module for use in core.

or something like that.p

[kouhei (in TOK), 2017/01/17 05:26:13]

Done.

+// bindings.
+//
+// Note: Use ModulatorImpl to provide scriptState context to actually

[dominicc (has gone to gerrit), 2017/01/11 03:23:47]

Hmm, why do these abstractions need to depend on one another? I feel like this
class is a leaf in the dependency graph. What you're saying here is that
ScriptModule needs a consistent ScriptState provided by someone else, I think.
You can probably make this comment more useful by explaining what's happening
here:

1. Using ScriptModules needs a ScriptState to operate in.
2. You should always provide the same ScriptState and not try to reuse
ScriptModules across different contexts.
3. It's inefficient to store the ScriptState with the ScriptModule, because a
page may have many modules?

BTW, I'm not sure point 3 is really true. If it was true, why do we embed
ScriptState in ScriptValue and ScriptPromise?

[kouhei (in TOK), 2017/01/17 05:26:12]

adamk, dominicc: PTAL

+// operate or extract information from ScriptModule.
+class CORE_EXPORT ScriptModule final {
+  DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
+
+ public:
+  static ScriptModule compile(v8::Isolate*,
+                              const String& source,
+                              const String& fileName);
+
+  ScriptModule() {}
+  ScriptModule(const ScriptModule& module)

[dominicc (has gone to gerrit), 2017/01/11 03:23:47]

Just curious but is it possible to use = default for this?

[kouhei (in TOK), 2017/01/17 05:26:13]

Done.

+      : m_module(module.m_module), m_identityHash(module.m_identityHash) {}
+  ~ScriptModule();
+
+  bool instantiate(ScriptState*);
+  void evaluate(ScriptState*);
+
+  Vector<String> moduleRequests(ScriptState*);
+
+  bool isNull() const { return !m_module || m_module->isEmpty(); }
+  ScriptModuleIdentifier identifier();
+
+ private:
+  friend class ScriptModuleIdentifier;
+  ScriptModule(v8::Isolate*, v8::Local<v8::Module>);
+
+  static v8::MaybeLocal<v8::Module> resolveModuleCallback(
+      v8::Local<v8::Context>,
+      v8::Local<v8::String> specifier,
+      v8::Local<v8::Module> referrer);
+
+  RefPtr<SharedPersistent<v8::Module>> m_module;
+  unsigned m_identityHash = 0;
+};
+
+// TODO(kouhei): Split header/cpp
+// TODO(kouhei): PeekType which can be construct w/ only v8::Local
+class ScriptModuleIdentifier {

[dominicc (has gone to gerrit), 2017/01/11 03:23:47]

Is this class pulling its weight? If you don't add m_scriptState to
ScriptModule, could we just fold this into ScriptModule? It's light enough.

[kouhei (in TOK), 2017/01/17 05:26:12]

Reason 1: One ScriptModule instance per v8::Module
As we shift ScriptModule to non-copyable ScopedPersistent<v8::Module>, I think
it would be clearer to ensure we only have one ScriptModule instance per
ModuleScript.

Reason 2: Identifier from SM::resolveModuleCallback()
In SM::resolveModuleCallback(), we currently construct a full-featured
ScriptModule just for using it for key to lookup in scriptModuleResolver::m_map.
I want to avoid leaking this for other use in the future, and we shouldn't need
a full v8 persistent handle here anyway.

+ public:
+  class Hash;
+
+  ScriptModuleIdentifier() = default;
+  ScriptModuleIdentifier(const ScriptModuleIdentifier& id)
+      : m_module(id.m_module), m_identityHash(id.m_identityHash) {}
+  ScriptModuleIdentifier(WTF::HashTableDeletedValueType)
+      : m_module(WTF::HashTableDeletedValue) {}
+  /*
+  FIXME(kouhei): need move op=
+  ScriptModuleIdentifier(ScriptModuleIdentifier&& id)
+      : m_module(std::move(id.m_module)), m_identityHash(id.m_identityHash) {}
+      */
+  ~ScriptModuleIdentifier() = default;
+
+  bool isHashTableDeletedValue() const {
+    return m_module.isHashTableDeletedValue();
+  }
+
+  bool operator==(const blink::ScriptModuleIdentifier& rhs) const {
+    if (m_module.isHashTableDeletedValue() &&

[dominicc (has gone to gerrit), 2017/01/11 03:23:47]

Hmm, why not just implement so isSafeToCompareEmptyOrDeleted = true? I think
this may be safe already?

[kouhei (in TOK), 2017/01/17 05:26:13]

Done.

+        rhs.m_module.isHashTableDeletedValue())
+      return true;
+
+    blink::SharedPersistent<v8::Module>* lhsp = m_module.get();
+    blink::SharedPersistent<v8::Module>* rhsp = rhs.m_module.get();
+    if (lhsp == rhsp)
+      return true;
+    if (!lhsp || !rhsp)
+      return false;
+    return *lhsp == *rhsp;
+  }
+
+ private:
+  friend class ScriptModule;
+  friend struct ScriptModuleIdentifierHash;
+
+  explicit ScriptModuleIdentifier(
+      const RefPtr<SharedPersistent<v8::Module>>& module,
+      unsigned identityHash)
+      : m_module(module), m_identityHash(identityHash) {}
+
+  // TODO(kouhei): Make ScriptModule move-only and make this ScopedPersistent.
+  RefPtr<SharedPersistent<v8::Module>> m_module;
+  unsigned m_identityHash = 0;
+};
+
+struct ScriptModuleIdentifierHash {
+  STATIC_ONLY(ScriptModuleIdentifierHash);
+
+ public:
+  static unsigned hash(const blink::ScriptModuleIdentifier& key) {
+    return static_cast<unsigned>(key.m_identityHash);
+  }
+
+  static bool equal(const blink::ScriptModuleIdentifier& a,
+                    const blink::ScriptModuleIdentifier& b) {
+    return a == b;
+  }
+
+  static const bool safeToCompareToEmptyOrDeleted = false;
+};
+
+}  // namespace blink
+
+namespace WTF {
+
+template <>
+struct DefaultHash<blink::ScriptModuleIdentifier> {
+  using Hash = blink::ScriptModuleIdentifierHash;
+};
+
+template <>
+struct HashTraits<blink::ScriptModuleIdentifier>
+    : public SimpleClassHashTraits<blink::ScriptModuleIdentifier> {};
+
+}  // namespace WTF
+
+#endif  // ScriptModule_h