looks good
What kind of test can we add to catch regressions like this?

Not sure what's easiest, and I'm not really familiar with chrome testing. 

But either an end-to-end test which sets up screencast of a static window and monitors the used rate. Or a test that checks the propagation, i.e., sets the constraint, and checks that the right value is included by the time ReconfigureVideoEncoder is called.

I'd vote for the latter. Checking rates is most likely going to be very flaky. And harder to track down to a particular issue should something go wrong.

Nisse, Erik,
Can you update this bug with version this was introduced.

Any progress on this yesterday?

We should really get this fixed asap and merge.

There's one bug affecting M51, which I think was introduced in Harald's constraints-related refactoring (but I haven't figured out how (and if) screencast_min_bitrate was handled before that change). This is fixed in cl https://codereview.chromium.org/1931813002/.

Then there's a different webrtc bug affecting M50. Probably introduced by either my cl https://codereview.webrtc.org/1608793004/ or by pbos' refactorings at about the same time. 

Should be fixed by the time https://codereview.webrtc.org/1695663003/ was landed, but I guess we'd want a separate smaller fix (see attached patch) for merging into M50. I don't know the process.

Deleted: m50-webrtc-bugfix.patch 1.2 KB

m50-webrtc-bugfix.patch 1.2 KB Download

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

commit dd7121647559c1618171f4f11ffe910a14fc93f9
Author: nisse <nisse@chromium.org>
Date: Fri Apr 29 14:12:12 2016

Propagate googScreencastMinBitrate constraint to webrtc's RTCConfiguration.

BUG= 607503 

Review-Url: https://codereview.chromium.org/1931813002
Cr-Commit-Position: refs/heads/master@{#390642}

[modify] https://crrev.com/dd7121647559c1618171f4f11ffe910a14fc93f9/content/renderer/media/rtc_peer_connection_handler.cc

This needs to be merged into M51, as it can severely impact the user experience when screensharing.
The fix is in the latest canary (52.0.2722.0), I've verified with manual testing that it works.

Your change meets the bar and is auto-approved for M51 (branch: 2704)

I've been trying to merge this, but I'm having some trouble. It should be easy (about four line change), but it's not a trivial cherry pick, since there's been close-by refactoring work between the M51 cut and master.

I've checked out branch-heads/2704, but I can't build it, because the commits listed in the DEPS file don't include required cherry-picks of the dependencies. E.g, the DEPS lists 

'v8_revision': '167dc63b4c9a1d0f0fe1b19af93644ac9a561e83'

while it seems I need the revision 

  commit d0eef6fb77024deddb07660aab4f6d3d24cd4366 (HEAD, tag: 5.1.281.25, origin/5.1-lkgr, branch-heads/5.1)

Is there some magic script that checks out the tag "branch-heads/5.1" of each dependency, instead of using the revision in the DEPS file like gclient sync?

I'm a bit confused by the build process, and I'd need a little help to make progress.

You want to merge it into https://chromium.googlesource.com/external/webrtc/+/branch-heads/51 of the webrtc repository, not into chromium directly.

danilchap@ landed one recently and could help you out.

There are two bugs. Today I've been focusing on the one affecting M51, and that's a bug in the chromium code, not in webrtc.

The fix for M50 (if we try to get that merged too) is different, it needs to go into the appropriate webrtc branch.

Ah, sorry. I thought you were doing webrtc-only changes here.

Please try to merge your change to M51 branch 2704 asap as we're cutting M51 beta candidate tomorrow. Thank you.

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

commit 36902805bacef84397a24ae5e3509bbb49a60d20
Author: Per <perkj@chromium.org>
Date: Tue May 03 08:06:07 2016

Manual cherry-pick of 'Propagate googScreencastMinBitrate constraint to webrtc.'

TBR=hta@chromium.org
BUG= 607503 

Review URL: https://codereview.chromium.org/1941343002 .

Cr-Commit-Position: refs/branch-heads/2704@{#347}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/36902805bacef84397a24ae5e3509bbb49a60d20/content/renderer/media/rtc_peer_connection_handler.cc

Tested the same on mac 10.11 chrome version 51.0.2704.36 - googScreencastMinBitrate in chrome::/webrtc-internals stays around 400(left the casted window unchanged for 5 mins)

Please find the screenshot

Deleted: Screen Shot 2016-05-04 at 12.56.45 PM.png 1.4 MB

	Screen Shot 2016-05-04 at 12.56.45 PM.png 1.4 MB View Download

What about M50, should we merge a bugfix (which is different from the M51 bugfix) there too?

After offline discussion, decided to leave M50 as is.