[DIAL] Make a copy of device id before erasing device from map.

[DIAL] Make a copy of device id before erasing device from map.

erase() by key takes input param as a const ref. Invoking erase() by
key using data that will be destroyed during the erase() call will
result in undefined behavior.

BUG=656073
Committed: https://crrev.com/2a375e615245ec9cc6ce32f755a5a46564a250e7
Cr-Commit-Position: refs/heads/master@{#426957}

[imcheng, 2016-10-21 23:36:58]

imcheng@chromium.org changed reviewers:
+ mfoltz@chromium.org

[imcheng, 2016-10-21 23:36:59]

PTAL. Post-merge I do want to rewrite the loop so it doesn't use erase-by-key on
the device_by_id_map_, or maybe just a way that doesn't require maintaining 2
data structures.

[imcheng, 2016-10-22 00:52:43]

imcheng@chromium.org changed reviewers:
+ wez@chromium.org

[Wez, 2016-10-22 00:55:39]

The CQ bit was checked by wez@chromium.org

[Wez, 2016-10-22 00:55:39]

lgtm

https://codereview.chromium.org/2436403003/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/dial/dial_registry.cc (right):

https://codereview.chromium.org/2436403003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/dial/dial_registry.cc:167: const size_t
num_erased_by_id = device_by_id_map_.erase(device_id);
As discussed offline... wow, really surprised that std::map<> is so fragile :-(

[commit-bot: I haz the power, 2016-10-22 00:56:22]

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, 2016-10-22 02:49:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-22 02:52:00]

Description was changed from

==========
[DIAL] Make a copy of device id before erasing device from map.

erase() by key takes input param as a const ref. Invoking erase() by
key using data that will be destroyed during the erase() call will
result in undefined behavior.

BUG=656073
==========

to

==========
[DIAL] Make a copy of device id before erasing device from map.

erase() by key takes input param as a const ref. Invoking erase() by
key using data that will be destroyed during the erase() call will
result in undefined behavior.

BUG=656073

Committed: https://crrev.com/2a375e615245ec9cc6ce32f755a5a46564a250e7
Cr-Commit-Position: refs/heads/master@{#426957}
==========

[commit-bot: I haz the power, 2016-10-22 02:52:00]

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

[miu, 2016-10-22 18:37:18]

miu@chromium.org changed reviewers:
+ miu@chromium.org

[miu, 2016-10-22 18:37:19]

https://codereview.chromium.org/2436403003/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/api/dial/dial_registry.cc (right):

https://codereview.chromium.org/2436403003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/dial/dial_registry.cc:166: std::string device_id =
device->device_id();
nit: Probably not a big deal for performance, but the copy can be avoided by
using std::move() here. Though, IIRC, our std::strings are ref-counted, so
there's probably no copy going on internally.