Bug 52471 – [chromium] Rename ChromiumBridge to PlatformBridge

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Fisher (:fishd, Google)]

[chromium] Rename ChromiumBridge to PlatformBridge

WebCore/platform/chromium/ChromiumBridge.h should be renamed PlatformBridge.  There is already a typedef in platform/chromium/PlatformBridge.h.

[Adam Klein]

Created attachment 79354 [details]
Not yet tested

[Adam Klein]

Created attachment 79367 [details]
Now with PlatformBridge.cpp

[David Levin]

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?

[Adam Klein]

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.

[Adam Klein]

Created attachment 79453 [details]
WebCore + WebKit changes

[Adam Klein]

Now with WebCore changes included in patch.  "ChromiumBridge" no longer appears anywhere in the source tree.

[WebKit Review Bot]

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.

[Adam Klein]

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.

[Darin Fisher (:fishd, Google)]

(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.

[Adam Klein]

Created attachment 79505 [details]
Fixed style errors

[WebKit Commit Bot]

Comment on attachment 79505 [details]
Fixed style errors

Clearing flags on attachment: 79505

Committed r76203: <http://trac.webkit.org/changeset/76203>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dmitry Titov]

This change broke compile on Chromium bots. Reverting.

Sample: http://build.chromium.org/p/chromium/builders/Webkit%20Win%20Builder/builds/5657

[Darin Fisher (:fishd, Google)]

Bummer, it looks like the change made to fix the style issue in Source/WebCore/platform/graphics/chromium/FontPlatformDataChromiumWin.cpp is to blame :-(

[Adam Klein]

Created attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build

[WebKit Review Bot]

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.

[Eric Seidel (no email)]

Comment on attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build

Looks sane.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.