RESOLVED FIXED Bug 92820
[chromium mac] DumpRenderTree compile fails with warning/error in LayoutTestHelper.mm with 10.7sdk
https://bugs.webkit.org/show_bug.cgi?id=92820
Summary [chromium mac] DumpRenderTree compile fails with warning/error in LayoutTestH...
James Robinson
Reported 2012-07-31 18:39:23 PDT
Attempting to compile DumpRenderTree on any mac with Xcode 4.4 fails: ../../Tools/DumpRenderTree/chromium/LayoutTestHelper.mm:45:1: error: 'CMProfileLocation' is deprecated [-Werror,-Wdeprecated-declarations] CMProfileLocation initialColorProfileLocation; // The locType field is initialized to 0 which is the same as cmNoProfileBase. ^ ../../Tools/DumpRenderTree/chromium/LayoutTestHelper.mm:56:5: error: 'CMProfileRef' is deprecated [-Werror,-Wdeprecated-declarations] CMProfileRef profile = 0; ^ /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/ApplicationServices.framework/Frameworks/ColorSync.framework/Headers/ColorSyncDeprecated.h:1002:41: note: 'CMProfileRef' declared here typedef struct OpaqueCMProfileRef* CMProfileRef DEPRECATED_IN_MAC_OS_X_VERSION_10_6_AND_LATER; We probably shouldn't be using this if it's deprecated in 10.6+
Attachments
Patch (6.32 KB, patch)
2012-07-31 23:04 PDT, Nico Weber
no flags
Patch for landing (6.60 KB, patch)
2012-08-03 06:53 PDT, Nico Weber
no flags
Nico Weber
Comment 1 2012-07-31 19:11:15 PDT
This used to work fine at r123866
Nico Weber
Comment 2 2012-07-31 21:10:21 PDT
While biking home, it occurred to me that this was likely caused by http://crrev.com/149198 . And indeed, after adding mac_deployment_target=10.5 to my GYP_DEFINES, DumpRenderTree builds fine again. So I recommend that as workaround for now. The real fix is to get rid of these deprecated functions.
Nico Weber
Comment 3 2012-07-31 21:20:33 PDT
See bug 80571 for what the apple port does. Apparently the color management functions where deprecated in 10.6 but replacements don't exist until 10.7 :-/ But we could #ifdef on the sdk being 10.7 I suppose, since people building with the 10.7 SDK will have 10.7 and will run locally. Better than not building.
Nico Weber
Comment 4 2012-07-31 23:04:28 PDT
Nico Weber
Comment 5 2012-07-31 23:05:58 PDT
Note: I haven't tried the attached patch, I just checked that it compiles. It'd be great if someone could check that it produces a DRT that passes pixel tests.
James Robinson
Comment 6 2012-08-02 19:13:49 PDT
I've confirmed that with this patch I get a DRT and it produces sane looking pixels, but since we don't have cr-mac Mountain Lion baselines I can't tell if they are correct. On many tests the only difference appears to be text rasterization which I'd expect to be different so it looks good.
Nico Weber
Comment 7 2012-08-02 19:17:46 PDT
You can try LayoutTests/fast/scrolling/scrollbar-tickmarks-styled.html . It doesn't have text, but it does have colors, so it would check if the colorspace handling is right.
James Robinson
Comment 8 2012-08-02 19:30:48 PDT
$ run-webkit-tests --chromium --release fast/scrolling/scrollbar-tickmarks-styled.html wdiff is not installed; please install from MacPorts or elsewhere The test ran as expected.
Nico Weber
Comment 9 2012-08-02 21:03:33 PDT
r? then.
jochen
Comment 10 2012-08-03 00:06:30 PDT
Comment on attachment 155724 [details] Patch Since you copied the code from the mac drt, should you add Apple to the copyright line in the header? View in context: https://bugs.webkit.org/attachment.cgi?id=155724&action=review > Tools/DumpRenderTree/chromium/LayoutTestHelper.mm:44 > + MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7 this can all go on one line
Nico Weber
Comment 11 2012-08-03 06:53:42 PDT
Created attachment 156360 [details] Patch for landing
Nico Weber
Comment 12 2012-08-03 06:54:25 PDT
Both done, thanks.
WebKit Review Bot
Comment 13 2012-08-03 07:40:06 PDT
Comment on attachment 156360 [details] Patch for landing Clearing flags on attachment: 156360 Committed r124612: <http://trac.webkit.org/changeset/124612>
WebKit Review Bot
Comment 14 2012-08-03 07:40:10 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.