[ES6 modules] Fix context leak. ModuleScript should use TraceWrapperV8Reference to hold onto v8::Mo…

third_party/WebKit/Source/core/dom/ModuleScript.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 ModuleScript_h
 #define ModuleScript_h
 
 #include "bindings/core/v8/ScriptModule.h"
 #include "bindings/core/v8/ScriptValue.h"
 #include "core/CoreExport.h"
@@ skipping 35 matching lines @@
   // and allows null ScriptModule.
   static ModuleScript* CreateForTest(Modulator*,
                                      ScriptModule,
                                      const KURL& base_url,
                                      const String& nonce,
                                      ParserDisposition,
                                      WebURLRequest::FetchCredentialsMode);
 
   ~ModuleScript() override = default;
 
-  const ScriptModule& Record() const { return record_; }
+  ScriptModule Record() const;
   const KURL& BaseURL() const { return base_url_; }
 
   ModuleInstantiationState InstantiationState() const {
     return instantiation_state_;
   }
 
   // Implements Step 7.1 of:
   // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure
   void SetInstantiationErrorAndClearRecord(ScriptValue error);
   // Implements Step 7.2 of:
@@ skipping 13 matching lines @@
   DECLARE_TRACE();
   DECLARE_TRACE_WRAPPERS();
 
  private:
   ModuleScript(Modulator* settings_object,
                ScriptModule record,
                const KURL& base_url,
                const String& nonce,
                ParserDisposition parser_state,
                WebURLRequest::FetchCredentialsMode credentials_mode,
-               const String& source_text)
-      : settings_object_(settings_object),
-        record_(record),
-        base_url_(base_url),
-        instantiation_error_(this),
-        nonce_(nonce),
-        parser_state_(parser_state),
-        credentials_mode_(credentials_mode),
-        source_text_(source_text) {}
+               const String& source_text);
 
   static ModuleScript* CreateInternal(const String& source_text,
                                       Modulator*,
                                       ScriptModule,
                                       const KURL& base_url,
                                       const String& nonce,
                                       ParserDisposition,
                                       WebURLRequest::FetchCredentialsMode);
 
   ScriptType GetScriptType() const override { return ScriptType::kModule; }
   bool IsEmpty() const override;
   bool CheckMIMETypeBeforeRunScript(Document* context_document,
                                     const SecurityOrigin*) const override;
   void RunScript(LocalFrame*, const SecurityOrigin*) const override;
   String InlineSourceTextForCSP() const override;
 
   friend class ModulatorImpl;
   friend class ModuleTreeLinkerTestModulator;
   // Access this func only via ModulatorImpl::GetInstantiationError(),
   // or via Modulator mocks for unit tests.
   v8::Local<v8::Value> CreateInstantiationErrorInternal(
       v8::Isolate* isolate) const {
     return instantiation_error_.NewLocal(isolate);
   }
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#settings-object
   Member<Modulator> settings_object_;
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-module-script-module-record
-  ScriptModule record_;

[haraken, 2017/05/24 09:28:12]

Is there any other place that is holding ScriptModule in an on-heap project? In
general it has a risk of causing leaks like this.

[kouhei (in TOK), 2017/05/24 13:06:36]

Yes. ScriptModuleResolverImpl. There I plan to make the SharedPersistent use
SetPhantom or SetWeak.

+  TraceWrapperV8Reference<v8::Module> record_;
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-module-script-base-url
   const KURL base_url_;
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-module-script-instantiation-state
   ModuleInstantiationState instantiation_state_ =
       ModuleInstantiationState::kUninstantiated;
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-module-script-instantiation-error
   //
@@ skipping 14 matching lines @@
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-module-script-credentials-mode
   const WebURLRequest::FetchCredentialsMode credentials_mode_;
 
   // For CSP check.
   const String source_text_;
 };
 
 }  // namespace blink
 
 #endif