Reland of Create classes_util API, change discover to return a list instead of a dict.

Reland of Create classes_util API, change discover to return a list instead of a dict. (patchset #1 id:1 of https://codereview.chromium.org/1263063003/)

Reason for revert:
The telemetry tests aren't broken. The perf_unittests are because they don't pick tests in a deterministic way. Reverting after a change to make them deterministic.

Original issue's description:
> Revert of Create classes_util API, change discover to return a list instead of a dict. (patchset #5 id:120001 of https://codereview.chromium.org/1244223002/)
>
> Reason for revert:
> This appears to break the XP telemetry tests. Details on the bug.
>
> Original issue's description:
> > Create classes_util API, change discover to return a list instead of a dict.
> >
> > BUG=498968
> > CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect
> >
> > Committed: https://crrev.com/e6cbec4747aa7fe3c96b1cdb89de21ae77b30ab0
> > Cr-Commit-Position: refs/heads/master@{#341129}
>
> TBR=dtu@chromium.org,bengr@chromium.org,aiolos@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=498968
>
> Committed: https://crrev.com/c85e3e1af7e170d5f8ec2b012df6e9337d2352bb
> Cr-Commit-Position: refs/heads/master@{#341218}

TBR=dtu@chromium.org,bengr@chromium.org,avi@chromium.org
BUG=498968
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect

Committed: https://crrev.com/7e6fefcf3b839256c97e2e7bc0bfc3c53af75c8d
Cr-Commit-Position: refs/heads/master@{#341646}

[aiolos (Not reviewing), 2015-07-30 23:34:15]

Created Reland of Create classes_util API, change discover to return a list
instead of a dict.

[Avi (use Gerrit), 2015-07-30 23:37:23]

On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> Created Reland of Create classes_util API, change discover to return a list
> instead of a dict.

What is the change to make the tests not fail?

[aiolos (Not reviewing), 2015-07-31 00:04:50]

On 2015/07/30 23:37:23, Avi wrote:
> On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > Created Reland of Create classes_util API, change discover to return a list
> > instead of a dict.
> 
> What is the change to make the tests not fail?

The error is most likely due to a combination of 2 things:

1) the test that is failing on this bot hasn't been running on it previously, so
it doesn't already have the file it needs locally.
2) cloud_storage is failing to download the file because something is broken.

I am wondering if the bot could be hitting this bug:
crbug.com/508721

My current plan is to add some debugging messages to the
binary_manager/support_dependencies before relanding this cl, which will help us
determine why we are failing to find the correct path.

One of us should create a bug for the failure itself, so that we can track
fixing the actual issue, which may not be cause by this cl so much as uncovered
by it. In said bug we should request a trooper, or someone else with access to
that bot, to run "which gsutil" on the bot to determine if it is indeed hitting
508721 ?

[Avi (use Gerrit), 2015-07-31 00:06:31]

On 2015/07/31 00:04:50, aiolos (slow review) wrote:
> On 2015/07/30 23:37:23, Avi wrote:
> > On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > > Created Reland of Create classes_util API, change discover to return a
list
> > > instead of a dict.
> > 
> > What is the change to make the tests not fail?
> 
> The error is most likely due to a combination of 2 things:
> 
> 1) the test that is failing on this bot hasn't been running on it previously,
so
> it doesn't already have the file it needs locally.
> 2) cloud_storage is failing to download the file because something is broken.
> 
> I am wondering if the bot could be hitting this bug:
> crbug.com/508721
> 
> My current plan is to add some debugging messages to the
> binary_manager/support_dependencies before relanding this cl, which will help
us
> determine why we are failing to find the correct path.
> 
> One of us should create a bug for the failure itself, so that we can track
> fixing the actual issue, which may not be cause by this cl so much as
uncovered
> by it. In said bug we should request a trooper, or someone else with access to
> that bot, to run "which gsutil" on the bot to determine if it is indeed
hitting
> 508721 ?

I'm only sheriff tomorrow, so anything I do will lose continuity. Since you're
working in this area, can you follow up to get the bot green?

[Avi (use Gerrit), 2015-07-31 00:13:45]

On 2015/07/31 00:06:31, Avi wrote:
> On 2015/07/31 00:04:50, aiolos (slow review) wrote:
> > On 2015/07/30 23:37:23, Avi wrote:
> > > On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > > > Created Reland of Create classes_util API, change discover to return a
> list
> > > > instead of a dict.
> > > 
> > > What is the change to make the tests not fail?
> > 
> > The error is most likely due to a combination of 2 things:
> > 
> > 1) the test that is failing on this bot hasn't been running on it
previously,
> so
> > it doesn't already have the file it needs locally.
> > 2) cloud_storage is failing to download the file because something is
broken.
> > 
> > I am wondering if the bot could be hitting this bug:
> > crbug.com/508721
> > 
> > My current plan is to add some debugging messages to the
> > binary_manager/support_dependencies before relanding this cl, which will
help
> us
> > determine why we are failing to find the correct path.
> > 
> > One of us should create a bug for the failure itself, so that we can track
> > fixing the actual issue, which may not be cause by this cl so much as
> uncovered
> > by it. In said bug we should request a trooper, or someone else with access
to
> > that bot, to run "which gsutil" on the bot to determine if it is indeed
> hitting
> > 508721 ?
> 
> I'm only sheriff tomorrow, so anything I do will lose continuity. Since you're
> working in this area, can you follow up to get the bot green?

