Also, we no longer rely on the swarming api contract since we use a custom merge step.  Depending on how you implement your solution we might also be able to remove the perf specific swarming contract here: 

https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/swarming/api.py?l=1086

How urgent is this?

Folks working on this is trying to wrap this up before they transfer team in a few weeks. So this is P1 in my opinion

Could you give some more details on this? I'm totally lost.

Started with a code search, but I don't see a usage example.
https://cs.chromium.org/search/?q=upload_results_to_perf_dashboard

This bug has an owner, thus, it's been triaged. Changing status to "assigned".

I can try to take a look later this week.

Oh, forgot about this. Ok, I will be doing this as part of the client.webrtc.perf LUCI migration...

Actually, Oleh will do it.

I still have this in my queue but it's really tough to untangle this.
Firstly, I'm not convinced at all that src-side upload is beneficial. Assuming that we do the upload src-side, we probably wouldn't want to directly depend on scripts in chromium/src anyway, because they could break us at any time. So the only option is to have WebRTC's own copy of the script. And since I prefer to not have it in webrtc/src either, there's no advantage in using a WebRTC-specific upload script in build/scripts/slave if there is already such a script (though you're calling it deprecated).

Yes the scripts in build/scripts/slave are deprecated so we won't be providing any maintenance to that code and code duplication is always a code smell.  I think it is more likely that that code will break than the src-side script since we are actively using that for the perf waterfall. 

The scripts are very similar, what is your hesitation besides breakage in using the src side script?

+Brian did a similar migration recently

Oleh, we need to figure out a way to be on the same page here so we don't have so much duplicate code and complexity.

Why is it worse to depend on chromium/src scripts versus depending on the scripts in build/scripts/slave? Either of them could be changed at any time, so breakage is always a concern. But at least the script https://chromium.googlesource.com/chromium/src/+/4a0d69517948a67c62dbae2ae407c6b713b7b419/tools/perf/core/upload_results_to_perf_dashboard.py is used by multiple teams, so if it breaks then everyone will prioritize fixing it (and if anything breaks you then you should feel free to revert it first and ask questions later)

There is no way to test how changes to it might affect WebRTC. The location is less obvious to look at when something goes wrong.
And I *can't* revert any such changes myself. We have 1 Chromium committer on the team, and even that is lucky.

And in general, it's part of infrastructure, not a Web browser.
Glad to share source code in infrastructure repositories.

Thanks for the detailed explanation, Oleh!

I think we need to think about this more carefully. I wonder if someone could explain to me why we needed to do the move in the first place?

Monorepos like Chromium do have some advantages: https://danluu.com/monorepo/

For the current build/scripts/slaves scripts, how does having the code there allow testing how changes might affect WebRTC?

If we really do need to have the code be in Chromium src repo, I am happy to do what I can to help make you a committer.

> For the current build/scripts/slaves scripts, how does having the code there allow testing how changes might affect WebRTC?

One can edit code in the repo locally, package it and run a LUCI build, using `led`.

You can do the same if it's src-side using "led edit-cr-cl" and pointing it to the src-side CL you want to test.

Even though WebRTC doesn't depend on the actual Chromium repo, that might actually work, because I added a workaround to run WebRTC's tryjobs on Chromium CLs. I didn't consider that this would also likely work with `edit-cr-cl` same as with `git cl try`.

I thought that WebRTC might get reviewed through chromium-review (some other third party dependencies work that way), but it looks like WebRTC has its own webrtc-review.

Looking at the code for the edit-cr-cl command, it might work with non-Chromium patches https://cs.chromium.org/chromium/infra/go/src/infra/tools/led/cmd_edit_cr_cl.go?q=edit-cr-cl&sq=package:chromium&dr=C&l=19. If it doesn't, you might also be able to just specify the patch_gerrit_url, patch_issue, and patch_set properties with "led edit -p"?

Changing the issue title to match the way I'm choosing to handle this.
https://chromium-review.googlesource.com/c/chromium/tools/build/+/1400123
I assume that was the main goal behind this.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/099af550dc86ec9f61123dccc0c307b0a7370e5c

commit 099af550dc86ec9f61123dccc0c307b0a7370e5c
Author: Oleh Prypin <oprypin@webrtc.org>
Date: Thu Jan 10 09:24:59 2019

WebRTC: Replace upload_perf_dashboard_results.py with a small custom script

(it preserves the same behavior of calling /add_point with Dashboard JSON v1)

TBR: sergiyb
Bug: 862134, 908001
Change-Id: I64e6791ed9bda04dcaab47bc6fc2cae3f2208066
Reviewed-on: https://chromium-review.googlesource.com/c/1400123
Reviewed-by: Oleh Prypin <oprypin@chromium.org>
Commit-Queue: Oleh Prypin <oprypin@chromium.org>

[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__l_nexus6_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_mac_10_11.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_linux_trusty.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__n_nexus6_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__l_nexus5_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__l_nexus4_.json
[add] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipe_modules/webrtc/resources/upload_perf_dashboard_results.py
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_mac_10_11.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_linux_trusty.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/README.recipes.md
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android64__n_pixel_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipe_modules/webrtc/steps.py
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__k_nexus5_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_win7.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipe_modules/webrtc/api.py
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android64__l_nexus9_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/luci_webrtc_perf_perf_android32__l_nexus7_2_.json
[modify] https://crrev.com/099af550dc86ec9f61123dccc0c307b0a7370e5c/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_perf_win7.json

So WebRTC's usage of upload_perf_dashboard_results.py is removed,
but it should be noted that another waterfall
https://ci.chromium.org/p/chromium/g/chromium.webrtc
still relies on performance upload that's part of runtest.py.
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8924725043182741936/+/steps/browser_tests/0/stdout#L37027_0
https://cs.chromium.org/chromium/build/scripts/slave/results_dashboard.py?q=Sending.result.%2Bof.%2Bto.dashboard

So maybe not all of this machinery can be removed :(
But do you also consider this kind of usage a blocker towards issue 862133?

I tried to migrate those to src-side config but it doesn't "just work", instead I get really obscure problems
(which, to be fair, are not about performance upload but rather my mistakes in usage of "isolate", but still difficult to resolve)
led run: https://chromium-swarm.appspot.com/task?id=424baa70077c3310

Here's my work in progress:
https://chromium-review.googlesource.com/q/author:oprypin+message:888429+-is:abandoned

Oleh, I'm happy to wait until you migrate fully and then I will delete everything all at once.

Thanks for the work on this!