Improvements to uses of base::SmallMap

Improvements to uses of base::SmallMap

Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Convert some uses of SmallMap to flat_map where appropriate.

Add an IPC serialization rule for flat_map (required by one of the SmallMap->flat_map conversions)

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2825853002
Cr-Commit-Position: refs/heads/master@{#465911}
Committed: https://chromium.googlesource.com/chromium/src/+/ca2ec763405c18960ad1ee66e5ac964a00c257e5

[brettw, 2017-04-18 19:59:57]

Description was changed from

==========
Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Improve the small_map documentation slightly.
==========

to

==========
Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[brettw, 2017-04-18 19:59:58]

Description was changed from

==========
Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[brettw, 2017-04-18 20:00:10]

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

[commit-bot: I haz the power, 2017-04-18 20:00:36]

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

[brettw, 2017-04-18 20:27:05]

brettw@chromium.org changed reviewers:
+ danakj@chromium.org

[brettw, 2017-04-18 20:27:06]

I made the small_map comments slightly less wrong but they're still not great.
I'm working on a better summary to put in the readme.

[danakj, 2017-04-18 21:35:51]

I don't believe most of these should be small_map anyhow. I left some comments.
LGTM to rename ofc.

https://codereview.chromium.org/2825853002/diff/1/base/containers/README.md
File base/containers/README.md (right):

https://codereview.chromium.org/2825853002/diff/1/base/containers/README.md#n...
base/containers/README.md:17: For STL-like containers our policy is that they
should use STL-like naming even
Can you explain that for methods is only when their behaviour matches the STL
methods, if it deviates we should use CamelCase?

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h
File base/containers/small_map.h (right):

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h...
base/containers/small_map.h:59: //   base::small_map<std::map<MyKey, MyValue>,
4, std::equal_to<KyKey> >
Sidenote: I find it hard to believe it's a good idea to make a std::map for 5
things. Users of small_map have cargo-culted this constant though. And places
that didn't picked a larger number that is still way too small for a std::map to
be good.

Probably what this class should do is just use a flat_map until 100 and then use
a std::map or something..

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h...
base/containers/small_map.h:85: //   base::small_map< std::map<string, int> >
days;
you can remove the spaces around the <>

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
File cc/scheduler/begin_frame_source.cc (right):

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
cc/scheduler/begin_frame_source.cc:348:
base::small_map<std::map<BeginFrameObserver*, uint64_t>, 4>
do my eyes deceive me or is this shadowing the member variable with a local of
the same name that will be empty?

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
File cc/scheduler/begin_frame_source.h (right):

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
cc/scheduler/begin_frame_source.h:286:
base::small_map<std::map<BeginFrameObserver*, uint64_t>, 4>
This should really be a flat_map, there's no way this is getting so large that a
map is a good idea, and it rarely changes.

https://codereview.chromium.org/2825853002/diff/1/cc/tiles/picture_layer_tili...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2825853002/diff/1/cc/tiles/picture_layer_tili...
cc/tiles/picture_layer_tiling.cc:264:
base::small_map<std::unordered_map<TileMapKey, gfx::Rect, TileMapKeyHash>, 16>
This should be a flat_map too

https://codereview.chromium.org/2825853002/diff/1/cc/trees/layer_tree_host_im...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2825853002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl.cc:1164: base::small_map<std::unordered_map<int,
int>> pass_references;
This should be a flat_map

https://codereview.chromium.org/2825853002/diff/1/content/zygote/zygote_linux.h
File content/zygote/zygote_linux.h (right):

https://codereview.chromium.org/2825853002/diff/1/content/zygote/zygote_linux...
content/zygote/zygote_linux.h:54: typedef
base::small_map<std::map<base::ProcessHandle, ZygoteProcessInfo>>
change "typedef" to "using" while you're here?

https://codereview.chromium.org/2825853002/diff/1/gpu/perftests/texture_uploa...
File gpu/perftests/texture_upload_perftest.cc (right):

https://codereview.chromium.org/2825853002/diff/1/gpu/perftests/texture_uploa...
gpu/perftests/texture_upload_perftest.cc:404:
base::small_map<std::map<std::string, Measurement>>
Seems overkill to use small_map in test code like this, it looks like flat_map
would work fine here.

https://codereview.chromium.org/2825853002/diff/1/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/1/ipc/ipc_message_utils.h#new...
ipc/ipc_message_utils.h:903: typedef base::small_map<NormalMap, kArraySize,
EqualKey, MapInit> param_type;
typedef->using?

https://codereview.chromium.org/2825853002/diff/1/storage/browser/blob/blob_m...
File storage/browser/blob/blob_memory_controller.cc (right):

https://codereview.chromium.org/2825853002/diff/1/storage/browser/blob/blob_m...
storage/browser/blob/blob_memory_controller.cc:248:
base::small_map<std::map<uint64_t, uint64_t>> file_id_to_sizes;
Aside.. but this is interesting. What it really wants is something like

vector<pair<int, int>> sizes;
for (each item i)
  sizes.push_back(file_id, offset+length);
sort(sizes, by_pair_first);  // group up same ids together
accumulate(sizes);  // reduce consecutive things with same first by adding their
second.

That could give much better performance I think than a map. I wonder if such
helpful STL tools exist.

https://codereview.chromium.org/2825853002/diff/1/ui/latency/latency_info.h
File ui/latency/latency_info.h (right):

https://codereview.chromium.org/2825853002/diff/1/ui/latency/latency_info.h#n...
ui/latency/latency_info.h:140: typedef base::small_map<
typedef->using?

This looks like a good contender for flat_map too.

https://codereview.chromium.org/2825853002/diff/1/ui/ozone/platform/drm/commo...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2825853002/diff/1/ui/ozone/platform/drm/commo...
ui/ozone/platform/drm/common/drm_util.cc:239:
base::small_map<std::map<ScopedDrmConnectorPtr::element_type*, int>>
almost certainly there are few enough monitors that flat_map makes sense. Also
this could build a vector to init the flat_map, as it's walking thru a set of
unique keys.

[commit-bot: I haz the power, 2017-04-18 22:00:28]

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

[commit-bot: I haz the power, 2017-04-18 22:00:29]

Dry run: This issue passed the CQ dry run.

[brettw, 2017-04-18 23:21:07]

Checkpoint

[brettw, 2017-04-19 00:12:31]

IPC

[brettw, 2017-04-19 00:12:31]

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

[commit-bot: I haz the power, 2017-04-19 00:13: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-04-19 00:23:36]

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

[commit-bot: I haz the power, 2017-04-19 00:23:37]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[brettw, 2017-04-19 16:18:49]

Fixes

[brettw, 2017-04-19 16:18:50]

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

[commit-bot: I haz the power, 2017-04-19 16:19:13]

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

[brettw, 2017-04-19 16:28:37]

Unit tests

[brettw, 2017-04-19 16:28:38]

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

[commit-bot: I haz the power, 2017-04-19 16:28:47]

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

[brettw, 2017-04-19 16:29:47]

brettw@chromium.org changed reviewers:
+ estark@chromium.org

[brettw, 2017-04-19 16:29:49]

This needs a re-review. I had to write some comparators and an IPC serializer
for flat_map that resulting from some of the review suggestions.


estark: Security review of new IPC serialization of base::flat_map in
ipc_message_utils.h

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h
File base/containers/small_map.h (right):

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h...
base/containers/small_map.h:59: //   base::small_map<std::map<MyKey, MyValue>,
4, std::equal_to<KyKey> >
On 2017/04/18 21:35:50, danakj wrote:
> Sidenote: I find it hard to believe it's a good idea to make a std::map for 5
> things. Users of small_map have cargo-culted this constant though. And places
> that didn't picked a larger number that is still way too small for a std::map
to
> be good.
> 
> Probably what this class should do is just use a flat_map until 100 and then
use
> a std::map or something..

Ack

https://codereview.chromium.org/2825853002/diff/1/base/containers/small_map.h...
base/containers/small_map.h:85: //   base::small_map< std::map<string, int> >
days;
On 2017/04/18 21:35:50, danakj wrote:
> you can remove the spaces around the <>

Done.

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
File cc/scheduler/begin_frame_source.cc (right):

https://codereview.chromium.org/2825853002/diff/1/cc/scheduler/begin_frame_so...
cc/scheduler/begin_frame_source.cc:348:
base::small_map<std::map<BeginFrameObserver*, uint64_t>, 4>
On 2017/04/18 21:35:50, danakj wrote:
> do my eyes deceive me or is this shadowing the member variable with a local of
> the same name that will be empty?

That's nuts. I'll fix it, hopefully I don't break anything.

https://codereview.chromium.org/2825853002/diff/1/cc/trees/layer_tree_host_im...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/2825853002/diff/1/cc/trees/layer_tree_host_im...
cc/trees/layer_tree_host_impl.cc:1161: std::set<int> pass_exists;
We discussed in chat also changing this to a flat_set

https://codereview.chromium.org/2825853002/diff/1/storage/browser/blob/blob_m...
File storage/browser/blob/blob_memory_controller.cc (right):

https://codereview.chromium.org/2825853002/diff/1/storage/browser/blob/blob_m...
storage/browser/blob/blob_memory_controller.cc:248:
base::small_map<std::map<uint64_t, uint64_t>> file_id_to_sizes;
An STL nerd would say yes, but my guess is it would be 76 lines of unreadable
std::for_each code :(

https://codereview.chromium.org/2825853002/diff/1/ui/ozone/platform/drm/commo...
File ui/ozone/platform/drm/common/drm_util.cc (right):

https://codereview.chromium.org/2825853002/diff/1/ui/ozone/platform/drm/commo...
ui/ozone/platform/drm/common/drm_util.cc:239:
base::small_map<std::map<ScopedDrmConnectorPtr::element_type*, int>>
On 2017/04/18 21:35:50, danakj wrote:
> almost certainly there are few enough monitors that flat_map makes sense. Also
> this could build a vector to init the flat_map, as it's walking thru a set of
> unique keys.

Hopefully someday I can have so many monitors this code becomes slow.

https://codereview.chromium.org/2825853002/diff/20001/cc/tiles/picture_layer_...
File cc/tiles/picture_layer_tiling.cc (right):

https://codereview.chromium.org/2825853002/diff/20001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling.cc:261: base::flat_map<TileMapKey, gfx::Rect>
remove_tiles;
I had to add an operator< to TileMapKey for this to work.

[commit-bot: I haz the power, 2017-04-19 16:41:20]

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

[commit-bot: I haz the power, 2017-04-19 16:41:21]

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

[danakj, 2017-04-19 17:32:17]

LGTM can you update the CL desc to explain about the conversions to flat_map (or
split them into another CL)

https://codereview.chromium.org/2825853002/diff/100001/base/containers/small_...
File base/containers/small_map.h (right):

https://codereview.chromium.org/2825853002/diff/100001/base/containers/small_...
base/containers/small_map.h:59: //   base::small_map<std::map<MyKey, MyValue>,
4, std::equal_to<KyKey> >
Oh, this has an extra space in the > > too

https://codereview.chromium.org/2825853002/diff/100001/content/zygote/zygote_...
File content/zygote/zygote_linux.h (right):

https://codereview.chromium.org/2825853002/diff/100001/content/zygote/zygote_...
content/zygote/zygote_linux.h:55: base::small_map<std::map<base::ProcessHandle,
ZygoteProcessInfo>>
This lost the ;

hence the indenting that happens below

https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:948: GetParamSize(sizer, static_cast<int>(p.size()));
checked_cast or saturated_cast (leaning toward the former)

https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:955: WriteParam(m, static_cast<int>(p.size()));
same

[brettw, 2017-04-19 17:49:11]

Description was changed from

==========
Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Improvements to uses of base::SmallMap

Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Convert some uses of SmallMap to flat_map where appropriate.

Add an IPC serialization rule for flat_map (required by one of the
SmallMap->flat_map conversions)

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[brettw, 2017-04-19 17:55:04]

https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/100001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:948: GetParamSize(sizer, static_cast<int>(p.size()));
On 2017/04/19 17:32:16, danakj wrote:
> checked_cast or saturated_cast (leaning toward the former)

Done.

estark: This file is full of these casts, should we change them all?

[brettw, 2017-04-19 17:55:34]

Fixes

[brettw, 2017-04-19 17:55:37]

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

[commit-bot: I haz the power, 2017-04-19 17:56: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-04-19 22:59:46]

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

[commit-bot: I haz the power, 2017-04-19 22:59:47]

Dry run: Try jobs failed on following builders:
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[estark, 2017-04-19 23:25:38]

estark@chromium.org changed reviewers:
+ dcheng@chromium.org

[estark, 2017-04-19 23:25:39]

I'm going to take a look at this shortly, but since I'm new to IPC security
reviews I'm adding dcheng as a shadow reviewer.

[estark, 2017-04-20 00:59:49]

lgtm but please wait for dcheng to tell me all the stuff I missed :)

I'm not sure about the checked_cast question. In fact I'm not sure why we should
use checked_cast here because what we're casting is trusted (I think). danakj or
dcheng can you elucidate?

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:957: typename param_type::const_iterator iter;
unneeded?

[dcheng, 2017-04-20 01:30:47]

LGTM

https://codereview.chromium.org/2825853002/diff/120001/base/containers/small_...
File base/containers/small_map.h (right):

https://codereview.chromium.org/2825853002/diff/120001/base/containers/small_...
base/containers/small_map.h:43: //    generate more code than std::map (at least
160 bytes for operator[])
o.O