Also, you have to keep the bots green so that everyone else can keep working. If
your CL causes the bots to go red, EVEN IF IT IS NOT YOUR CL'S FAULT you still
have the responsibility to track down the knock-on changes and fix them or get
them fixed. Please take all the steps you mention to get the bot green again.

[aiolos (Not reviewing), 2015-07-31 00:36:09]

On 2015/07/31 00:13:45, Avi wrote:
> On 2015/07/31 00:06:31, Avi wrote:
> > On 2015/07/31 00:04:50, aiolos (slow review) wrote:
> > > On 2015/07/30 23:37:23, Avi wrote:
> > > > On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > > > > Created Reland of Create classes_util API, change discover to return a
> > list
> > > > > instead of a dict.
> > > > 
> > > > What is the change to make the tests not fail?
> > > 
> > > The error is most likely due to a combination of 2 things:
> > > 
> > > 1) the test that is failing on this bot hasn't been running on it
> previously,
> > so
> > > it doesn't already have the file it needs locally.
> > > 2) cloud_storage is failing to download the file because something is
> broken.
> > > 
> > > I am wondering if the bot could be hitting this bug:
> > > crbug.com/508721
> > > 
> > > My current plan is to add some debugging messages to the
> > > binary_manager/support_dependencies before relanding this cl, which will
> help
> > us
> > > determine why we are failing to find the correct path.
> > > 
> > > One of us should create a bug for the failure itself, so that we can track
> > > fixing the actual issue, which may not be cause by this cl so much as
> > uncovered
> > > by it. In said bug we should request a trooper, or someone else with
access
> to
> > > that bot, to run "which gsutil" on the bot to determine if it is indeed
> > hitting
> > > 508721 ?
> > 
> > I'm only sheriff tomorrow, so anything I do will lose continuity. Since
you're
> > working in this area, can you follow up to get the bot green?
> 
> Also, you have to keep the bots green so that everyone else can keep working.
If
> your CL causes the bots to go red, EVEN IF IT IS NOT YOUR CL'S FAULT you still
> have the responsibility to track down the knock-on changes and fix them or get
> them fixed. Please take all the steps you mention to get the bot green again.

I know that. Notice that I haven't submitted this cl yet because the fix hasn't
happened. (Even though I could have just as easily disabled the test on the bot
instead to avoid fixing the error.)

But my being responsible for my cl not breaking the bots doesn't mean that there
shouldn't have been a separate bug filed for the breakage and then assigned to
me to fix, with or without the original revert. Or that this revert shouldn't
land to verify that the suspected problem has been fixed.

[Avi (use Gerrit), 2015-07-31 00:39:33]

On 2015/07/31 00:36:09, aiolos (slow review) wrote:
> On 2015/07/31 00:13:45, Avi wrote:
> > On 2015/07/31 00:06:31, Avi wrote:
> > > On 2015/07/31 00:04:50, aiolos (slow review) wrote:
> > > > On 2015/07/30 23:37:23, Avi wrote:
> > > > > On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > > > > > Created Reland of Create classes_util API, change discover to return
a
> > > list
> > > > > > instead of a dict.
> > > > > 
> > > > > What is the change to make the tests not fail?
> > > > 
> > > > The error is most likely due to a combination of 2 things:
> > > > 
> > > > 1) the test that is failing on this bot hasn't been running on it
> > previously,
> > > so
> > > > it doesn't already have the file it needs locally.
> > > > 2) cloud_storage is failing to download the file because something is
> > broken.
> > > > 
> > > > I am wondering if the bot could be hitting this bug:
> > > > crbug.com/508721
> > > > 
> > > > My current plan is to add some debugging messages to the
> > > > binary_manager/support_dependencies before relanding this cl, which will
> > help
> > > us
> > > > determine why we are failing to find the correct path.
> > > > 
> > > > One of us should create a bug for the failure itself, so that we can
track
> > > > fixing the actual issue, which may not be cause by this cl so much as
> > > uncovered
> > > > by it. In said bug we should request a trooper, or someone else with
> access
> > to
> > > > that bot, to run "which gsutil" on the bot to determine if it is indeed
> > > hitting
> > > > 508721 ?
> > > 
> > > I'm only sheriff tomorrow, so anything I do will lose continuity. Since
> you're
> > > working in this area, can you follow up to get the bot green?
> > 
> > Also, you have to keep the bots green so that everyone else can keep
working.
> If
> > your CL causes the bots to go red, EVEN IF IT IS NOT YOUR CL'S FAULT you
still
> > have the responsibility to track down the knock-on changes and fix them or
get
> > them fixed. Please take all the steps you mention to get the bot green
again.
> 
> I know that. Notice that I haven't submitted this cl yet because the fix
hasn't
> happened. (Even though I could have just as easily disabled the test on the
bot
> instead to avoid fixing the error.)
> 
> But my being responsible for my cl not breaking the bots doesn't mean that
there
> shouldn't have been a separate bug filed for the breakage and then assigned to
> me to fix, with or without the original revert. Or that this revert shouldn't
> land to verify that the suspected problem has been fixed.

Sorry about not filing a bug, but I'm still even waiting for the XP Tests (1)
tester to even cycle in my revert to see how it affected things. Ugh.

[aiolos (Not reviewing), 2015-07-31 00:47:10]

On 2015/07/31 00:39:33, Avi wrote:
> On 2015/07/31 00:36:09, aiolos (slow review) wrote:
> > On 2015/07/31 00:13:45, Avi wrote:
> > > On 2015/07/31 00:06:31, Avi wrote:
> > > > On 2015/07/31 00:04:50, aiolos (slow review) wrote:
> > > > > On 2015/07/30 23:37:23, Avi wrote:
> > > > > > On 2015/07/30 23:34:15, aiolos (slow review) wrote:
> > > > > > > Created Reland of Create classes_util API, change discover to
return
> a
> > > > list
> > > > > > > instead of a dict.
> > > > > > 
> > > > > > What is the change to make the tests not fail?
> > > > > 
> > > > > The error is most likely due to a combination of 2 things:
> > > > > 
> > > > > 1) the test that is failing on this bot hasn't been running on it
> > > previously,
> > > > so
> > > > > it doesn't already have the file it needs locally.
> > > > > 2) cloud_storage is failing to download the file because something is
> > > broken.
> > > > > 
> > > > > I am wondering if the bot could be hitting this bug:
> > > > > crbug.com/508721
> > > > > 
> > > > > My current plan is to add some debugging messages to the
> > > > > binary_manager/support_dependencies before relanding this cl, which
will
> > > help
> > > > us
> > > > > determine why we are failing to find the correct path.
> > > > > 
> > > > > One of us should create a bug for the failure itself, so that we can
> track
> > > > > fixing the actual issue, which may not be cause by this cl so much as
> > > > uncovered
> > > > > by it. In said bug we should request a trooper, or someone else with
> > access
> > > to
> > > > > that bot, to run "which gsutil" on the bot to determine if it is
indeed
> > > > hitting
> > > > > 508721 ?
> > > > 
> > > > I'm only sheriff tomorrow, so anything I do will lose continuity. Since
> > you're
> > > > working in this area, can you follow up to get the bot green?
> > > 
> > > Also, you have to keep the bots green so that everyone else can keep
> working.
> > If
> > > your CL causes the bots to go red, EVEN IF IT IS NOT YOUR CL'S FAULT you
> still
> > > have the responsibility to track down the knock-on changes and fix them or
> get
> > > them fixed. Please take all the steps you mention to get the bot green
> again.
> > 
> > I know that. Notice that I haven't submitted this cl yet because the fix
> hasn't
> > happened. (Even though I could have just as easily disabled the test on the
> bot
> > instead to avoid fixing the error.)
> > 
> > But my being responsible for my cl not breaking the bots doesn't mean that
> there
> > shouldn't have been a separate bug filed for the breakage and then assigned
to
> > me to fix, with or without the original revert. Or that this revert
shouldn't
> > land to verify that the suspected problem has been fixed.
> 
> Sorry about not filing a bug, but I'm still even waiting for the XP Tests (1)
> tester to even cycle in my revert to see how it affected things. Ugh.

Yeah, slow tests are annoying. I already know what the general issue is, and dtu
is working on an architecture fallback for the file that can't be found, which
is the most likely cause other than gsutil being the wrong version. Either way,
I'm pretty sure that the fact that my cl was causing a test that hadn't
previously been run on that slave to run is what is causing the failure,
regardless of whether my assumption on the specific cause is correct. You
shouldn't be too nervous about it not turning green.

[bengr, 2015-07-31 18:46:49]

chrome_proxy lgtm

[aiolos (Not reviewing), 2015-08-03 20:34:08]

https://codereview.chromium.org/1267123004/ has landed, and hopefully fixed the
XP support_binaries issue. Attempting to re-land this cl now.

[aiolos (Not reviewing), 2015-08-03 20:34:14]

The CQ bit was checked by aiolos@chromium.org

[commit-bot: I haz the power, 2015-08-03 20:34:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1262623007/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1262623007/1

[aiolos (Not reviewing), 2015-08-03 20:34:55]

The CQ bit was unchecked by aiolos@chromium.org

[aiolos (Not reviewing), 2015-08-03 20:35:24]

The CQ bit was checked by aiolos@chromium.org

[commit-bot: I haz the power, 2015-08-03 20:35:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1262623007/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1262623007/1

[commit-bot: I haz the power, 2015-08-03 20:36:26]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-08-03 20:37:03]

Patchset 1 (id:??) landed as
https://crrev.com/2c1531af216c026be0b4aed672dc89d264f95a72
Cr-Commit-Position: refs/heads/master@{#341603}

[aiolos (Not reviewing), 2015-08-03 21:39:56]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1272433002/ by aiolos@chromium.org.

The reason for reverting is: Landed even though the patch failed..

[aiolos (Not reviewing), 2015-08-03 21:55:47]

The CQ bit was checked by aiolos@chromium.org

[aiolos (Not reviewing), 2015-08-03 21:55:47]

The patchset sent to the CQ was uploaded after l-g-t-m from bengr@chromium.org
Link to the patchset: https://codereview.chromium.org/1262623007/#ps280001
(title: "Rebase")

[commit-bot: I haz the power, 2015-08-03 21:55:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1262623007/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1262623007/280001

[commit-bot: I haz the power, 2015-08-03 23:50:01]

Committed patchset #2 (id:280001)

[commit-bot: I haz the power, 2015-08-03 23:50:51]

Patchset 2 (id:??) landed as
https://crrev.com/7e6fefcf3b839256c97e2e7bc0bfc3c53af75c8d
Cr-Commit-Position: refs/heads/master@{#341646}

[jochen (gone - plz use gerrit), 2015-08-04 07:03:45]

A revert of this CL (patchset #2 id:280001) has been created in
https://codereview.chromium.org/1267243002/ by jochen@chromium.org.

The reason for reverting is: fails reliably on XP tests:
https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20(1)/builds/39....