Bug 64281

Summary: [skia] remove platform helpers for gradients and patterns
Product: WebKit Reporter: Mike Reed <reed>
Component: New BugsAssignee: Mike Reed <reed>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dongseong.hwang, eric, jamesr, kbr, senorblanco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ec2-cr-linux-02
none
Patch
none
Patch
none
Patch none

Description Mike Reed 2011-07-11 07:39:38 PDT
[skia] remove platform helpers for gradients and patterns
Comment 1 Mike Reed 2011-07-11 07:43:03 PDT
Created attachment 100298 [details]
Patch
Comment 2 WebKit Review Bot 2011-07-11 07:44:47 PDT
Attachment 100298 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Review Bot 2011-07-11 08:02:59 PDT
Comment on attachment 100298 [details]
Patch

Attachment 100298 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9013341

New failing tests:
fast/dom/Geolocation/no-page-cache.html
fast/dom/location-new-window-no-crash.html
fast/backgrounds/background-leakage.html
fast/dom/Window/window-early-properties.html
fast/box-shadow/scaled-box-shadow.html
fast/dom/Window/setting-properties-on-closed-window.html
fast/dom/Document/early-document-access.html
http/tests/appcache/crash-when-navigating-away-then-back.html
fast/backgrounds/repeat/negative-offset-repeat.html
http/tests/cache/history-only-cached-subresource-loads.html
fast/dom/Window/window-onFocus.html
http/tests/cache/history-only-cached-subresource-loads-max-age-https.html
fast/blockflow/japanese-lr-selection.html
fast/dom/DeviceOrientation/no-page-cache.html
editing/selection/4975120.html
fast/dom/Window/console-functions.html
editing/selection/caret-and-focus-ring.html
fast/dom/Window/new-window-opener.html
fast/dom/Geolocation/window-close-crash.html
fast/dom/wrapper-classes.html
fast/dom/DeviceMotion/no-page-cache.html
fast/blockflow/japanese-rl-selection.html
fast/dom/Window/Location/set-location-after-close.html
fast/dom/HTMLDocument/hasFocus.html
editing/selection/selection-background.html
Comment 4 WebKit Review Bot 2011-07-11 08:03:03 PDT
Created attachment 100301 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 5 Mike Reed 2011-07-11 08:09:11 PDT
Created attachment 100302 [details]
Patch
Comment 6 Mike Reed 2011-07-11 08:39:28 PDT
Created attachment 100305 [details]
Patch
Comment 7 Stephen White 2011-07-11 10:30:47 PDT
Comment on attachment 100305 [details]
Patch

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

Thanks for the cleanup.  r=me

> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:376
> +                                      SkColor color) const {

Nit:  Don't wrap at 80 cols.

> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:397
> +                m_state->m_fillColor);

Nit:  Don't wrap at 80 cols.

> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:406
> +                m_state->m_strokeColor);

Nit:  Don't wrap at 80 cols.

> Source/WebCore/platform/graphics/skia/PlatformContextSkia.h:86
> +    void setGraphicsContext(const GraphicsContext* gc) { m_gc = gc; }

This is a bit circular, but I guess it's ok.  We had avoided the need for a backpointer up until now, but I think the cleaner approach in the rest of your patch justifies a circular reference.
Comment 8 Mike Reed 2011-07-11 10:51:51 PDT
Created attachment 100328 [details]
Patch
Comment 9 Mike Reed 2011-07-11 10:52:55 PDT
no code change from last review, just fixing 80-columns
Comment 10 Stephen White 2011-07-11 10:59:01 PDT
Comment on attachment 100328 [details]
Patch

Looks good.
Comment 11 WebKit Review Bot 2011-07-11 11:39:24 PDT
Comment on attachment 100328 [details]
Patch

Clearing flags on attachment: 100328

Committed r90767: <http://trac.webkit.org/changeset/90767>
Comment 12 WebKit Review Bot 2011-07-11 11:39:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Dongseong Hwang 2011-07-11 18:22:43 PDT
*** Bug 63871 has been marked as a duplicate of this bug. ***