../../chrome/browser/media/webrtc/webrtc_browsertest_base.cc(333): error: Value of: content::ExecuteScriptAndExtractString( tab_contents, javascript, &result)
  Actual: false
Expected: true
../../chrome/browser/media/webrtc/webrtc_browsertest_base.cc(671): error: Value of: result == "ok-sender-with-track-found" || result == "ok-sender-with-track-not-found"
  Actual: false
Expected: true
../../chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc(183): error: Value of: HasSenderWithTrack(left_tab_, audio_track_id)
  Actual: false
Expected: true

Sugguests that maybe the tests are racy and don't wait for js execution to complete?

It's that same message for all three tests.

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FCrWinClang_tester%2F15604%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FWebRtcRtpBrowserTest.AddAndRemoveTracksWithoutStream%2F0
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FCrWinClang_tester%2F15604%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FWebRtcRtpBrowserTest.AddAndRemoveTracksWithSharedStream%2F0
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FCrWinClang_tester%2F15604%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FWebRtcRtpBrowserTest.AddAndRemoveTracksWithIndividualStreams%2F0

These are failing on the pinned bots. Maybe we should revert?

But they seem to pass everywhere else, which kind of suggests this might be a compiler bug, no?

(But given the tests's history and the failure message, all the other bots also might just get lucky)

It could be a compiler problem, but I figured clang on Windows has graduated from FYI status to "must be green". If it hasn't already, it will soon.

I reproduced the crash, and I suspect this is use-after-move in ShallowCopy:

std::unique_ptr<RTCRtpSender> RTCRtpSender::ShallowCopy() const {
  std::vector<std::unique_ptr<WebRtcMediaStreamAdapterMap::AdapterRef>>
      stream_adapter_copies(stream_adapters_.size());
  for (size_t i = 0; i < stream_adapters_.size(); ++i) {
    stream_adapter_copies[i] = stream_adapters_[i]->Copy();
  }
  return base::MakeUnique<RTCRtpSender>(webrtc_rtp_sender_,
                                        track_adapter_->Copy(),
                                        std::move(stream_adapter_copies));
}

I haven't narrowed it down yet, but here goes.

Hm, it's not that. I fixed this null pointer dereference, though:

diff --git a/content/renderer/media/webrtc/rtc_rtp_sender.cc b/content/renderer/media/webrtc/rtc_rtp_sender.cc
index 21f5968..cae981c 100644
--- a/content/renderer/media/webrtc/rtc_rtp_sender.cc
+++ b/content/renderer/media/webrtc/rtc_rtp_sender.cc
@@ -60,8 +60,12 @@ std::unique_ptr<RTCRtpSender> RTCRtpSender::ShallowCopy() const {
   for (size_t i = 0; i < stream_adapters_.size(); ++i) {
     stream_adapter_copies[i] = stream_adapters_[i]->Copy();
   }
+  std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef> track_adapter;
+  if (track_adapter_) {
+    track_adapter = track_adapter_->Copy();
+  }
   return base::MakeUnique<RTCRtpSender>(webrtc_rtp_sender_,
-                                        track_adapter_->Copy(),
+                                        std::move(track_adapter),
                                         std::move(stream_adapter_copies));
 }


But that's not enough to fix the bug. I think with other compilers track_adapter_ isn't null at this point.

I declare defeat, this requires too much local knowledge to debug through the layers.

These logs might be useful:

...

[15832:10984:0720/112459.242:INFO:CONSOLE(13)] "Returning ok-received-candidates to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
[15832:10984:0720/112459.246:INFO:CONSOLE(13)] "Returning ok-gc to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
[15832:10984:0720/112459.247:INFO:CONSOLE(13)] "Returning ok-stream-with-track-not-found to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
[15832:10984:0720/112459.247:INFO:CONSOLE(13)] "Returning ok-stream-with-track-found to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
../../chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc(308): error: Value of: HasRemoteStreamWithTrack(right_tab_, video_stream_id, video_track_id)
  Actual: true
Expected: false
[15832:10984:0720/112459.248:INFO:CONSOLE(13)] "Returning ok-receiver-with-track-not-found to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
[15832:10984:0720/112459.249:INFO:CONSOLE(13)] "Returning ok-receiver-with-track-found to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
../../chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc(310): error: Value of: HasReceiverWithTrack(right_tab_, video_track_id)
  Actual: true
Expected: false
[15832:10984:0720/112459.251:INFO:CONSOLE(13)] "Returning Test failed: Error: getReceivers().length != expectedNumTracks
    at failTest (http://127.0.0.1:55627/webrtc/test_functions.js:46:15)
    at verifyRtpReceivers (http://127.0.0.1:55627/webrtc/peerconnection_rtp.js:76:11)
    at <anonymous>:1:1 to test.", source: http://127.0.0.1:55627/webrtc/test_functions.js (13)
../../chrome/browser/media/webrtc/webrtc_browsertest_base.cc(598): error:       Expected: "ok-receivers-verified"
To be equal to: ExecuteJavascript(javascript, tab)
      Which is: "Test failed: Error: getReceivers().length != expectedNumTracks\n    at failTest (http://127.0.0.1:55627/webrtc/test_functions.js:46:15)\n    at verifyRtpReceivers (http://127.0.0.1:55627/webrtc/peerconnection_rtp.js:76:11)\n    at <anonymous>:1:1"
With diff:
@@ -1,1 +1,4 @@
-ok-receivers-verified
+Test failed: Error: getReceivers().length != expectedNumTracks
+    at failTest (http://127.0.0.1:55627/webrtc/test_functions.js:46:15)
+    at verifyRtpReceivers (http://127.0.0.1:55627/webrtc/peerconnection_rtp.js:76:11)
+    at <anonymous>:1:1

[15832:10984:0720/112459.252:INFO:CONSOLE(76)] "Uncaught Error: getReceivers().length != expectedNumTracks", source: http://127.0.0.1:55627/webrtc/peerconnection_rtp.js (76)
[15832:10984:0720/112459.287:INFO:user_input_monitor_win.cc(157)] RegisterRawInputDevices() failed for RIDEV_REMOVE: The parameter is incorrect. (0x57)
[  FAILED  ] WebRtcRtpBrowserTest.AddAndRemoveTracksWithIndividualStreams, where TypeParam =  and GetParam() =  (3264 ms)
[----------] 1 test from WebRtcRtpBrowserTest (3265 ms total)


So, things get off the rails late in the browser test.

Loaner windows laptop doesn't work. I'm getting my windows laptop re-imaged so that it can be used for development. I should look into getting a proper windows machine that doesn't take several hours to compile but people are on vacation :) Hopefully I can repro in a couple of days before my vacation.

I tried another loaner laptop, couldn't run script necessary to get the code due to permissions. I've got my windows laptop reimaged but it's blocked by security software from getting the code also. The stack traces don't make sense to me, I need to debug locally. I'll be OOO for the next two weeks, will revisit.

You might be able to patch in Hans's change to BUILDCONFIG.gn to test this on the win_chromium_rel_ng trybot: https://chromium-review.googlesource.com/c/588077/8/build/config/BUILDCONFIG.gn

I think there's something wrong in the video track removal logic, since that's where expectations start to fail.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a

commit 1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a
Author: Henrik Boström <hbos@chromium.org>
Date: Wed Aug 16 16:44:56 2017

RTCPeerConnectionHandler: Keep track of senders.

Re-upload of https://chromium-review.googlesource.com/c/567184

https://chromium-review.googlesource.com/c/566806 showed that the
WebRtcRtpBrowserTest.AddAndRemoveTracks* tests would reliably crash
if the garbage collector was invoked between addTrack and getSenders.
Tracks that were added using addTrack would have their track adapters
(glue between blink and webrtc) kept alive only by the blink layer
sender holding on to a reference, since these were not kept alive by
any "local streams" holding a reference.

The blink::RTCPeerConnection now holds a strong reference to senders
and receivers to prevent GC while in-use, with TODOs to remove ones
no longer used when addStream/removeStream is implemented using
addTrack/removeTrack.

Furthermore the content::RTCPeerConnectionHandler holds on to the
content layer representation of senders so that their associated set
of streams are not forgotten between addTrack and getSenders.

Having the handler keep track of senders is good practice, this gets
rid of the assumption that blink layer senders have to be kept alive.
A TODO was added to do the same for receivers (not yet required because
blink layer receivers are kept alive).

With this change, the AddAndRemoveTracks* tests pass and are
re-enabled.

TBR=deadbeef@chromium.org,jochen@chromium.org

Bug: 700916,  740650 ,  746971 
Change-Id: I8591b2596cc283c1f1eae6aa6dcfde63f704bd44
Reviewed-on: https://chromium-review.googlesource.com/581207
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494823}
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/rtc_rtp_sender.cc
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/rtc_rtp_sender.h
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.cc
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/1110d5d63a33b37b6636d5dd95d0dd2992cb7c4a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h

The win-clang problem had to do with the order of evaluation arguments of a function call. The first argument used a variable that was moved in the second argument, on all bots except win-clang this was ok, but on win-clang the move happened before the first argument was evaluated, resulting in inserting an element in the wrong location of an std::map.

The argument evaluation order is undefined in C++ so this was a real bug only evident on win-clang. With the re-land of the above CL, this was fixed and verified.

For reference, this was the code before the fix:

  auto it = rtp_senders_
                .insert(std::make_pair(
                    RTCRtpSender::getId(webrtc_sender),
                    base::MakeUnique<RTCRtpSender>(std::move(webrtc_sender),
                                                   std::move(track_adapter),
                                                   std::move(stream_adapters))))

It accesses moved-from scoped_refptr when evaluated RTL. It would be great if we had a warning for this: http://crbug.com/409318

I wasn't able to find this bug even when I was staring at the code looking for it, so clearly we need compiler help.