Mojo: Support enums as map keys in WTF

Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

Also fix hashing for StructPtr, which was broken during a clean-up.

BUG=696858
Review-Url: https://codereview.chromium.org/2715153004
Cr-Commit-Position: refs/heads/master@{#454805}
Committed: https://chromium.googlesource.com/chromium/src/+/3bb4fdb452973721bd26d6adbcf00c9adacede4b

[tibell, 2017-02-28 02:47:38]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 02:48:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 02:52:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 02:52:37]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tibell, 2017-02-28 02:58:03]

tibell@chromium.org changed reviewers:
+ dcheng@chromium.org, yzshen@chromium.org

[tibell, 2017-02-28 02:58:04]

Since WTF::HashMap requires tombstone values we end up having to pick some,
which feels unclean. I did consider instead dynamically picking an unused value
on a per-enum basis, but this doesn't play well with versioning and extensible
enums. At least there's a static check to make sure that if this comes and bites
us we'll know.

[tibell, 2017-02-28 03:01:49]

Description was changed from

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

BUG=696858
==========

to

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

BUG=696858
==========

[tibell, 2017-02-28 03:01:49]

tibell@chromium.org changed reviewers:
- dcheng@chromium.org

[tibell, 2017-02-28 03:54:29]

Merge

[tibell, 2017-02-28 03:57:17]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 03:57:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tibell, 2017-02-28 04:06:46]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 04:06:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 04:35:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 04:35:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tibell, 2017-02-28 06:10:24]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 06:11:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 07:41:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 07:41:33]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-02-28 18:48:35]

Can the following TODO removed?

https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/blink_binding...

https://codereview.chromium.org/2715153004/diff/80001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl (right):

https://codereview.chromium.org/2715153004/diff/80001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl:93: {%-   for
enum in enums %}
you could use "all_enums" for all enums at top level / in structs / in
interfaces.

For example:
https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/generators/cp...

[tibell, 2017-03-01 23:08:14]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[tibell, 2017-03-01 23:08:40]

PTAL

Also removed the TODO/blacklist.

https://codereview.chromium.org/2715153004/diff/80001/mojo/public/tools/bindi...
File mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl (right):

https://codereview.chromium.org/2715153004/diff/80001/mojo/public/tools/bindi...
mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl:93: {%-   for
enum in enums %}
On 2017/02/28 18:48:34, yzshen1 wrote:
> you could use "all_enums" for all enums at top level / in structs / in
> interfaces.
> 
> For example:
>
https://cs.chromium.org/chromium/src/mojo/public/tools/bindings/generators/cp...

Done.

[commit-bot: I haz the power, 2017-03-01 23:08:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yzshen1, 2017-03-01 23:13:27]

LGTM 

Thanks!

[commit-bot: I haz the power, 2017-03-01 23:37:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 23:37:36]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[tibell, 2017-03-02 00:51:54]

Re-add broken tests

[tibell, 2017-03-02 00:51:57]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 00:52:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 01:06:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 01:06:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tibell, 2017-03-02 01:40:54]

Merge

[tibell, 2017-03-02 01:40:56]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 01:41:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tibell, 2017-03-02 01:46:03]

Fix merge mistake

[tibell, 2017-03-02 01:46:07]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 01:46:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tibell, 2017-03-02 01:47:08]

PTAL

In removing the blacklist I found that using StructPtrs as keys had been broken
and the tests for that functionality had been removed. I've re-added the
important parts of those tests (the two added files) and fixed up the StructPtr
hashing code.

[tibell, 2017-03-02 01:47:30]

Description was changed from

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

BUG=696858
==========

to

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

Also fix hashing for StructPtr, which was broken during a clean-up.

BUG=696858
==========

[commit-bot: I haz the power, 2017-03-02 05:41:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 05:41:18]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yzshen1, 2017-03-02 17:25:03]

LGTM++

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/tests/map_unittest.cc (right):

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/tests/map_unittest.cc:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
2017?

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/tests/wtf_map_unittest.cc (right):

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/tests/wtf_map_unittest.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
2017?

[tibell, 2017-03-02 23:16:50]

The one test failure looks like a flake. Will submit.

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/tests/map_unittest.cc (right):

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/tests/map_unittest.cc:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
On 2017/03/02 17:25:03, yzshen1 wrote:
> 2017?

Done.

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/tests/wtf_map_unittest.cc (right):

https://codereview.chromium.org/2715153004/diff/160001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/tests/wtf_map_unittest.cc:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2017/03/02 17:25:03, yzshen1 wrote:
> 2017?

Done.

[tibell, 2017-03-02 23:17:05]

The CQ bit was checked by tibell@chromium.org

[tibell, 2017-03-02 23:17:06]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2715153004/#ps180001
(title: "Address review comments")

[commit-bot: I haz the power, 2017-03-02 23:17:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 01:22:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 01:22:02]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tibell, 2017-03-05 23:11:41]

The CQ bit was checked by tibell@chromium.org

[commit-bot: I haz the power, 2017-03-05 23:11:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 00:49:43]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1488755501379260,
"parent_rev": "4a1788dd92171c1f9384a3ebd6b751e1aa239926", "commit_rev":
"3bb4fdb452973721bd26d6adbcf00c9adacede4b"}

[commit-bot: I haz the power, 2017-03-06 00:50:19]

Description was changed from

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

Also fix hashing for StructPtr, which was broken during a clean-up.

BUG=696858
==========

to

==========
Mojo: Support enums as map keys in WTF

Since WTF::HashMap requires two special tombstone values ("empty" and "deleted")
we pick two values unlikely to be used in an enum and add a static check if they
ever are used.

Also fix hashing for StructPtr, which was broken during a clean-up.

BUG=696858

Review-Url: https://codereview.chromium.org/2715153004
Cr-Commit-Position: refs/heads/master@{#454805}
Committed:
https://chromium.googlesource.com/chromium/src/+/3bb4fdb452973721bd26d6adbcf0...
==========

[commit-bot: I haz the power, 2017-03-06 00:50:20]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/3bb4fdb452973721bd26d6adbcf0...