Fix leak in VRServiceImpl

Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit, and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org

Committed: https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8
Cr-Commit-Position: refs/heads/master@{#439333}

[mthiesse, 2016-12-14 20:03:52]

Description was changed from

==========
Fix leak in VRServiceImpl

BUG=651245
==========

to

==========
Fix leak in VRServiceImpl

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[mthiesse, 2016-12-14 20:06:02]

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

[mthiesse, 2016-12-14 20:06:02]

dcheng@, do you mind reviewing this? Brandon's on leave and (sorry) you raised
the issue :P

[mthiesse, 2016-12-14 20:07:26]

Description was changed from

==========
Fix leak in VRServiceImpl

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[dcheng, 2016-12-16 02:50:17]

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.cc
File device/vr/vr_service_impl.cc (right):

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.c...
device/vr/vr_service_impl.cc:20: // displays_ is a map which first element is a
VRDevice pointer,
first element => the key
second element => the value

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.c...
device/vr/vr_service_impl.cc:22: for (DisplayImplMap::iterator it =
displays_.begin(); it != displays_.end();
Nit:

for (auto& it : displays_)
  it->first->RemoveDisplay(it->second.get());

Out of curiosity though, why can't VRDisplayImpl just remove itself from the
device on destruction? It has a pointer to the device, and it seems a bit nicer
than embedding that logic here.

[mthiesse, 2016-12-16 15:45:44]

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.cc
File device/vr/vr_service_impl.cc (right):

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.c...
device/vr/vr_service_impl.cc:20: // displays_ is a map which first element is a
VRDevice pointer,
On 2016/12/16 02:50:17, dcheng wrote:
> first element => the key
> second element => the value

Acknowledged.

https://codereview.chromium.org/2571323002/diff/1/device/vr/vr_service_impl.c...
device/vr/vr_service_impl.cc:22: for (DisplayImplMap::iterator it =
displays_.begin(); it != displays_.end();
On 2016/12/16 02:50:17, dcheng wrote:
> Nit:
> 
> for (auto& it : displays_)
>   it->first->RemoveDisplay(it->second.get());
> 
> Out of curiosity though, why can't VRDisplayImpl just remove itself from the
> device on destruction? It has a pointer to the device, and it seems a bit
nicer
> than embedding that logic here.

Good point, done.

[mthiesse, 2016-12-16 15:47:30]

mthiesse@chromium.org changed reviewers:
+ bajones@chromium.org, brettw@chromium.org

[mthiesse, 2016-12-16 15:47:31]

brettw@chromium.org: Please owners review trivial content/browser/ change

bajones@chromium.org: Please review changes in device/vr

[bajones, 2016-12-16 16:12:45]

On 2016/12/16 15:47:31, mthiesse wrote:
> mailto:brettw@chromium.org: Please owners review trivial content/browser/
change
> 
> mailto:bajones@chromium.org: Please review changes in device/vr

Device/VR LGTM

[dcheng, 2016-12-16 18:13:57]

LGTM, thanks for cleaning this up!

[mthiesse, 2016-12-17 03:58:18]

Description was changed from

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org
==========

[mthiesse, 2016-12-17 04:00:59]

I'm TBRing the trivial rename in content/browser as I'll be OOO until early
january and won't be able to fix any merge conflicts that may come up.

[mthiesse, 2016-12-17 04:01:03]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-12-17 04:01:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
bajones@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2571323002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-17 04:01:36]

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-12-17 07:35:59]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481947263111150,
"parent_rev": "349953ccfa532f5611d031285246f6710bf04c55", "commit_rev":
"bfb24a8fbb779006bc71892e79e3a4d75cc192dd"}

[commit-bot: I haz the power, 2016-12-17 07:36:27]

Description was changed from

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org
==========

to

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org

Review-Url: https://codereview.chromium.org/2571323002
==========

[commit-bot: I haz the power, 2016-12-17 07:36:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-17 07:38:31]

Description was changed from

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org

Review-Url: https://codereview.chromium.org/2571323002
==========

to

==========
Fix leak in VRServiceImpl

This makes the implicit Strong Binding between the service and clients explicit,
and fixes the service leaking when the binding is closed.

No functional changes here.

BUG=651245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
#TBR-ing a simple rename to match style.
TBR=brettw@chromium.org

Committed: https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8
Cr-Commit-Position: refs/heads/master@{#439333}
==========

[commit-bot: I haz the power, 2016-12-17 07:38:32]

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