Bug 41398 - Unfork Chromium's FontPlatformDataMac.mm
Summary: Unfork Chromium's FontPlatformDataMac.mm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 41148 44845
Blocks: 44937
  Show dependency treegraph
 
Reported: 2010-06-30 03:04 PDT by Jeremy Moskovich
Modified: 2010-08-31 02:37 PDT (History)
10 users (show)

See Also:


Attachments
platform/graphics/mac -> platform/graphics/cocoa (26.79 KB, patch)
2010-08-24 06:00 PDT, Jeremy Moskovich
no flags Details | Formatted Diff | Diff
Patch (36.90 KB, patch)
2010-08-25 06:37 PDT, Jeremy Moskovich
no flags Details | Formatted Diff | Diff
Patch for landing (30.82 KB, patch)
2010-08-28 22:31 PDT, Jeremy Moskovich
no flags Details | Formatted Diff | Diff
Patch (32.94 KB, patch)
2010-08-29 22:36 PDT, Jeremy Moskovich
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Moskovich 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.
Comment 1 Jeremy Moskovich 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.
Comment 2 Eric Seidel (no email) 2010-08-24 06:20:54 PDT
Attachment 65262 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3705586
Comment 3 Jeremy Moskovich 2010-08-24 06:34:41 PDT
(in response to Mac EWS: builds fine for me locally, I'll double check before landing),
Comment 4 Eric Seidel (no email) 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
Comment 5 Dimitri Glazkov (Google) 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.
Comment 6 Jeremy Moskovich 2010-08-25 06:37:23 PDT
Created attachment 65411 [details]
Patch
Comment 7 Jeremy Moskovich 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.
Comment 8 Dimitri Glazkov (Google) 2010-08-25 06:58:59 PDT
Comment on attachment 65411 [details]
Patch

ok.
Comment 9 Jeremy Moskovich 2010-08-28 22:31:43 PDT
Created attachment 65847 [details]
Patch for landing
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2010-08-28 23:46:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Review Bot 2010-08-29 00:13:21 PDT
http://trac.webkit.org/changeset/66329 might have broken Qt Linux Release
Comment 13 Darin Adler 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.
Comment 14 James Robinson 2010-08-29 18:31:08 PDT
It most definitely did.  Reverted at r66342.
Comment 15 Jeremy Moskovich 2010-08-29 22:36:58 PDT
Created attachment 65878 [details]
Patch
Comment 16 Kent Tamura 2010-08-29 22:46:15 PDT
Comment on attachment 65878 [details]
Patch

Looks OK.
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2010-08-29 23:13:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 James Robinson 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?
Comment 20 Darin Adler 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.