Bug 162643 - Use Color references where possible
Summary: Use Color references where possible
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-09-27 16:38 PDT by Dean Jackson
Modified: 2016-09-28 13:22 PDT (History)
3 users (show)

See Also:


Attachments
Patch (54.97 KB, patch)
2016-09-27 16:45 PDT, Dean Jackson
simon.fraser: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (1.21 MB, application/zip)
2016-09-27 17:52 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.38 MB, application/zip)
2016-09-27 17:54 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (1.86 MB, application/zip)
2016-09-27 17:57 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2 (8.08 MB, application/zip)
2016-09-27 18:06 PDT, Build Bot
no flags Details
Patch (52.54 KB, patch)
2016-09-28 11:34 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2016-09-27 16:38:31 PDT
Use Color references where possible
Comment 1 Radar WebKit Bug Importer 2016-09-27 16:41:13 PDT
<rdar://problem/28506550>
Comment 2 Dean Jackson 2016-09-27 16:45:33 PDT
Created attachment 290021 [details]
Patch
Comment 3 Simon Fraser (smfr) 2016-09-27 17:35:42 PDT
Comment on attachment 290021 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsContext.cpp:920
> +    const Color& oldFillColor = fillColor();

Don't you actually need to make a copy here?

> Source/WebCore/rendering/style/RenderStyle.cpp:1210
> +const Color& RenderStyle::color() const
> +{
> +    return inherited->color;
> +}
> +
> +const Color& RenderStyle::visitedLinkColor() const
> +{
> +    return inherited->visitedLinkColor;
> +}
> +
> +void RenderStyle::setColor(const Color& v)
> +{
> +    SET_VAR(inherited, color, v);
> +}
> +
> +void RenderStyle::setVisitedLinkColor(const Color& v)
> +{
> +    SET_VAR(inherited, visitedLinkColor, v);
> +}

Not sure why you unwrapped these.
Comment 4 Dean Jackson 2016-09-27 17:42:47 PDT
Comment on attachment 290021 [details]
Patch

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

>> Source/WebCore/platform/graphics/GraphicsContext.cpp:920
>> +    const Color& oldFillColor = fillColor();
> 
> Don't you actually need to make a copy here?

Yep. Good catch.

>> Source/WebCore/rendering/style/RenderStyle.cpp:1210
>> +}
> 
> Not sure why you unwrapped these.

Because it was ugly.
Comment 5 Build Bot 2016-09-27 17:52:43 PDT
Comment on attachment 290021 [details]
Patch

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

New failing tests:
fast/text/empty-shadow.html
Comment 6 Build Bot 2016-09-27 17:52:46 PDT
Created attachment 290035 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-09-27 17:54:29 PDT
Comment on attachment 290021 [details]
Patch

Attachment 290021 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2158020

New failing tests:
fast/text/empty-shadow.html
Comment 8 Build Bot 2016-09-27 17:54:32 PDT
Created attachment 290036 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-09-27 17:57:04 PDT
Comment on attachment 290021 [details]
Patch

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

New failing tests:
fast/text/empty-shadow.html
Comment 10 Build Bot 2016-09-27 17:57:06 PDT
Created attachment 290038 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Build Bot 2016-09-27 18:06:52 PDT
Comment on attachment 290021 [details]
Patch

Attachment 290021 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2158027

New failing tests:
fast/text/empty-shadow.html
Comment 12 Build Bot 2016-09-27 18:06:55 PDT
Created attachment 290041 [details]
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 13 Dean Jackson 2016-09-28 11:34:46 PDT
Created attachment 290104 [details]
Patch
Comment 14 Dean Jackson 2016-09-28 13:22:34 PDT
Committed r206538: <http://trac.webkit.org/changeset/206538>