1447828 - Remove StyleBackendType.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Emilio Cobos Álvarez [:emilio]]

We can do it now that there's just one backend type.

Comment 1

[Jet Villegas (inactive)]

[Triage 2018/03/23 - P3]

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation.

Review commit: https://reviewboard.mozilla.org/r/231892/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231892/

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 2 - Remove StyleBackendType uses from nsXULPrototypeCache.

Review commit: https://reviewboard.mozilla.org/r/231894/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231894/

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 3 - Remove StyleBackendType uses from MediaList.

Review commit: https://reviewboard.mozilla.org/r/231896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231896/

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 4 - Remove StyleBackendType uses from Loader.

Review commit: https://reviewboard.mozilla.org/r/231898/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231898/

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 5 - Remove StyleBackendType uses from nsStyleSheetService.

Review commit: https://reviewboard.mozilla.org/r/231900/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231900/

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 6 - Have only a single nsLayoutStylesheetCache instance.

Review commit: https://reviewboard.mozilla.org/r/231902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231902/

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 7 - Remove StyleBackendType uses from PreloadedStyleSheet.

Review commit: https://reviewboard.mozilla.org/r/231904/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231904/

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1447828 part 8 - Remove remaining uses of StyleBackendType as well as the type itself.

Review commit: https://reviewboard.mozilla.org/r/231906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231906/

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963060 [details]
Bug 1447828 part 8 - Remove remaining uses of StyleBackendType as well as the type itself.

https://reviewboard.mozilla.org/r/231906/#review237472

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963059 [details]
Bug 1447828 part 7 - Remove StyleBackendType uses from PreloadedStyleSheet.

https://reviewboard.mozilla.org/r/231904/#review237474

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963058 [details]
Bug 1447828 part 6 - Have only a single nsLayoutStylesheetCache instance.

https://reviewboard.mozilla.org/r/231902/#review237476

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963057 [details]
Bug 1447828 part 5 - Remove StyleBackendType uses from nsStyleSheetService.

https://reviewboard.mozilla.org/r/231900/#review237478

Comment 14

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Servo PR

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963056 [details]
Bug 1447828 part 4 - Remove StyleBackendType uses from Loader.

https://reviewboard.mozilla.org/r/231898/#review237538

::: layout/style/Loader.cpp:385
(Diff revision 1)
>  }
>  
> +Loader::Loader(DocGroup* aDocGroup)
> +  : Loader()
> +{
> +  mDocGroup = aDocGroup;

nit: Put this in the initializer list?

::: layout/style/Loader.cpp:391
(Diff revision 1)
> -  , mReporter(new ConsoleReportCollector())
> -#ifdef DEBUG
> -  , mSyncCallback(false)
> -#endif
>  {
> +  mDocument = aDocument;

Ditto

::: layout/style/PreloadedStyleSheet.cpp:61
(Diff revision 1)
>  
>    RefPtr<StyleSheet>& sheet =
>      aType == StyleBackendType::Gecko ? mGecko : mServo;
>  
>    if (!sheet) {
> -    RefPtr<css::Loader> loader = new css::Loader(aType, nullptr);
> +    RefPtr<css::Loader> loader = new css::Loader;

We have MakeRefPtr, not sure if it's cleaner or not... Certainly doesn't matter much.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963055 [details]
Bug 1447828 part 3 - Remove StyleBackendType uses from MediaList.

https://reviewboard.mozilla.org/r/231896/#review237540

::: layout/style/MediaQueryList.cpp:30
(Diff revision 1)
>                                 CallerType aCallerType)
>    : mDocument(aDocument)
>    , mMatches(false)
>    , mMatchesValid(false)
>  {
> -  mMediaList =
> +  mMediaList = MediaList::Create(aMediaQueryList, aCallerType);

nit: Now it fits in one line, so maybe move to the initializer? Doesn't matter much in any case.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963054 [details]
Bug 1447828 part 2 - Remove StyleBackendType uses from nsXULPrototypeCache.

https://reviewboard.mozilla.org/r/231894/#review237542

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963053 [details]
Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation.

https://reviewboard.mozilla.org/r/231892/#review237546

::: layout/style/ServoBindings.cpp:678
(Diff revision 1)
>        // MaybeUpdateCascadeResults) since we know for sure that the cascade has
>        // changed, but we were unable to call MarkCascadeUpdated when we noticed
>        // it since we avoid mutating state as part of the Servo parallel
>        // traversal.
>        presContext->EffectCompositor()
> -                 ->UpdateCascadeResults(StyleBackendType::Servo,
> +                 ->UpdateCascadeResults(*effectSet,

nit (feel free to ignore): I _think_ this can fit in the line above now.

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8963054 [details]
Bug 1447828 part 2 - Remove StyleBackendType uses from nsXULPrototypeCache.

https://reviewboard.mozilla.org/r/231894/#review237542

Comment 20

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8963053 [details]
Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation.

https://reviewboard.mozilla.org/r/231892/#review237546

> nit (feel free to ignore): I _think_ this can fit in the line above now.

It does but you cannot make your remaining args fit.

Comment 21

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8963056 [details]
Bug 1447828 part 4 - Remove StyleBackendType uses from Loader.

https://reviewboard.mozilla.org/r/231898/#review237538

> nit: Put this in the initializer list?

I'd rather not... It is not completely clear to me the rule of initializer list when there is delegated constructor involves, and I don't really want to wait for another try push to check all the compilers :/

> Ditto

Ditto.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dbfda12b1fe
part 1 - Remove StyleBackendType uses from dom/animation. r=emilio
https://hg.mozilla.org/integration/autoland/rev/f39ff5342c9d
part 2 - Remove StyleBackendType uses from nsXULPrototypeCache. r=emilio
https://hg.mozilla.org/integration/autoland/rev/8f67af6a636e
part 3 - Remove StyleBackendType uses from MediaList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/151e6133adcf
part 4 - Remove StyleBackendType uses from Loader. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ea7634ccd024
part 5 - Remove StyleBackendType uses from nsStyleSheetService. r=emilio
https://hg.mozilla.org/integration/autoland/rev/b9afe5965374
part 6 - Have only a single nsLayoutStylesheetCache instance. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d52333f70551
part 7 - Remove StyleBackendType uses from PreloadedStyleSheet. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ddf011ffcbc1
part 8 - Remove remaining uses of StyleBackendType as well as the type itself. r=emilio

Comment 23

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/8dbfda12b1fe
https://hg.mozilla.org/mozilla-central/rev/f39ff5342c9d
https://hg.mozilla.org/mozilla-central/rev/8f67af6a636e
https://hg.mozilla.org/mozilla-central/rev/151e6133adcf
https://hg.mozilla.org/mozilla-central/rev/ea7634ccd024
https://hg.mozilla.org/mozilla-central/rev/b9afe5965374
https://hg.mozilla.org/mozilla-central/rev/d52333f70551
https://hg.mozilla.org/mozilla-central/rev/ddf011ffcbc1