that's pretty big

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:974: vect.resize(size);
Hmm... too bad there's no great way to do this with reserve. Oh well!

[dcheng, 2017-04-20 01:33:51]

On 2017/04/20 00:59:49, estark wrote:
> lgtm but please wait for dcheng to tell me all the stuff I missed :)
> 
> I'm not sure about the checked_cast question. In fact I'm not sure why we
should
> use checked_cast here because what we're casting is trusted (I think). danakj
or
> dcheng can you elucidate?

Since we're checking on the (trusted) serialization side, I think what we really
want is

DCHECK(base::IsValueInRangeForNumericType<int>(container.size());

(It looks like none of the other serializations bother doing this but it
probably doesn't hurt to add it, as unlikely as we currently are to hit it)

> 
> https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h
> File ipc/ipc_message_utils.h (right):
> 
>
https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils....
> ipc/ipc_message_utils.h:957: typename param_type::const_iterator iter;
> unneeded?

[brettw, 2017-04-20 04:24:24]

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2825853002/diff/120001/ipc/ipc_message_utils....
ipc/ipc_message_utils.h:974: vect.resize(size);
On 2017/04/20 01:30:47, dcheng (OOO through May 2) wrote:
> Hmm... too bad there's no great way to do this with reserve. Oh well!

I could certainly write reserve() and I did the first time I wrote this. It
would look like:

vect.reserve(size);
param_type::value_type pair;
for (...) {
  ReadParam(..., &pair.first);
  ReadParam(..., &pair.second);
  vect.push_back(std::move(pair));
}

This only does one default construction of the pair compared to default
constructing everything with the version I wrote. But it adds a move for
everything. I decided default constructions are usually cheaper than moves so
opted for the resize version.

[brettw, 2017-04-20 04:24:55]

Review comments

[brettw, 2017-04-20 04:24:56]

The CQ bit was checked by brettw@chromium.org

[brettw, 2017-04-20 04:24:56]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
dcheng@chromium.org, estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2825853002/#ps140001
(title: "Review comments")

[commit-bot: I haz the power, 2017-04-20 04:25:33]

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

[brettw, 2017-04-20 04:26:11]

Review comments

[brettw, 2017-04-20 04:26:12]

The CQ bit was checked by brettw@chromium.org

[brettw, 2017-04-20 04:26:12]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
dcheng@chromium.org, estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2825853002/#ps160001
(title: "Review comments")

[commit-bot: I haz the power, 2017-04-20 04:26:38]

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

[brettw, 2017-04-20 04:28:07]

Review comments

[brettw, 2017-04-20 04:28:37]

Patchset #10 (id:180001) has been deleted

[brettw, 2017-04-20 04:28:50]

Patchset #9 (id:160001) has been deleted

[brettw, 2017-04-20 04:29:12]

The CQ bit was checked by brettw@chromium.org

[brettw, 2017-04-20 04:29:12]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
dcheng@chromium.org, estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2825853002/#ps140001
(title: "Review comments")

[commit-bot: I haz the power, 2017-04-20 04:29:40]

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-04-20 06:10:29]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1492662552143230,
"parent_rev": "170a64cd3c62e469dce0fd4790843e414ea8e5ef", "commit_rev":
"ca2ec763405c18960ad1ee66e5ac964a00c257e5"}

[commit-bot: I haz the power, 2017-04-20 06:10:38]

Description was changed from

==========
Improvements to uses of base::SmallMap

Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Convert some uses of SmallMap to flat_map where appropriate.

Add an IPC serialization rule for flat_map (required by one of the
SmallMap->flat_map conversions)

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Improvements to uses of base::SmallMap

Rename base::SmallMap to base::small_map.

Add a readme to base/containers to codify naming rules for containers.

Convert some uses of SmallMap to flat_map where appropriate.

Add an IPC serialization rule for flat_map (required by one of the
SmallMap->flat_map conversions)

Improve the small_map documentation slightly.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2825853002
Cr-Commit-Position: refs/heads/master@{#465911}
Committed:
https://chromium.googlesource.com/chromium/src/+/ca2ec763405c18960ad1ee66e5ac...
==========

[commit-bot: I haz the power, 2017-04-20 06:10:40]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ca2ec763405c18960ad1ee66e5ac...