(Just to make sure this doesn't fall off the radar)

Ping Brandon/Klaus, is this something you're able to look at?  Seems like we'd want to merge a fix for this if it's a legit leak.

Any updates? It looks like this is shipping behind an origin trial now: it would be good to address this before M56 is going to hit stable.

I'll look into this I guess.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8

commit 7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8
Author: mthiesse <mthiesse@chromium.org>
Date: Sat Dec 17 07:35:55 2016

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
Cr-Commit-Position: refs/heads/master@{#439333}

[modify] https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8/device/vr/vr_display_impl.cc
[modify] https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8/device/vr/vr_service_impl.cc
[modify] https://crrev.com/7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8/device/vr/vr_service_impl.h

Your change meets the bar and is auto-approved for M56 (branch: 2924)

Biao, would you mind merging this while I'm OOO?

https://codereview.chromium.org/2590883003 is created.
There were conflict in device/vr/vr_service_impl.cc file. Basically the two removed functions VRServiceImpl::Bind and VRServiceImpl::RemoveFromDeviceManager have different implementation in M56 and TOT. To solve the conflict, I removed them anyway. It looks safe but mthiesse@, can you take a look when you are free.

Actually, it is this CL that cause the conflict: https://codereview.chromium.org/2537723002
And there is a interface change from VRDevice::RemoveService to VRDevice::RemoveDisplay which we also need to update. I will do a local build to see if it works.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/225bb8d94ed0a406519a9935c6577a2855aafed1

commit 225bb8d94ed0a406519a9935c6577a2855aafed1
Author: bshe <bshe@chromium.org>
Date: Wed Dec 21 18:49:43 2016

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=brettw@chromium.org

Review-Url: https://codereview.chromium.org/2571323002
Cr-Commit-Position: refs/heads/master@{#439333}
(cherry picked from commit 7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8)

Review-Url: https://codereview.chromium.org/2590883003 .
Cr-Commit-Position: refs/branch-heads/2924@{#581}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/225bb8d94ed0a406519a9935c6577a2855aafed1/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/225bb8d94ed0a406519a9935c6577a2855aafed1/device/vr/vr_display_impl.cc
[modify] https://crrev.com/225bb8d94ed0a406519a9935c6577a2855aafed1/device/vr/vr_service_impl.cc
[modify] https://crrev.com/225bb8d94ed0a406519a9935c6577a2855aafed1/device/vr/vr_service_impl.h

Thanks Biao, looks good.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49f0745139f12e0a300bba313696a802efbc78cc

commit 49f0745139f12e0a300bba313696a802efbc78cc
Author: mthiesse <mthiesse@chromium.org>
Date: Wed Jan 04 21:27:54 2017

Revert of Fix leak in VRServiceImpl (patchset #2 id:20001 of https://codereview.chromium.org/2590883003/ )

Reason for revert:
Causes crash on Chrome Beta 56.0.2924.51

Original issue's description:
> 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=brettw@chromium.org
>
> Review-Url: https://codereview.chromium.org/2571323002
> Cr-Commit-Position: refs/heads/master@{#439333}
> (cherry picked from commit 7980a0e0ca7f5a6abda7e57ab28fbf8f845bfbf8)
>
> Committed: https://chromium.googlesource.com/chromium/src/+/225bb8d94ed0a406519a9935c6577a2855aafed1

TBR=brettw@chromium.org,bshe@chromium.org
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 651245 

Review-Url: https://codereview.chromium.org/2615723002
Cr-Commit-Position: refs/branch-heads/2924@{#669}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/49f0745139f12e0a300bba313696a802efbc78cc/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/49f0745139f12e0a300bba313696a802efbc78cc/device/vr/vr_display_impl.cc
[modify] https://crrev.com/49f0745139f12e0a300bba313696a802efbc78cc/device/vr/vr_service_impl.cc
[modify] https://crrev.com/49f0745139f12e0a300bba313696a802efbc78cc/device/vr/vr_service_impl.h

Well, this is still fixed in M57, only the backport to M56 broke.

I spent ~half a day trying to get M56 building with no success (build folks are looking into the issues I was having). I'd say this is probably not worth the effort and risk of fixing and backporting to M56. The impact of this leak should be tiny, as it's a small leak that only occurs when leaving or reloading a WebVR page (and WebVR is still gated on origin trial or chrome flag).

Feel free to reopen if you think it's worth me spending more time on.