Bug 52471

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 Flags
Not yet tested
none
Now with PlatformBridge.cpp
none
WebCore + WebKit changes
none
Fixed style errors
none
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build none

Description Darin Fisher (:fishd, Google) 2011-01-14 13:03:09 PST
[chromium] Rename ChromiumBridge to PlatformBridge

WebCore/platform/chromium/ChromiumBridge.h should be renamed PlatformBridge.  There is already a typedef in platform/chromium/PlatformBridge.h.
Comment 1 Adam Klein 2011-01-18 16:46:06 PST
Created attachment 79354 [details]
Not yet tested
Comment 2 Adam Klein 2011-01-18 17:36:54 PST
Created attachment 79367 [details]
Now with PlatformBridge.cpp
Comment 3 David Levin 2011-01-18 22:12:59 PST
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?
Comment 4 Adam Klein 2011-01-19 10:58:56 PST
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.
Comment 5 Adam Klein 2011-01-19 11:06:54 PST
Created attachment 79453 [details]
WebCore + WebKit changes
Comment 6 Adam Klein 2011-01-19 11:07:57 PST
Now with WebCore changes included in patch.  "ChromiumBridge" no longer appears anywhere in the source tree.
Comment 7 WebKit Review Bot 2011-01-19 11:08:40 PST
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.
Comment 8 Adam Klein 2011-01-19 11:51:29 PST
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.
Comment 9 Darin Fisher (:fishd, Google) 2011-01-19 15:20:19 PST
(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.
Comment 10 Adam Klein 2011-01-19 15:44:30 PST
Created attachment 79505 [details]
Fixed style errors
Comment 11 WebKit Commit Bot 2011-01-19 21:18:12 PST
Comment on attachment 79505 [details]
Fixed style errors

Clearing flags on attachment: 79505

Committed r76203: <http://trac.webkit.org/changeset/76203>
Comment 12 WebKit Commit Bot 2011-01-19 21:18:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Dmitry Titov 2011-01-19 22:02:36 PST
This change broke compile on Chromium bots. Reverting.

Sample: http://build.chromium.org/p/chromium/builders/Webkit%20Win%20Builder/builds/5657
Comment 14 Darin Fisher (:fishd, Google) 2011-01-19 22:38:58 PST
Bummer, it looks like the change made to fix the style issue in Source/WebCore/platform/graphics/chromium/FontPlatformDataChromiumWin.cpp is to blame :-(
Comment 15 Adam Klein 2011-01-20 10:38:11 PST
Created attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build
Comment 16 WebKit Review Bot 2011-01-20 10:41:51 PST
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 17 Eric Seidel (no email) 2011-01-21 03:16:43 PST
Comment on attachment 79615 [details]
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build

Looks sane.
Comment 18 WebKit Commit Bot 2011-01-21 03:31:24 PST
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>
Comment 19 WebKit Commit Bot 2011-01-21 03:31:30 PST
All reviewed patches have been landed.  Closing bug.