1449806 - Remove GenericSpecifiedValues

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

After the old style system got removed, we can replace GenericSpecifiedValues with just ServoSpecifiedValues everywhere.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Introduce MappedDeclarations.

The idea would be that this header is only included in cpp files, thus it's ok
to include ServoBindings, etc.

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

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix function declarations in headers.

Automatic patch with sed, but I forgot to paste the exact command.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix cpp signatures and forwarding.

$ find . -type f -name '*.cpp' -exec sed -i 's/GenericSpecifiedValues\* aData/MappedDeclarations\& aDecls/g' {} \;;
$ find . -type f -name '*.cpp' -exec sed -i 's/aAttributes, aData/aAttributes, aDecls/g' {} \;;

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

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix cpp setters.

$ find . -type f -name '*.cpp' -exec sed -i 's/aData->Set/aDecls.Set/g' {} \;;

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

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix PropertyIsSet checks.

$ find . -type f -name '*.cpp' -exec sed -i 's/aData->Property/aDecls.Property/g' {} \;;

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

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix Document() accessors.

$ find . -type f -name '*.cpp' -exec sed -i 's/aData->Document/aDecls.Document/g' {} \;;

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

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Fix up *Inlines.h includes.

$ find . -type f -name '*.cpp' -exec sed -i 's/GenericSpecifiedValuesInlines/MappedDeclarations/g' {} \;;

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

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Some manual fixes.

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

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Final touch.

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

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1449806: Remove MOZ_STYLO_* macros.

Yay :)

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

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987131 [details]
Bug 1449806: Introduce MappedDeclarations.

https://reviewboard.mozilla.org/r/252354/#review259058

::: layout/style/MappedDeclarations.h:23
(Diff revision 1)
> +// This provides a common interface for attribute mappers
> +// (MapAttributesIntoRule) to use regardless of the style backend. If the style
> +// backend is Gecko, this will contain an nsRuleData. If it is Servo, it will be
> +// a PropertyDeclarationBlock.

This comment should be updated, I guess. The old style system is no longer relevant.

::: layout/style/MappedDeclarations.h:43
(Diff revision 1)
> +    return mDocument;
> +  }
> +
> +  already_AddRefed<RawServoDeclarationBlock> TakeDeclarationBlock()
> +  {
> +    return mDecl.forget();

Maybe add an assertion here that mDecl isn't double taken, and another assertion in destructor that mDecl has been taken.

::: layout/style/MappedDeclarations.h:210
(Diff revision 1)
> +  RefPtr<RawServoDeclarationBlock> mDecl;
> +};
> +
> +} // namespace mozilla
> +
> +#endif

// mozilla_MappedDeclarations_h

::: layout/style/MappedDeclarations.cpp:7
(Diff revision 1)
> +/* vim: set ts=8 sts=2 et sw=2 tw=80: */
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#include "MappedDeclarations.h"

`"mozilla/MappedDeclarations.h"`

Although they probably don't have difference effectively but it's probably better to keep consistency.

::: layout/style/MappedDeclarations.cpp:41
(Diff revision 1)
> +  aValue.ToString(str);
> +  Servo_DeclarationBlock_SetBackgroundImage(
> +    mDecl, str, mDocument->DefaultStyleAttrURLData());
> +}
> +
> +

I don't think we need two empty lines here.

Comment 12

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987132 [details]
Bug 1449806: Fix function declarations in headers.

https://reviewboard.mozilla.org/r/252356/#review259062

Comment 13

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987133 [details]
Bug 1449806: Fix cpp signatures and forwarding.

https://reviewboard.mozilla.org/r/252358/#review259064

Comment 14

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987134 [details]
Bug 1449806: Fix cpp setters.

https://reviewboard.mozilla.org/r/252360/#review259066

Comment 15

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987135 [details]
Bug 1449806: Fix PropertyIsSet checks.

https://reviewboard.mozilla.org/r/252362/#review259068

Comment 16

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987136 [details]
Bug 1449806: Fix Document() accessors.

https://reviewboard.mozilla.org/r/252364/#review259070

Comment 17

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987137 [details]
Bug 1449806: Fix up *Inlines.h includes.

https://reviewboard.mozilla.org/r/252366/#review259074

Comment 18

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987138 [details]
Bug 1449806: Some manual fixes.

https://reviewboard.mozilla.org/r/252368/#review259076

Comment 19

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987139 [details]
Bug 1449806: Final touch.

https://reviewboard.mozilla.org/r/252370/#review259078

Comment 20

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8987140 [details]
Bug 1449806: Remove MOZ_STYLO_* macros.

https://reviewboard.mozilla.org/r/252372/#review259080

\o/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ba328c141b7
Merge {Servo,Generic}SpecifiedValues into MappedDeclarations. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/d535d6534b35
Remove MOZ_STYLO_* macros. r=xidorn

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ac1b18d489e
followup: Fix a typo that I made while addressing review comments. r=me CLOSED TREE

Comment 23

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5ba328c141b7
https://hg.mozilla.org/mozilla-central/rev/d535d6534b35
https://hg.mozilla.org/mozilla-central/rev/0ac1b18d489e