Bug 176844

Summary:

Use OptionSet<TextDecoration> instead of bitmask

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

WebCore Misc.

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, darin, mmaxfield, simon.fraser, thisiskatewinslet, zalan

Priority:

Version:

WebKit Local Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-elcapitan	none

Daniel Bates

Reported 2017-09-13 09:08:25 PDT

Use OptionSet<TextDecoration> instead of bitmask.

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 attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2017-09-13 09:12:50 PDT

Created attachment 320643 [details] Patch

Daniel Bates

Comment 2 2017-09-13 09:31:05 PDT

Created attachment 320644 [details] Patch

Daniel Bates

Comment 3 2017-09-13 09:45:55 PDT

Created attachment 320648 [details] Patch

Build Bot

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

Build Bot

Comment 5 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

Myles C. Maxfield

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

Darin Adler

Comment 7 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?

Daniel Bates

Comment 8 2018-08-04 16:48:05 PDT

This was fixed in the patch for bug #186015.

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