Bug 109700 - [chromium] fix TestRunner build with enable_webrtc=0
Summary: [chromium] fix TestRunner build with enable_webrtc=0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: jochen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-13 08:19 PST by jochen
Modified: 2013-02-13 13:48 PST (History)
4 users (show)

See Also:

Attachments
Patch (7.11 KB, patch)
2013-02-13 08:20 PST, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing (7.09 KB, patch)
2013-02-13 10:58 PST, jochen 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description jochen 2013-02-13 08:19:22 PST 
[chromium] fix TestRunner build with enable_webrtc=0
Comment 1 jochen 2013-02-13 08:20:03 PST 
Created attachment 188084 [details]
Patch
Comment 2 Tommy Widenflycht 2013-02-13 08:56:46 PST 
Comment on attachment 188084 [details]
Patch

Nice!
Comment 3 Tony Chang 2013-02-13 10:37:03 PST 
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?
Comment 4 jochen 2013-02-13 10:58:59 PST 
Created attachment 188122 [details]
Patch for landing
Comment 5 jochen 2013-02-13 11:00:13 PST 
(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 6 Adam Barth 2013-02-13 12:01:26 PST 
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 7 WebKit Review Bot 2013-02-13 13:48:43 PST 
Comment on attachment 188122 [details]
Patch for landing

Clearing flags on attachment: 188122

Committed r142790: <http://trac.webkit.org/changeset/142790>
Comment 8 WebKit Review Bot 2013-02-13 13:48:47 PST 
All reviewed patches have been landed.  Closing bug.