Bug 55745

Summary: [chromium] MAC: update expectations for CSS3 linear gradient tests
Product: WebKit Reporter: noel gordon <noel.gordon>
Component: CSSAssignee: noel gordon <noel.gordon>
Status: RESOLVED FIXED    
Severity: Normal CC: jamesr, mihaip, senorblanco, webkit.review.bot
Priority: P2 Keywords: LayoutTestFailure
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on: 56736    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch
none
repeating linear gradient test failure
none
Patch
none
Patch
none
Patch for landing none

Description noel gordon 2011-03-03 22:05:47 PST
After the CG changes on bug 28152, the chromium MAC port should pass these tests. However, the do not pass currently, not sure why.

fast/gradients/css3-color-stop-units.html
fast/gradients/css3-color-stops.html
fast/gradients/css3-linear-angle-gradients.html
fast/gradients/css3-radial-gradients.html
fast/gradients/css3-radial-gradients2.html
fast/gradients/css3-radial-gradients3.html
fast/gradients/css3-repeating-radial-gradients.html
Comment 1 James Robinson 2011-03-03 23:07:32 PST
fast/gradients/css3-color-stops.html looks like 32bit vs 64bit CoreGraphics differences (Chrome always compiles in 32 bit currently whereas Safari is a 64 bit app on Snow Leopard).
Comment 2 noel gordon 2011-03-04 00:36:45 PST
Maybe fast/gradients/css3-color-stop-units.html fast/gradients/css3-linear-angle-gradients.html could go in the same bucket (32bit vs 64bit).

The radial gradient tests are broken though.  Chrome-mac shows "circles", other platforms show "ellipsoids".  For example ...

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fgradients%2Fcss3-radial-gradients3.html&showExpectations=true&group=%40ToT%20-%20chromium.org
Comment 3 noel gordon 2011-03-14 22:45:48 PDT
Got a Snow Leopard box setup, and compiled chromium mac, and see the 32bit vs 64bit compile issue.

Gradient.h includes <config.h>
   which includes -> <wtf/Platform.h>
     which includes -> /Developer/SDKs/MacOSX10.5.sdk/usr/include/AvailabilityMacros.h
 
Snow Leopard is not defined in the 10.5 SDK AvailabilityMacros.h, so the (32-bit) chromium-mac compile
thinks it is BUILDING_ON_LEOPARD via <wtf/Platform.h> :o

The makes Gradient.h define:
  #define USE_CG_SHADING defined(BUILDING_ON_TIGER) || defined(BUILDING_ON_LEOPARD)

and makes the gradient paint code take the USE_CG_SHADING (wrong) path in
  http://trac.webkit.org/changeset/74915/trunk/WebCore/platform/graphics/cg/GradientCG.cpp

Circular gradients result, rather than ellipsoids. Evil bug.
Comment 4 noel gordon 2011-03-14 23:08:52 PDT
Created attachment 85779 [details]
Proposed patch

A workaround.  Rebaselines will come later if we go with this patch.
Comment 5 James Robinson 2011-03-15 00:14:43 PDT
But we do build on Leopard - does this patch work there?
Comment 7 Mihai Parparita 2011-03-15 12:52:44 PDT
(In reply to comment #6)
> Fair call, chrome try jobs sent ...
>   http://build.chromium.org/p/tryserver.chromium/builders/mac_layout_rel/builds/119
>   http://build.chromium.org/p/tryserver.chromium/builders/mac_layout/builds/463

I'm pretty sure that Mac try servers run on Snow Leopard. From the output:
    setenv DISTCC_COMPILER "gcc version 4.2.1 (Apple Inc. build 5659)"
    setenv DISTCC_HOSTS localhost
    setenv DISTCC_SYSTEM "10.6.6 (10J567, i386)"
Comment 8 noel gordon 2011-03-15 18:16:48 PDT
Thanks Mihai, I'm searching round for leopard machine now ...
Comment 9 noel gordon 2011-03-16 01:08:44 PDT
Found a 10.5.8 machine, built chromium dumpRenderTree, and the repeating linear gradient tests are well broken ...
Comment 10 noel gordon 2011-03-16 01:10:22 PDT
Created attachment 85919 [details]
repeating linear gradient test failure
Comment 11 noel gordon 2011-03-16 01:27:33 PDT
Best to fix this once there's a chrome 10.6 build/release.
Comment 12 noel gordon 2011-03-21 04:15:05 PDT
To salvage something from this bug, noted the following tests are passing on
dumpRenderTree --chromium on both OSX 10.5, OSX 10.6 chrome builds.
 
fast/gradients/css3-color-stop-units.html
fast/gradients/css3-color-stops.html
fast/gradients/css3-linear-angle-gradients.html

Change the bug title to match.
Comment 13 noel gordon 2011-03-21 04:20:25 PDT
Filed bug 56736 about fixing the radial gradient cases.
Comment 14 noel gordon 2011-03-21 04:31:50 PDT
Created attachment 86306 [details]
Patch
Comment 15 Mihai Parparita 2011-03-22 17:12:01 PDT
Comment on attachment 86306 [details]
Patch

I'm not seeing any new or updated images in the patch (and it's only 1.96K). Also note that we're currently passing these tests on Snow Leopard, so we want to keep using the mac/ baseline there. These baselines should go into chromium-mac-leopard (or better yet, mac-leopard if you can verify that the mac port would pass with them on Leopard).
Comment 16 Mihai Parparita 2011-03-22 17:14:06 PDT
(In reply to comment #12)
> To salvage something from this bug, noted the following tests are passing on
> dumpRenderTree --chromium on both OSX 10.5, OSX 10.6 chrome builds.

Oh, I see now, you're saying that these tests pass with the current baselines. However, I'm not seeing that on the dashboard:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fgradients%2Fcss3-color-stop-units.html%2Cfast%2Fgradients%2Fcss3-color-stops.html%2Cfast%2Fgradients%2Fcss3-linear-angle-gradients.html&group=%40ToT%20-%20chromium.org

Where did you see them pass on 10.5?
Comment 18 noel gordon 2011-12-22 00:01:57 PST
Oh, now we have skia on the mac, some rebaselines are in order ...
Comment 19 noel gordon 2011-12-22 00:05:37 PST
Created attachment 120283 [details]
Patch
Comment 20 noel gordon 2012-01-03 20:47:38 PST
Created attachment 121050 [details]
Patch for landing
Comment 21 Stephen White 2012-01-04 07:12:50 PST
Comment on attachment 121050 [details]
Patch for landing

OK.  r=me
Comment 22 WebKit Review Bot 2012-01-05 02:04:06 PST
Comment on attachment 121050 [details]
Patch for landing

Clearing flags on attachment: 121050

Committed r104128: <http://trac.webkit.org/changeset/104128>
Comment 23 WebKit Review Bot 2012-01-05 02:04:13 PST
All reviewed patches have been landed.  Closing bug.