Bug 79539 - [chromium] WebKit::setColorNames is a client API
Summary: [chromium] WebKit::setColorNames is a client API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on: 79544
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-24 15:43 PST by James Robinson
Modified: 2012-02-24 17:57 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.66 KB, patch)
2012-02-24 15:44 PST, James Robinson
no flags Details | Formatted Diff | Diff
Patch (20.15 KB, patch)
2012-02-24 15:50 PST, James Robinson
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-02-24 15:43:54 PST
[chromium] WebKit::setColorNames is a client API
Comment 1 James Robinson 2012-02-24 15:44:22 PST
Created attachment 128819 [details]
Patch
Comment 2 James Robinson 2012-02-24 15:45:54 PST
I moved WebColor into Platform/ since it's needed for some compositor APIs, but upon reflection WebKit::setNamedColors is really logically a client API.  This moves setNamedColors and the WebColorName enums back into the client space.  I can update the chromium headers as a follow-up and then remove the platform/WebColorName.h forwarding header.  I think WebColor as a typedef for unsigned should remain in both APIs.
Comment 3 James Robinson 2012-02-24 15:48:05 PST
Actually, I should probably move the setNamedColors() impl from WebColor.cpp to WebColorName.cpp if I do this.  Will update patch...
Comment 4 James Robinson 2012-02-24 15:50:21 PST
Created attachment 128820 [details]
Patch
Comment 5 James Robinson 2012-02-24 15:58:38 PST
Committed r108860: <http://trac.webkit.org/changeset/108860>
Comment 6 Adrienne Walker 2012-02-24 16:23:49 PST
Rolled out in r108865. From http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests/builds/18924/steps/compile/logs/stdio:

--SNIP--
  CXX(target) out/Release/obj.target/content_common/content/common/swapped_out_messages.o
In file included from ./third_party/WebKit/Source/WebKit/chromium/public/platform/WebColorName.h:29,
                 from ./content/common/css_colors.h:13,
                 from ./content/common/view_messages.h:11,
                 from content/browser/resolve_proxy_msg_helper.cc:10:
./third_party/WebKit/Source/WebKit/chromium/public/platform/../WebColorName.h:34:29:error: public/WebColor.h: No such file or directory
./third_party/WebKit/Source/WebKit/chromium/public/platform/../WebColorName.h:35:30:error: public/WebCommon.h: No such file or directory
--SNIP--
Comment 7 James Robinson 2012-02-24 16:27:30 PST
Code on the chromium side that #inclues WebColorName.h depends on the WebKit client API but not the Platform API, so it doesn't have the Platform API's include paths.  I think the fix is to use full relative include paths like "../../../Platform/chromium/public/WebColor.h" in WebKit/chromium/public/WebColorName.h or to jigger the dependencies around so that every target that depends on the WebKit client API also has the Platform API's include paths.
Comment 8 James Robinson 2012-02-24 17:57:13 PST
Committed r108877: <http://trac.webkit.org/changeset/108877>