Summary: | [chromium] Rename ChromiumBridge to PlatformBridge | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Fisher (:fishd, Google) <fishd> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | adamk, commit-queue, dimich, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | 52784 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Darin Fisher (:fishd, Google)
2011-01-14 13:03:09 PST
Created attachment 79354 [details]
Not yet tested
Created attachment 79367 [details]
Now with PlatformBridge.cpp
Comment on attachment 79367 [details]
Now with PlatformBridge.cpp
And then one more change to get rid of ChromiumBridge.h and put it in PlatformBridge.h?
That was supposed to be in this patch, actually; somehow, I seem to be missing everything under Source/WebCore. I suspect it's something to do with the fact that I had to switch from git to svn when generating this patch. Will try to get a fixed patch up soon. Created attachment 79453 [details]
WebCore + WebKit changes
Now with WebCore changes included in patch. "ChromiumBridge" no longer appears anywhere in the source tree. Attachment 79453 [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/WebCore/platform/chromium/SuddenTerminationChromium.cpp:34: One or more unexpected \r (^M) found; better to use only a \n [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Source/WebCore/platform/graphics/chromium/GlyphPageTreeNodeChromiumWin.cpp:83: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
Source/WebCore/platform/graphics/chromium/FontCacheChromiumWin.cpp:291: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:69: Code inside a namespace should not be indented. [whitespace/indent] [4]
Source/WebCore/platform/chromium/PlatformBridge.h:94: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:120: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:121: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:149: The parameter name "font" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:183: The parameter name "url" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:217: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:232: The parameter name "fireTime" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:342: The parameter name "id" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/chromium/PlatformBridge.h:343: The parameter name "id" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebCore/platform/graphics/chromium/FontPlatformDataChromiumWin.cpp:39: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 16 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
None of these style violations was added by me; what's the usual policy for such things? My instinct would be to avoid making unrelated changes in this patch. (In reply to comment #8) > None of these style violations was added by me; what's the usual policy for such things? My instinct would be to avoid making unrelated changes in this patch. The issue here is that the style rules have changed over time. Instead of fixing all of the code when the style rules change, we have the convention of fixing up files as they are modified. So, you may go ahead and do that. Created attachment 79505 [details]
Fixed style errors
Comment on attachment 79505 [details] Fixed style errors Clearing flags on attachment: 79505 Committed r76203: <http://trac.webkit.org/changeset/76203> All reviewed patches have been landed. Closing bug. This change broke compile on Chromium bots. Reverting. Sample: http://build.chromium.org/p/chromium/builders/Webkit%20Win%20Builder/builds/5657 Bummer, it looks like the change made to fix the style issue in Source/WebCore/platform/graphics/chromium/FontPlatformDataChromiumWin.cpp is to blame :-( Created attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build
Attachment 79615 [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/WebCore/platform/graphics/chromium/FontPlatformDataChromiumWin.cpp:39: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 55 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build
Looks sane.
Comment on attachment 79615 [details] Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build Clearing flags on attachment: 79615 Committed r76340: <http://trac.webkit.org/changeset/76340> All reviewed patches have been landed. Closing bug. |