Remove uses of base::hash_map from //chrome

Remove uses of base::hash_map from //chrome

In cases where it seemed like there might be a high query volume or I had no way of knowing, it is replaced with a std::unordered_map (which base::hash_map is a typedef for already).

In cases where I understood the workload and it was appropriate, base::flat_map is used. In a few places that were create-once-then-read-only, the flat_map optimized construction from a vector is used.

In all other cases, std::map is used.

Review-Url: https://codereview.chromium.org/2829163004
Cr-Commit-Position: refs/heads/master@{#483431}
Committed: https://chromium.googlesource.com/chromium/src/+/84cff3faf4bd3ae60fcb70b0e2c526c08709be5e

[brettw, 2017-05-12 21:26:01]

.

[brettw, 2017-05-12 21:54:52]

Description was changed from

==========
chrome_hash

BUG=
==========

to

==========
Remove uses of base::hash_map from //chrome

In cases where it seemed like there might be a high query volume or I had no way
of knowing, it is replaced with a std::unordered_map (which base::hash_map is a
typedef for already).

In cases where I understood the workload and it was appropriate, base::flat_map
is used. In a few places that were create-once-then-read-only, the flat_map
optimized construction from a vector is used.

In all other cases, std::map is used.
==========

[brettw, 2017-05-12 21:54:52]

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

[brettw, 2017-05-12 21:55:07]

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

[commit-bot: I haz the power, 2017-05-12 21:56:15]

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-05-12 22:13:41]

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

[commit-bot: I haz the power, 2017-05-12 22:13:42]

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...)

[brettw, 2017-05-15 21:20:29]

Fixes

[brettw, 2017-05-15 21:20:29]

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

[commit-bot: I haz the power, 2017-05-15 21:21:36]

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-05-15 21:46:09]

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

[commit-bot: I haz the power, 2017-05-15 21:46:10]

Dry run: Try jobs failed on following builders:
  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-...)

[brettw, 2017-05-15 22:52:02]

Fixes

[brettw, 2017-05-15 22:52:03]

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

[commit-bot: I haz the power, 2017-05-15 22:53: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-05-15 23:16:40]

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

[commit-bot: I haz the power, 2017-05-15 23:16:41]

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

[brettw, 2017-05-22 22:04:47]

Merge

[brettw, 2017-05-22 22:04:47]

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

[commit-bot: I haz the power, 2017-05-22 22:05:33]

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-05-22 22:32:47]

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

[commit-bot: I haz the power, 2017-05-22 22:32:48]

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

[brettw, 2017-05-22 23:07:12]

Fixes

[brettw, 2017-05-22 23:07:13]

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

[commit-bot: I haz the power, 2017-05-22 23:07:47]

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-05-23 00:06:42]

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

[commit-bot: I haz the power, 2017-05-23 00:06:43]

Dry run: This issue passed the CQ dry run.

[brettw, 2017-05-23 17:06:39]

brettw@chromium.org changed reviewers:
+ chrisha@chromium.org
- jdoerrie@chromium.org

[chrisha, 2017-05-23 20:08:00]

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_util.cc:389:
map_entry.resource_id);
nit: Instead of dynamically creating a const flat_map here, can't we simply
search through the already ordered static array?

