Bug 52471 - [chromium] Rename ChromiumBridge to PlatformBridge
Summary: [chromium] Rename ChromiumBridge to PlatformBridge
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 52784
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-14 13:03 PST by Darin Fisher (:fishd, Google)
Modified: 2011-01-21 03:31 PST (History)
4 users (show)

See Also:


Attachments
Not yet tested (130.02 KB, patch)
2011-01-18 16:46 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Now with PlatformBridge.cpp (113.14 KB, patch)
2011-01-18 17:36 PST, Adam Klein
no flags Details | Formatted Diff | Diff
WebCore + WebKit changes (223.41 KB, patch)
2011-01-19 11:06 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Fixed style errors (226.50 KB, patch)
2011-01-19 15:44 PST, Adam Klein
no flags Details | Formatted Diff | Diff
Reverted style fixes in FontPlatformDataChromiumWin.cpp to fix Chromium/Windows build (226.29 KB, patch)
2011-01-20 10:38 PST, Adam Klein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.