Adding JSEP PeerConnection glue.

This CL was committed and reverted due to mem leaks. Since I'm on leave perkj has taken over this CL to fix the issues and commit.
https://chromiumcodereview.appspot.com/10008077/

---

Adding JSEP PeerConnection glue.

This adds glue for JSEP PeerConnection. PeerConnectionHandler is split in two classes and a base class. The class name is kept for the old ROAP PeerConnection to be aligned with WebKit naming. ROAP is planned to be removed pretty soon, then the classes can be refactored.

See also main WebKit bug https://bugs.webkit.org/show_bug.cgi?id=80589
(In particular https://bugs.webkit.org/show_bug.cgi?id=82450)


TEST=content_unittests and manual webrtc test.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131949

[Henrik Grunell, 2012-03-15 12:14:56]

This can't land until all WebKit patches are in. Furthermore, it hasn't been
tested completely yet. So there will likely be some changes. Tommi, can you do a
first round of review mainly on the class splitting and high level stuff?

[tommi (sloooow) - chröme, 2012-03-15 12:31:52]

took a very quick look.  will take a closer look when things are ready for
submit.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/media_stream_center.cc:57: if
(!native_candidate->ToString(&sdp))
is this the same as oom? If so, then maybe just CHECK().
same below.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/media_stream_impl.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/media_stream_impl.cc:123: client,
strange indent. run lint?

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/peer_connection_handler_base.h (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:24: : public
webrtc::PeerConnectionObserver {
no need to wrap?

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:43: // media_stream_impl_
is a weak pointer, and is valid for the lifetime of this
nit: s/weak/raw
since we have WeakPtr<>

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:60:
scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
maybe a note on what we use this for.
also, is this a pointer to a "known" message loop such as the IO or UI loops?

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/peer_connection_handler_jsep.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_jsep.cc:250:
base::Unretained(this),
is there no chance that the PeerConnectionHandlerJsep object be deleted before
this or other messages with base::Unretained are processed?

[Henrik Grunell, 2012-03-23 12:50:45]

- Code review fixes.
- Bug fixes.
- Updates for new WebKit interface, mostly name changes.
- Moved multiple PeerConnection support CL here.
(https://chromiumcodereview.appspot.com/9543001/)
- Adding unit test.
- One or two merges.
- Maybe something else I forgot.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/media_stream_center.cc:57: if
(!native_candidate->ToString(&sdp))
On 2012/03/15 12:31:52, tommi wrote:
> is this the same as oom? If so, then maybe just CHECK().
> same below.

This could be some other error according to libjingle developers. So we need to
handle it.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/media_stream_impl.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/media_stream_impl.cc:123: client,
On 2012/03/15 12:31:52, tommi wrote:
> strange indent. run lint?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/peer_connection_handler_base.h (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:24: : public
webrtc::PeerConnectionObserver {
On 2012/03/15 12:31:52, tommi wrote:
> no need to wrap?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:43: // media_stream_impl_
is a weak pointer, and is valid for the lifetime of this
On 2012/03/15 12:31:52, tommi wrote:
> nit: s/weak/raw
> since we have WeakPtr<>

Done.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_base.h:60:
scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
On 2012/03/15 12:31:52, tommi wrote:
> maybe a note on what we use this for.
> also, is this a pointer to a "known" message loop such as the IO or UI loops?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
File content/renderer/media/peer_connection_handler_jsep.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/1/content/renderer/media/...
content/renderer/media/peer_connection_handler_jsep.cc:250:
base::Unretained(this),
On 2012/03/15 12:31:52, tommi wrote:
> is there no chance that the PeerConnectionHandlerJsep object be deleted before
> this or other messages with base::Unretained are processed?

Actually, we are guaranteed to be called on the correct thread. We will always
be called on native PeerConnection's signaling thread, which is the render
thread (ensured by MediaStreamImpl). So I've removed posting at all. I've
checked that blocking won't be an issue.

[tommi (sloooow) - chröme, 2012-03-26 12:34:45]

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/media_stream_impl.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:123: LOG(ERROR) << "Could not create
PeerConnection handler";
if this is an oom situation, we more commonly just do:

Foo* foo = new Foo();
CHECK(foo);

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:140: if (pc_handler) {
same here

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:155:
video_renderers_.erase(vr_it++);
ah, I see now that you're using the temporary from the postfix ++ operator to
pass the 'previous' element to erase.

but what about doing this instead:

vr_it = video_renderers_.erase(vr_it);

This is why erase() has a return value.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_media_stream_dependency_factory.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:128:
MockSessionDescription(std::string sdp)
const std::string&

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:136: virtual bool
AddCandidate(const IceCandidateInterface* candidate) {
OVERRIDE?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:160:
MockIceCandidate(std::string label, std::string sdp)
const std::string& for both arguments.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_peer_connection_impl.h (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_peer_connection_impl.h:72: const std::string
kDummyOffer;
static const char[] ?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_web_peer_connection_00_handler_client.h
(right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_web_peer_connection_00_handler_client.h:19:
MockWebPeerConnection00HandlerClient();
are there hard tabs in this file?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_base.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:64:
native_stream->AddTrack(static_cast<webrtc::AudioTrackInterface*>(track));
add a DCHECK() that the type is audio?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:94: // TODO(grunell): Set
id to something unique.
hmm... 6 space indent?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:106:
UTF8ToUTF16(video_track->label()),
fix indent

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_jsep.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:77: delete native_offer;
scoped_ptr?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:84: delete native_answer;
scoped_ptr?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:171: delete
native_candidate;
scoped_ptr?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:249: return;
just use break; here and below?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:330: delete
native_candidate;
scoped_ptr

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_jsep_unittest.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep_unittest.cc:145:
static_cast<size_t>(1));
1u isn't enough?

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep_unittest.cc:182:
static_cast<size_t>(0));
0u?

[scherkus (not reviewing), 2012-03-26 22:04:37]

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/media_stream_impl.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:123: LOG(ERROR) << "Could not create
PeerConnection handler";
On 2012/03/26 12:34:45, tommi wrote:
> if this is an oom situation, we more commonly just do:
> 
> Foo* foo = new Foo();
> CHECK(foo);

actually chrome's OOM handler will take care of the situation by crashing so
there's little reason to code for OOM situations (i.e., don't bother with a
CHECK())

[Henrik Grunell, 2012-03-27 07:22:05]

Code review fixes and also an update to the MediaStreamImpl unit test to add
creating JSEP PeerConnection. Note that it's disabled currently, but I have
enabled it and tested locally.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/media_stream_impl.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:123: LOG(ERROR) << "Could not create
PeerConnection handler";
On 2012/03/26 22:04:37, scherkus wrote:
> On 2012/03/26 12:34:45, tommi wrote:
> > if this is an oom situation, we more commonly just do:
> > 
> > Foo* foo = new Foo();
> > CHECK(foo);
> 
> actually chrome's OOM handler will take care of the situation by crashing so
> there's little reason to code for OOM situations (i.e., don't bother with a
> CHECK())

OK, I'll remove the if-else and logging, just put it in the list and return.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:140: if (pc_handler) {
On 2012/03/26 12:34:45, tommi wrote:
> same here

Same as above.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/media_stream_impl.cc:155:
video_renderers_.erase(vr_it++);
On 2012/03/26 12:34:45, tommi wrote:
> ah, I see now that you're using the temporary from the postfix ++ operator to
> pass the 'previous' element to erase.
> 
> but what about doing this instead:
> 
> vr_it = video_renderers_.erase(vr_it);
> 
> This is why erase() has a return value.

It's a map, it doesn't have an iterator return value. This is afaik the cleanest
way to do the thing here. Keep the ideas coming! :-)

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_media_stream_dependency_factory.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:128:
MockSessionDescription(std::string sdp)
On 2012/03/26 12:34:45, tommi wrote:
> const std::string&

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:136: virtual bool
AddCandidate(const IceCandidateInterface* candidate) {
On 2012/03/26 12:34:45, tommi wrote:
> OVERRIDE?

Yes. Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_media_stream_dependency_factory.cc:160:
MockIceCandidate(std::string label, std::string sdp)
On 2012/03/26 12:34:45, tommi wrote:
> const std::string& for both arguments.

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_peer_connection_impl.h (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_peer_connection_impl.h:72: const std::string
kDummyOffer;
On 2012/03/26 12:34:45, tommi wrote:
> static const char[] ?

Sure. Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/mock_web_peer_connection_00_handler_client.h
(right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/mock_web_peer_connection_00_handler_client.h:19:
MockWebPeerConnection00HandlerClient();
On 2012/03/26 12:34:45, tommi wrote:
> are there hard tabs in this file?

Nope.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_base.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:64:
native_stream->AddTrack(static_cast<webrtc::AudioTrackInterface*>(track));
On 2012/03/26 12:34:45, tommi wrote:
> add a DCHECK() that the type is audio?

Sounds good. Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:94: // TODO(grunell): Set
id to something unique.
On 2012/03/26 12:34:45, tommi wrote:
> hmm... 6 space indent?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:106:
UTF8ToUTF16(video_track->label()),
On 2012/03/26 12:34:45, tommi wrote:
> fix indent

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_jsep.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:77: delete native_offer;
On 2012/03/26 12:34:45, tommi wrote:
> scoped_ptr?

Agree. Done throughout the file.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:84: delete native_answer;
On 2012/03/26 12:34:45, tommi wrote:
> scoped_ptr?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:171: delete
native_candidate;
On 2012/03/26 12:34:45, tommi wrote:
> scoped_ptr?

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:249: return;
On 2012/03/26 12:34:45, tommi wrote:
> just use break; here and below?

Sounds good. Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep.cc:330: delete
native_candidate;
On 2012/03/26 12:34:45, tommi wrote:
> scoped_ptr

Done.

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
File content/renderer/media/peer_connection_handler_jsep_unittest.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep_unittest.cc:145:
static_cast<size_t>(1));
On 2012/03/26 12:34:45, tommi wrote:
> 1u isn't enough?

No. We have actually discussed this in a previous CL review. :-)

https://chromiumcodereview.appspot.com/9699069/diff/13001/content/renderer/me...
content/renderer/media/peer_connection_handler_jsep_unittest.cc:182:
static_cast<size_t>(0));
On 2012/03/26 12:34:45, tommi wrote:
> 0u?

Same here.

[Henrik Grunell, 2012-03-29 08:16:21]

Darin, please review the files in content/ and content/renderer/ (not
content/renderer/media/ unless you want to). Tommi has reviewed all files
already.

[darin (slow to review), 2012-04-04 18:16:43]

LGTM w/ nits picked

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
content/renderer/render_view_impl.cc:669: WebKit::WebPeerConnection00Handler*
nit: normally, we add a using declaration at the top of this file to avoid the
WebKit:: prefixing.  if you did that, then you would not need the line break
between return type and class name.

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
content/renderer/render_view_impl.cc:673: if
(!cmd_line->HasSwitch(switches::kEnableMediaStream))
nit: how about moving this command line check into EnsureMediaStreamImpl?  this
boiler plate code is now replicated 3 times!!

http://codereview.chromium.org/9699069/diff/17002/content/renderer/renderer_w...
File content/renderer/renderer_webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9699069/diff/17002/content/renderer/renderer_w...
content/renderer/renderer_webkitplatformsupport_impl.cc:723:
WebKit::WebPeerConnection00Handler*
add using declaration to the top of the file as was done for
WebPeerConnectionHandler.

[Henrik Grunell, 2012-04-10 20:20:25]

Fixed Darin's nits. Also rebased in separate patch.

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
content/renderer/render_view_impl.cc:669: WebKit::WebPeerConnection00Handler*
On 2012/04/04 18:16:44, darin wrote:
> nit: normally, we add a using declaration at the top of this file to avoid the
> WebKit:: prefixing.  if you did that, then you would not need the line break
> between return type and class name.

Done.

http://codereview.chromium.org/9699069/diff/17002/content/renderer/render_vie...
content/renderer/render_view_impl.cc:673: if
(!cmd_line->HasSwitch(switches::kEnableMediaStream))
On 2012/04/04 18:16:44, darin wrote:
> nit: how about moving this command line check into EnsureMediaStreamImpl? 
this
> boiler plate code is now replicated 3 times!!

Done.

http://codereview.chromium.org/9699069/diff/17002/content/renderer/renderer_w...
File content/renderer/renderer_webkitplatformsupport_impl.cc (right):

http://codereview.chromium.org/9699069/diff/17002/content/renderer/renderer_w...
content/renderer/renderer_webkitplatformsupport_impl.cc:723:
WebKit::WebPeerConnection00Handler*
On 2012/04/04 18:16:44, darin wrote:
> add using declaration to the top of the file as was done for
> WebPeerConnectionHandler.

Done.

[perkj_chrome, 2012-04-11 12:28:19]

https://chromiumcodereview.appspot.com/9699069/diff/26002/content/renderer/me...
File content/renderer/media/mock_peer_connection_impl.h (right):

https://chromiumcodereview.appspot.com/9699069/diff/26002/content/renderer/me...
content/renderer/media/mock_peer_connection_impl.h:28: local_streams() OVERRIDE;
where are local_streams() and remote_streams() implemented?

https://chromiumcodereview.appspot.com/9699069/diff/26002/content/renderer/me...
File content/renderer/media/peer_connection_handler_base.cc (right):

https://chromiumcodereview.appspot.com/9699069/diff/26002/content/renderer/me...
content/renderer/media/peer_connection_handler_base.cc:77: if (!native_streams)
should be !native_streams.get()

[tommi (sloooow) - chröme, 2012-04-11 15:07:43]

lgtm

http://codereview.chromium.org/9699069/diff/26002/content/renderer/media/mock...
File content/renderer/media/mock_peer_connection_impl.h (right):

http://codereview.chromium.org/9699069/diff/26002/content/renderer/media/mock...
content/renderer/media/mock_peer_connection_impl.h:28: local_streams() OVERRIDE;
On 2012/04/11 12:28:19, perkj wrote:
> where are local_streams() and remote_streams() implemented?

the implementations are in the cc file.  it just looks a bit weird due to the
very long return type :)

http://codereview.chromium.org/9699069/diff/26002/content/renderer/media/peer...
File content/renderer/media/peer_connection_handler_base.cc (right):

http://codereview.chromium.org/9699069/diff/26002/content/renderer/media/peer...
content/renderer/media/peer_connection_handler_base.cc:77: if (!native_streams)
On 2012/04/11 12:28:19, perkj wrote:
> should be !native_streams.get()

not for scoped_refptr.  this is the preferred way.

[commit-bot: I haz the power, 2012-04-11 15:11:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grunell@chromium.org/9699069/26002

[commit-bot: I haz the power, 2012-04-11 16:06:54]

Try job failure for 9699069-26002 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-04-11 18:54:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grunell@chromium.org/9699069/35007

[commit-bot: I haz the power, 2012-04-11 22:20:58]

Try job failure for 9699069-35007 (retry) on win_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-04-12 06:16:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grunell@chromium.org/9699069/35007

[commit-bot: I haz the power, 2012-04-12 07:37:07]

Change committed as 131949

[eugenis, 2012-04-12 10:14:29]

On 2012/04/12 07:37:07, I haz the power (commit-bot) wrote:
> Change committed as 131949

reverted: http://code.google.com/p/chromium/issues/detail?id=123130