RESOLVED FIXED 166888
Decouple module loading initiator from ScriptElement 
https://bugs.webkit.org/show_bug.cgi?id=166888
Summary Decouple module loading initiator from ScriptElement
Yusuke Suzuki
Reported 2017-01-10 06:31:28 PST
Decouple module loading initiator from ScriptElement
Attachments
Patch (57.69 KB, patch)
2017-01-10 06:37 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (57.80 KB, patch)
2017-01-10 18:49 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (57.82 KB, patch)
2017-01-10 19:15 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (57.68 KB, patch)
2017-01-11 02:33 PST, Yusuke Suzuki 		rniwa: review+
DetailsFormatted DiffDiff
Patch for landing (91.89 KB, patch)
2017-01-11 04:09 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2017-01-10 06:37:32 PST
Created attachment 298460 [details] Patch
Yusuke Suzuki
Comment 2 2017-01-10 18:49:59 PST
Created attachment 298545 [details] Patch
Yusuke Suzuki
Comment 3 2017-01-10 19:15:33 PST
Created attachment 298546 [details] Patch
Saam Barati
Comment 4 2017-01-11 02:21:26 PST
Comment on attachment 298546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298546&action=review This LGTM, but it's worth having somebody else who knows the WebCore bits more than I do give the thumbs up. > Source/JavaScriptCore/runtime/JSScriptInitiator.h:2 > + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> 2017 > Source/JavaScriptCore/runtime/ScriptInitiator.h:2 > + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> 2017 > Source/WebCore/ForwardingHeaders/runtime/JSScriptInitiator.h:2 > + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> 2017 > Source/WebCore/ForwardingHeaders/runtime/ScriptInitiator.h:2 > + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> 2017
Yusuke Suzuki
Comment 5 2017-01-11 02:30:17 PST
Comment on attachment 298546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298546&action=review Thanks :) >> Source/JavaScriptCore/runtime/JSScriptInitiator.h:2 >> + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> > > 2017 Fixed. >> Source/JavaScriptCore/runtime/ScriptInitiator.h:2 >> + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> > > 2017 Fixed. >> Source/WebCore/ForwardingHeaders/runtime/JSScriptInitiator.h:2 >> + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> > > 2017 Fixed. >> Source/WebCore/ForwardingHeaders/runtime/ScriptInitiator.h:2 >> + * Copyright (C) 2016 Yusuke Suzuki <utatane.tea@gmail.com> > > 2017 Fixed.
Yusuke Suzuki
Comment 6 2017-01-11 02:33:36 PST
Created attachment 298564 [details] Patch
Ryosuke Niwa
Comment 7 2017-01-11 02:47:27 PST
Comment on attachment 298546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298546&action=review > Source/JavaScriptCore/ChangeLog:17 > + * runtime/JSScriptInitiator.cpp: Copied from Source/WebCore/dom/LoadableScript.cpp. > + (JSC::JSScriptInitiator::destroy): > + * runtime/JSScriptInitiator.h: Added. I'm not sure if "initiator" is a good name for this. How about ScriptLoader or ScriptFetcher? Or perhaps ScriptOrigin?
Ryosuke Niwa
Comment 8 2017-01-11 02:48:02 PST
Comment on attachment 298564 [details] Patch r=me with my previous comments.
Yusuke Suzuki
Comment 9 2017-01-11 03:21:37 PST
Comment on attachment 298546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298546&action=review Thank you for your review. >> Source/JavaScriptCore/ChangeLog:17 >> + * runtime/JSScriptInitiator.h: Added. > > I'm not sure if "initiator" is a good name for this. How about ScriptLoader or ScriptFetcher? > Or perhaps ScriptOrigin? I think `ScriptFetcher` is better since ScriptLoader is ambiguous with ModuleLoader thing in JSC.
Yusuke Suzuki
Comment 10 2017-01-11 04:09:58 PST
Created attachment 298567 [details] Patch for landing
Yusuke Suzuki
Comment 11 2017-01-11 04:13:46 PST
Committed r210585: <http://trac.webkit.org/changeset/210585>
Note You need to log in before you can comment on or make changes to this bug.