Bug 176844 - Use OptionSet<TextDecoration> instead of bitmask
Summary: Use OptionSet<TextDecoration> instead of bitmask
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-13 09:08 PDT by Daniel Bates
Modified: 2022-09-07 08:28 PDT (History)
6 users (show)

See Also:

Attachments
Patch (41.33 KB, patch)
2017-09-13 09:12 PDT, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch (43.17 KB, patch)
2017-09-13 09:31 PDT, Daniel Bates 		no flags Details | Formatted Diff | Diff
Patch (44.87 KB, patch)
2017-09-13 09:45 PDT, Daniel Bates 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-elcapitan (338.54 KB, application/zip)
2017-09-13 10:56 PDT, Build Bot 		no flags Details
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2017-09-13 09:08:25 PDT 
Use OptionSet<TextDecoration> instead of bitmask.
Comment 1 Daniel Bates 2017-09-13 09:12:50 PDT 
Created attachment 320643 [details]
Patch
Comment 2 Daniel Bates 2017-09-13 09:31:05 PDT 
Created attachment 320644 [details]
Patch
Comment 3 Daniel Bates 2017-09-13 09:45:55 PDT 
Created attachment 320648 [details]
Patch
Comment 4 Build Bot 2017-09-13 10:56:30 PDT 
Comment on attachment 320648 [details]
Patch

Attachment 320648 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4535312

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2017-09-13 10:56:31 PDT 
Created attachment 320659 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Myles C. Maxfield 2017-09-13 17:00:08 PDT 
Comment on attachment 320648 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320648&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1764
> +static Ref<CSSValue> renderTextDecorationFlagsToCSSValue(const OptionSet<TextDecoration>& textDecorations)

We went from passing by value to passing by reference. Does this mean that this patch migrates from a lightweight type to a heavyweight type? If so, that's a little troubling.
Comment 7 Darin Adler 2017-11-13 09:21:55 PST 
Comment on attachment 320648 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320648&action=review

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1764
>> +static Ref<CSSValue> renderTextDecorationFlagsToCSSValue(const OptionSet<TextDecoration>& textDecorations)
> 
> We went from passing by value to passing by reference. Does this mean that this patch migrates from a lightweight type to a heavyweight type? If so, that's a little troubling.

No, should go back to passing by value, here, and in many other places.

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:295
> +        OptionSet<TextDecoration> decorations = style.textDecorationsInEffect();

auto?

> Source/WebCore/style/InlineTextBoxStyle.cpp:109
> +    OptionSet<TextDecoration> decorations = lineStyle.textDecorationsInEffect();

auto?
Comment 8 Daniel Bates 2018-08-04 16:48:05 PDT 
This was fixed in the patch for bug #186015.