Closed Bug 1447828 Opened 6 years ago Closed 6 years ago

Remove StyleBackendType.

Categories

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

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: emilio, Assigned: xidorn)

References

Details

Attachments

(9 files)

Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 2 - Remove StyleBackendType uses from nsXULPrototypeCache.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 3 - Remove StyleBackendType uses from MediaList.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 4 - Remove StyleBackendType uses from Loader.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 5 - Remove StyleBackendType uses from nsStyleSheetService.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 6 - Have only a single nsLayoutStylesheetCache instance.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 7 - Remove StyleBackendType uses from PreloadedStyleSheet.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Bug 1447828 part 8 - Remove remaining uses of StyleBackendType as well as the type itself.
59 bytes, text/x-review-board-request
emilio
: review+
Details
Servo PR
41 bytes, text/x-github-pull-request
Details | Review 
We can do it now that there's just one backend type.
[Triage 2018/03/23 - P3]
Priority: -- → P3
Assignee: nobody → xidorn+moz
Blocks: 1449400
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
Attachment #8963060 - Flags: review?(emilio) → review+
Comment on attachment 8963059 [details]
Bug 1447828 part 7 - Remove StyleBackendType uses from PreloadedStyleSheet.

https://reviewboard.mozilla.org/r/231904/#review237474
Attachment #8963059 - Flags: review?(emilio) → review+
Comment on attachment 8963058 [details]
Bug 1447828 part 6 - Have only a single nsLayoutStylesheetCache instance.

https://reviewboard.mozilla.org/r/231902/#review237476
Attachment #8963058 - Flags: review?(emilio) → review+
Comment on attachment 8963057 [details]
Bug 1447828 part 5 - Remove StyleBackendType uses from nsStyleSheetService.

https://reviewboard.mozilla.org/r/231900/#review237478
Attachment #8963057 - Flags: review?(emilio) → review+
Attached file Servo PR — 

    
    

    
  
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.

        Attachment #8963056 -
        Flags: review?(emilio) → review+

    
    

    
  
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.

        Attachment #8963055 -
        Flags: review?(emilio) → review+

    
    

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

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

        Attachment #8963054 -
        Flags: review?(emilio) → review+

    
    

    
  
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.

        Attachment #8963053 -
        Flags: review?(emilio) → review+

    
    

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

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

    
    

    
  
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 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.

    
    

    
  
Pushed by xquan@mozilla.com:
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

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: