Summary: | [chromium] fix TestRunner build with enable_webrtc=0 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | jochen | ||||||
Component: | New Bugs | Assignee: | jochen | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, alecflett, tommyw, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
jochen
2013-02-13 08:19:22 PST
Created attachment 188084 [details]
Patch
Comment on attachment 188084 [details]
Patch
Nice!
Comment on attachment 188084 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188084&action=review > Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCPeerConnectionHandler.cpp:33 > +#if defined(ENABLE_WEBRTC) Why is some of this defined(ENABLE_WEBRTC) and some ENABLE_WEBRTC? Should we #define ENABLE in config.h? Created attachment 188122 [details]
Patch for landing
(In reply to comment #3) > (From update of attachment 188084 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188084&action=review > > > Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCPeerConnectionHandler.cpp:33 > > +#if defined(ENABLE_WEBRTC) > > Why is some of this defined(ENABLE_WEBRTC) and some ENABLE_WEBRTC? > Should we #define ENABLE in config.h? ah, that was an omission. I decided not to use an ENABLE() macro, as TestRunner only sees the defines from base/common.gypi, and they have different names than the ones in features.gypi, so I concluded that this was confusing. Comment on attachment 188122 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=188122&action=review > Tools/DumpRenderTree/chromium/TestRunner/src/MockConstraints.cpp:33 > +#if ENABLE_WEBRTC nit: We usually have a blank line after these sorts of lines. Comment on attachment 188122 [details] Patch for landing Clearing flags on attachment: 188122 Committed r142790: <http://trac.webkit.org/changeset/142790> All reviewed patches have been landed. Closing bug. |