RESOLVED FIXED 41398
Unfork Chromium's FontPlatformDataMac.mm
https://bugs.webkit.org/show_bug.cgi?id=41398
Summary Unfork Chromium's FontPlatformDataMac.mm
Jeremy Moskovich
Reported 2010-06-30 03:04:27 PDT
https://bugs.webkit.org/show_bug.cgi?id=41148 forks FontPlatformDataMac.mm for the Chromium port to enable cross-process font loading. This bug tracks unforking the file.
Attachments
platform/graphics/mac -> platform/graphics/cocoa (26.79 KB, patch)
2010-08-24 06:00 PDT, Jeremy Moskovich
no flags
Patch (36.90 KB, patch)
2010-08-25 06:37 PDT, Jeremy Moskovich
no flags
Patch for landing (30.82 KB, patch)
2010-08-28 22:31 PDT, Jeremy Moskovich
no flags
Patch (32.94 KB, patch)
2010-08-29 22:36 PDT, Jeremy Moskovich
no flags
Jeremy Moskovich
Comment 1 2010-08-24 06:00:18 PDT
Created attachment 65262 [details] platform/graphics/mac -> platform/graphics/cocoa As a first step, per Joseph's comment at https://lists.webkit.org/pipermail/webkit-dev/2010-July/013409.html I've attached a patch that moves FontPlatformData.h and FontPlatformDataMac.mm to platform/graphics/cocoa . The next step will be merging the actual code changes.
Eric Seidel (no email)
Comment 2 2010-08-24 06:20:54 PDT
Jeremy Moskovich
Comment 3 2010-08-24 06:34:41 PDT
(in response to Mac EWS: builds fine for me locally, I'll double check before landing),
Eric Seidel (no email)
Comment 4 2010-08-24 07:27:19 PDT
The EWS is saying: In file included from /Users/eseidel/Projects/MacEWS/WebCore/platform/graphics/FontFallbackList.h:25, from /Users/eseidel/Projects/MacEWS/WebCore/platform/graphics/Font.h:30, from /Users/eseidel/Projects/MacEWS/WebCore/rendering/style/RenderStyle.h:48, from /Users/eseidel/Projects/MacEWS/WebCore/css/CSSStyleSelector.h:28, from /Users/eseidel/Projects/MacEWS/WebCore/inspector/InspectorDOMAgent.cpp:41: /Users/eseidel/Projects/MacEWS/WebCore/platform/graphics/SimpleFontData.h:28:30: error: FontPlatformData.h: No such file or directory
Dimitri Glazkov (Google)
Comment 5 2010-08-24 08:20:00 PDT
Comment on attachment 65262 [details] platform/graphics/mac -> platform/graphics/cocoa something is wrong with the diff. It's showing FontPlatformData* as removed, not copied.
Jeremy Moskovich
Comment 6 2010-08-25 06:37:23 PDT
Jeremy Moskovich
Comment 7 2010-08-25 06:40:09 PDT
New patch uploaded with the help of the webkit-patch script, hopefully making for a better diff. In case it's not clear from the changelog, the only changes in this patch are: * Move 2 source files. * Update WebCore XCodeproj & Chromium .gypi file to point to the new files.
Dimitri Glazkov (Google)
Comment 8 2010-08-25 06:58:59 PDT
Comment on attachment 65411 [details] Patch ok.
Jeremy Moskovich
Comment 9 2010-08-28 22:31:43 PDT
Created attachment 65847 [details] Patch for landing
WebKit Commit Bot
Comment 10 2010-08-28 23:46:18 PDT
Comment on attachment 65847 [details] Patch for landing Clearing flags on attachment: 65847 Committed r66329: <http://trac.webkit.org/changeset/66329>
WebKit Commit Bot
Comment 11 2010-08-28 23:46:24 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 12 2010-08-29 00:13:21 PDT
http://trac.webkit.org/changeset/66329 might have broken Qt Linux Release
Darin Adler
Comment 13 2010-08-29 14:37:09 PDT
I think this broke the build. The failures here <http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/12791/steps/compile-webkit/logs/stdio> look related to this.
James Robinson
Comment 14 2010-08-29 18:31:08 PDT
It most definitely did. Reverted at r66342.
Jeremy Moskovich
Comment 15 2010-08-29 22:36:58 PDT
Kent Tamura
Comment 16 2010-08-29 22:46:15 PDT
Comment on attachment 65878 [details] Patch Looks OK.
WebKit Commit Bot
Comment 17 2010-08-29 23:13:06 PDT
Comment on attachment 65878 [details] Patch Clearing flags on attachment: 65878 Committed r66358: <http://trac.webkit.org/changeset/66358>
WebKit Commit Bot
Comment 18 2010-08-29 23:13:12 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 19 2010-08-29 23:57:00 PDT
We rely on header include path order to compile? That's incredibly fragile :(. Is there really no better way?
Darin Adler
Comment 20 2010-08-30 00:06:39 PDT
This header include path dependency was added during the Chromium merge. Before that we used to use differently named header files and #if's in the central header but some Google folks wanted to use header paths because it scales better and does not require a single header with #ifs for each platform.
Note You need to log in before you can comment on or make changes to this bug.