Bug 109700 – [chromium] fix TestRunner build with enable_webrtc=0

• Component: New Bugs
• Status: RESOLVED FIXED

[chromium] fix TestRunner build with enable_webrtc=0

Created attachment 188084 [details]
Patch

[Tommy Widenflycht]

Comment on attachment 188084 [details]
Patch

Nice!

[Tony Chang]

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.

[Adam Barth]

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.

[WebKit Review Bot]

Comment on attachment 188122 [details]
Patch for landing

Clearing flags on attachment: 188122

Committed r142790: <http://trac.webkit.org/changeset/142790>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.