Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer modes

Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer modes

Now callers directly specify what type of pointer they want IDMap to
hold using the template parameter.

Part 1 is at https://codereview.chromium.org/2480293004/

BUG=647091
TBR=dbeam (for chrome/browser/ui/webui/options/certificate_manager_handler.cc)
TBR=jam (for all other non-base/ changes)

Committed: https://crrev.com/de24c6dcd7173b5ea020223b22e0081c0f60e7d1
Cr-Commit-Position: refs/heads/master@{#435539}

[rlanday, 2016-11-10 19:56:54]

rlanday@chromium.org changed reviewers:
+ aelias@chromium.org

[rlanday, 2016-11-10 19:56:54]

rlanday@chromium.org changed required reviewers:
+ aelias@chromium.org

[aelias_OOO_until_Jul13, 2016-11-11 22:43:24]

lgtm, let's follow up on landing after http://crrev.com/2480293004/

[rlanday, 2016-11-30 19:43:34]

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

[rlanday, 2016-11-30 19:43:34]

rlanday@chromium.org changed required reviewers:
+ danakj@chromium.org

[rlanday, 2016-11-30 19:48:52]

On 2016/11/11 at 22:43:24, aelias wrote:
> lgtm, let's follow up on landing after http://crrev.com/2480293004/

danakj, this is a follow-up to the refactor in http://crrev.com/2480293004/ . I
modified base/id_map.h and then ran a mechanical transformation on the code base
using a Python script, and then manually cleaned up code that wasn't transformed
properly by my regex. I then ran git cl format and kept all the formatting
changes touching a line with IDMap in it. If the changes to base/ look good, I
think it makes sense to TBR the mechanical changes.

[danakj, 2016-11-30 23:02:21]

LGTM.

It'd be nice to update any "typedef" usage to be "using" instead while you're
touching them. There's a bunch thoughout.

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h
File base/id_map.h (right):

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h#newcode29
base/id_map.h:29: // This class does not have a virtual destructor, do not
inherit from it when
This comment doesn't quite make sense anymore. Does anything inherit from this?
That'd be crazy. Can you maybe just mark the class as final instead?

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h#newcode31
base/id_map.h:31: template <typename V, typename K = int32_t>
Maybe worth a comment now saying that the values (represented as the V
parameter) can be any type that can be dereferenced, such as a raw pointer or a
smart pointer.

https://codereview.chromium.org/2496653002/diff/20001/components/drive/job_sc...
File components/drive/job_scheduler.h (right):

https://codereview.chromium.org/2496653002/diff/20001/components/drive/job_sc...
components/drive/job_scheduler.h:385: typedef IDMap<std::unique_ptr<JobEntry>>
JobIDMap;
s/typedef/using/ while you're here?

https://codereview.chromium.org/2496653002/diff/20001/components/spellcheck/r...
File components/spellcheck/renderer/spellcheck_provider.h (right):

https://codereview.chromium.org/2496653002/diff/20001/components/spellcheck/r...
components/spellcheck/renderer/spellcheck_provider.h:38: typedef
IDMap<blink::WebTextCheckingCompletion*> WebTextCheckCompletions;
s/typedef/using/ while you're here?

[rlanday, 2016-12-01 00:50:07]

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h
File base/id_map.h (right):

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h#newcode29
base/id_map.h:29: // This class does not have a virtual destructor, do not
inherit from it when
On 2016/11/30 at 23:02:21, danakj wrote:
> This comment doesn't quite make sense anymore. Does anything inherit from
this? That'd be crazy. Can you maybe just mark the class as final instead?

Yeah it looks like nothing inherits from this

https://codereview.chromium.org/2496653002/diff/20001/base/id_map.h#newcode31
base/id_map.h:31: template <typename V, typename K = int32_t>
On 2016/11/30 at 23:02:21, danakj wrote:
> Maybe worth a comment now saying that the values (represented as the V
parameter) can be any type that can be dereferenced, such as a raw pointer or a
smart pointer.

Ok, that's a good point

[rlanday, 2016-12-01 01:22:20]

Description was changed from

==========
Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer
modes

Now callers directly specify what type of pointer they want IDMap to
hold using the template parameter.

Part 1 is at https://codereview.chromium.org/2480293004/

BUG=647091
==========

to

==========
Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer
modes

Now callers directly specify what type of pointer they want IDMap to
hold using the template parameter.

Part 1 is at https://codereview.chromium.org/2480293004/

BUG=647091

TBR=dbeam (for chrome/browser/ui/webui/options/certificate_manager_handler.cc)
TBR=jam (for all other non-base/ changes)
==========

[rlanday, 2016-12-01 01:25:38]

dbeam + jam, danakj has reviewed the changes in base/id_map.h, the remaining
changes fall into one of these categories:
- automated mechanical changes to users of IDMap
- manual fixes to the automated changes
- updating typedefs to using statements as danakj requested
- formatting changes to the modified code from git cl format

So I am TBRing these changes.

[rlanday, 2016-12-01 01:26:28]

The CQ bit was checked by rlanday@chromium.org

[rlanday, 2016-12-01 01:26:29]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2496653002/#ps40001
(title: "typedefs => using statements, update comments in base/id_map.h")

[commit-bot: I haz the power, 2016-12-01 01:27:00]

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

[danakj, 2016-12-01 01:53:40]

Thanks for the usings :)

[commit-bot: I haz the power, 2016-12-01 03:05:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480555588466470,
"parent_rev": "e7f1c08d5e4aa312a4688feec92cba07e687671d", "commit_rev":
"459c8f079eb1df03fb23c08bc190ef0ec659088d"}

[commit-bot: I haz the power, 2016-12-01 03:06:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-01 03:10:59]

Description was changed from

==========
Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer
modes

Now callers directly specify what type of pointer they want IDMap to
hold using the template parameter.

Part 1 is at https://codereview.chromium.org/2480293004/

BUG=647091

TBR=dbeam (for chrome/browser/ui/webui/options/certificate_manager_handler.cc)
TBR=jam (for all other non-base/ changes)
==========

to

==========
Part 2 of base::IDMap refactor to eliminate IDMapOwnPointer/IDMapExternalPointer
modes

Now callers directly specify what type of pointer they want IDMap to
hold using the template parameter.

Part 1 is at https://codereview.chromium.org/2480293004/

BUG=647091

TBR=dbeam (for chrome/browser/ui/webui/options/certificate_manager_handler.cc)
TBR=jam (for all other non-base/ changes)

Committed: https://crrev.com/de24c6dcd7173b5ea020223b22e0081c0f60e7d1
Cr-Commit-Position: refs/heads/master@{#435539}
==========

[commit-bot: I haz the power, 2016-12-01 03:11:01]

Patchset 3 (id:??) landed as
https://crrev.com/de24c6dcd7173b5ea020223b22e0081c0f60e7d1
Cr-Commit-Position: refs/heads/master@{#435539}