1362119 - Moving ScriptLoader in dom/script and split the code in multiple files

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: part 1 - dom/script

Comment 2

[Andrea Marchesini [:baku]]

Attachment: part 2 - split

Comment 3

[Andrea Marchesini [:baku]]

Attachment: part 3 - code style

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

This isn't touching worker script loader yet, right?  HoPang has some patches in progress against the worker script loader that it would be nice not to conflict with.

Comment 5

[Andrea Marchesini [:baku]]

No workers involved at the moment.

Comment 6

[Andrea Marchesini [:baku]]

Attachment: part 1 - dom/script

Comment 7

[(no longer active)]

Sorry Andrea something else came up today which I couldn't push further back.  I'll try to review your patches during the weekend since I may again not be able to get back to them next week either...

Comment 8

[(no longer active)]

Comment on attachment 8864820 [details] [diff] [review]
part 1 - dom/script

Review of attachment 8864820 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/script/ScriptLoader.h
@@ +72,3 @@
>                        uint32_t aVersion,
>                        mozilla::CORSMode aCORSMode,
>                        const mozilla::dom::SRIMetadata &aIntegrity)

Nit: indentation has been changed like this in a bunch of places, do you mind having a look over the patches and fixing things where you can?  The patches are way too large for me to nit on every single indentation issue and probably not really worth your time to fix each and every single issue either.

I really *really* wish we'd do automatic code formatting already.  :/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d77f6b14633
part 1 - Moving dom/base/Script{Loader,Element}.* in dom/script, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8abdd77a92c
part 2 - Splitting ScriptLoader in multiple files, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/30104bffcd4b
part 2 - Fixing the code style in dom/script, r=ehsan

Comment 10

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=97233979&repo=mozilla-inbound

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c440a4d4cc6c
Backed out changeset 30104bffcd4b 
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a935b38ef45
Backed out changeset a8abdd77a92c 
https://hg.mozilla.org/integration/mozilla-inbound/rev/62680dc80fb7
Backed out changeset 5d77f6b14633 for android bustage in nsCCUncollectableMarker.cpp:500:7: error: 'TraceScriptHolder' is not a member of 'mozilla'

Comment 12

[Carsten Book [:Tomcat]]

note: [task 2017-05-08T08:08:57.863366Z] 08:08:57     INFO - GECKO(3072) | [Child 3128] ###!!! ASSERTION: Destroying a currently-showing document: '!mIsShowing', file /home/worker/workspace/build/src/dom/base/nsDocument.cpp, line 1490 

might be also related - https://treeherder.mozilla.org/logviewer.html#?job_id=97237424&repo=mozilla-inbound&lineNumber=9076

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0788c41e592b
part 1 - Moving dom/base/Script{Loader,Element}.* in dom/script, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/692529b223ec
part 2 - Splitting ScriptLoader in multiple files, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0b31688e9f3
part 2 - Fixing the code style in dom/script, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/1054eb4b2782
part 4 - Fixing includes, r=me

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0788c41e592b
https://hg.mozilla.org/mozilla-central/rev/692529b223ec
https://hg.mozilla.org/mozilla-central/rev/f0b31688e9f3
https://hg.mozilla.org/mozilla-central/rev/1054eb4b2782