Bug 109700

Summary: [chromium] fix TestRunner build with enable_webrtc=0
Product: WebKit Reporter: jochen
Component: New BugsAssignee: 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 Flags
Patch
none
Patch for landing none

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.