(This isn't a huge absolute cost, but duplicating memory for no reason is
wasteful. If only we also had a way to check and enforce the no-dupes and sorted
requirements at compile time...)

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_util.cc:425: auto iter =
english_to_resource_id_.find(key_string);
Just use std::binary_search?

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/api/downloads/downloads_api.cc (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/downloads/downloads_api.cc:414:
AppendFilter(kBytesReceivedKey, DownloadQuery::SORT_BYTES_RECEIVED, &v);
AppendSortQuery? AppendSortType?

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/downloads/downloads_api.cc:428: }
At a higher level: Should both of the short lists simply be stored statically,
and directly std::binary_search'd or even linear-searched through? We're already
paying the storage cost in the image via the generated code, and then we're
paying it a second time here on the heap. We're also recalculating these maps
with every call to RunDownloadQuery.

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:1264: // 
http://crbug.com/365334
Fix leading spaces in this line and the one above.

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/font_fa...
File chrome/browser/font_family_cache.h (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/font_fa...
chrome/browser/font_family_cache.h:27: // keys used in both inner and outer aps
are compile time constants.
maps*

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/sync_fi...
File chrome/browser/sync_file_system/drive_backend/sync_worker.h (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/sync_fi...
chrome/browser/sync_file_system/drive_backend/sync_worker.h:114: typedef
std::unordered_map<std::string, AppStatus> AppStatusMap;
using?

[brettw, 2017-06-28 23:56:46]

Comments, no downloads.

[brettw, 2017-06-28 23:58:25]

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

[brettw, 2017-06-28 23:58:39]

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/input_method/input_method_util.cc:389:
map_entry.resource_id);
On 2017/05/23 20:07:59, chrisha wrote:
> nit: Instead of dynamically creating a const flat_map here, can't we simply
> search through the already ordered static array?
> 
> (This isn't a huge absolute cost, but duplicating memory for no reason is
> wasteful. If only we also had a way to check and enforce the no-dupes and
sorted
> requirements at compile time...)

That would be better but I just spent 2 hours on the downloads one. I don't
think this (which I'm not even set up to test locally) would be a good use of my
time.

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/api/downloads/downloads_api.cc (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/downloads/downloads_api.cc:428: }
I did the downloads one in a separate CL:
https://codereview.chromium.org/2956213003/

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:1264: // 
http://crbug.com/365334
On 2017/05/23 20:07:59, chrisha wrote:
> Fix leading spaces in this line and the one above.

Done.

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/sync_fi...
File chrome/browser/sync_file_system/drive_backend/sync_worker.h (right):

https://codereview.chromium.org/2829163004/diff/140001/chrome/browser/sync_fi...
chrome/browser/sync_file_system/drive_backend/sync_worker.h:114: typedef
std::unordered_map<std::string, AppStatus> AppStatusMap;
On 2017/05/23 20:07:59, chrisha wrote:
> using?

Done.

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

Dry run: CQ is trying da patch.

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

[brettw, 2017-06-29 00:13:09]

Downloads back

[brettw, 2017-06-29 00:13:20]

I've abandoned the downloads refactor requested and put it back the way I had
it. I wasted literally the entire afternoon on this and it still doesn't work. I
think it's fine the way it is.

[brettw, 2017-06-29 00:13:31]

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

[commit-bot: I haz the power, 2017-06-29 00:13:55]

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-06-29 02:04:28]

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

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

Dry run: This issue passed the CQ dry run.

[chrisha, 2017-06-29 17:44:14]

lgtm!

[brettw, 2017-06-29 18:20:44]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2017-06-29 18:21:07]

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-06-29 18:26:53]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1498760444526770,
"parent_rev": "e5e0e9ee896c6e6ce80549d6ef2fcbd96589532f", "commit_rev":
"84cff3faf4bd3ae60fcb70b0e2c526c08709be5e"}

[commit-bot: I haz the power, 2017-06-29 18:27:05]

Description was changed from

==========
Remove uses of base::hash_map from //chrome

In cases where it seemed like there might be a high query volume or I had no way
of knowing, it is replaced with a std::unordered_map (which base::hash_map is a
typedef for already).

In cases where I understood the workload and it was appropriate, base::flat_map
is used. In a few places that were create-once-then-read-only, the flat_map
optimized construction from a vector is used.

In all other cases, std::map is used.
==========

to

==========
Remove uses of base::hash_map from //chrome

In cases where it seemed like there might be a high query volume or I had no way
of knowing, it is replaced with a std::unordered_map (which base::hash_map is a
typedef for already).

In cases where I understood the workload and it was appropriate, base::flat_map
is used. In a few places that were create-once-then-read-only, the flat_map
optimized construction from a vector is used.

In all other cases, std::map is used.

Review-Url: https://codereview.chromium.org/2829163004
Cr-Commit-Position: refs/heads/master@{#483431}
Committed:
https://chromium.googlesource.com/chromium/src/+/84cff3faf4bd3ae60fcb70b0e2c5...
==========

[commit-bot: I haz the power, 2017-06-29 18:27:06]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/84cff3faf4bd3ae60fcb70b0e2c5...