[Chromium][Mac] Remove loadFont from PlatformSupport
Created attachment 165184 [details] Patch
Attachment 165184 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
I have many words for you, WebKit Style Guidelines. None are appropriate in mixed company.
Are you using webkit-patch upload? It would warn you about these before uploading the patch? Or you can run check-webkit-style manually
Maybe it checks style before you author the change log?
Comment on attachment 165184 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165184&action=review > Source/WebCore/platform/chromium/PlatformSupport.h:94 > #if OS(DARWIN) > - static bool loadFont(NSFont* srcFont, CGFontRef*, uint32_t* fontID); > #elif OS(UNIX) Should we clean up this ifdef? > Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:126 > + WebKit::WebSandboxSupport* ss = WebKit::Platform::current()->sandboxSupport(); ss -> sandboxSupport (please use complete words in variable names) > Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:128 > + if (!ss) > + return 0; Should we add an ASSERT_NOT_REACHED here like the old code had?
Created attachment 165233 [details] Patch
Comment on attachment 165233 [details] Patch Style nits addressed. abarth's suggestions implemented.
Comment on attachment 165233 [details] Patch Rejecting attachment 165233 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: rejects to file Source/WebCore/platform/chromium/PlatformSupport.h.rej patching file Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm patching file Source/WebKit/chromium/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/chromium/src/PlatformSupport.cpp Hunk #1 succeeded at 218 with fuzz 2. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13950846
Created attachment 165242 [details] Patch
Comment on attachment 165242 [details] Patch Repatched to ToT.
Comment on attachment 165242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165242&action=review > Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:130 > + // This function should only be called in response to an error loading a > + // font due to being blocked by the sandbox. > + // This by definition shouldn't happen if there is no sandbox support. Looks like these should be indented one more space, right?
Comment on attachment 165242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165242&action=review > Source/WebKit/chromium/src/PlatformSupport.cpp:220 > #if OS(DARWIN) This ifdef doesn't seem to be needed anymore since none of the cases do anything. ;)
Created attachment 165461 [details] Patch
Comment on attachment 165461 [details] Patch Nits addressed.
Comment on attachment 165461 [details] Patch Clearing flags on attachment: 165461 Committed r129429: <http://trac.webkit.org/changeset/129429>
All reviewed patches have been landed. Closing bug.