Reilly, was there supposed to be any obscure details to this other than just making SensorProviderProxy a Document supplement?

https://chromium-review.googlesource.com/c/chromium/src/+/1160161 is causing  telemetry.internal.backends.chrome_inspector.inspector_memory_unittest.InspectorMemoryTest.testGetDOMStats to fail because DeviceOrientationInspectorAgent::DeviceOrientationInspectorAgent() calls SensorProviderProxy::From() and apparently we end up with an extra reference to Document (coming from Supplement<T> having a strong reference to T).

I'm guessing this is not a new problem and we were also holding a reference to LocalFrame before, but I'd like some confirmation of what I'm seeing here.

Erm, in the end it was something much more prosaic: if I understood things correctly, DeviceOrientationInspectorAgent stays alive for longer than a Document. Since it keeps a SensorInspectorAgent (and consequently a SensorProviderProxy) around as well, SensorProviderProxy outlives the document and prevents it from being GC'ed. Manually resetting SensorInspectorAgent in DeviceOrientationInspectorAgent::DidCommitLoadForLocalFrame() worked.

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

commit d1034e1e6f204eb0467974a30cc0cdef1bf36c2b
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Fri Sep 07 14:23:22 2018

sensors: Make SensorProviderProxy supplement Document, not LocalFrame

Supplementing LocalFrame causes the same SensorProviderProxy to be used
across navigations, and so is the connection to the actual sensor provider
via mojo.

Making SensorProviderProxy supplement Document also helps with testing: as
described in the related bug, running a manual test (which does not use
sensor mocks) and then a regular test causes the latter to fail because we
never establish a connection to the mock we need, but rather try to use an
actual sensor that is not present.

Bug:  861675 
Change-Id: Ic3d2ea42f5b4654ba5ef42b6ad757a49d3e5ef08
Reviewed-on: https://chromium-review.googlesource.com/1160161
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#589514}
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/WebKit/LayoutTests/http/tests/devtools/device-orientation-success-expected.txt
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/WebKit/LayoutTests/http/tests/devtools/device-orientation-success.js
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/device_orientation/device_orientation_inspector_agent.cc
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor.cc
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor_inspector_agent.cc
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor_inspector_agent.h
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor_provider_proxy.cc
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor_provider_proxy.h
[modify] https://crrev.com/d1034e1e6f204eb0467974a30cc0cdef1bf36c2b/third_party/blink/renderer/modules/sensor/sensor_proxy.cc

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

commit c30292f971699886fbc08e87ff32cb82aa58f99d
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Tue Oct 02 20:03:19 2018

sensors: Ensure a document without an associated frame does not crash

Commit d1034e1e6 ("sensors: Make SensorProviderProxy supplement Document,
not LocalFrame") tied a sensor's lifetime to a document rather than a frame,
but we continued to assume Document::GetFrame() would never return null.

This is not true, as evidenced by the crash reports in bug 889754, caused by
SensorProxy::ShouldSuspendUpdates() trying to invoke methods on a LocalFrame
that can actually be a nullptr.

The original backtrace in the bug report seems to come from sensor creation,
but it is easier to trigger the same crash with a focus change after
destroying a sensor's document's frame.

Bug:  861675 , 889754
Change-Id: Idb9ed5c18a655e113e2fb76cde6615aeefcc544a
Reviewed-on: https://chromium-review.googlesource.com/1256826
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#595958}
[modify] https://crrev.com/c30292f971699886fbc08e87ff32cb82aa58f99d/third_party/WebKit/LayoutTests/external/wpt/accelerometer/Accelerometer-iframe-access.https-expected.txt
[modify] https://crrev.com/c30292f971699886fbc08e87ff32cb82aa58f99d/third_party/WebKit/LayoutTests/external/wpt/generic-sensor/generic-sensor-iframe-tests.sub.js
[modify] https://crrev.com/c30292f971699886fbc08e87ff32cb82aa58f99d/third_party/WebKit/LayoutTests/external/wpt/geolocation-sensor/GeolocationSensor-iframe-access.https-expected.txt
[modify] https://crrev.com/c30292f971699886fbc08e87ff32cb82aa58f99d/third_party/WebKit/LayoutTests/external/wpt/magnetometer/Magnetometer-iframe-access.https-expected.txt
[modify] https://crrev.com/c30292f971699886fbc08e87ff32cb82aa58f99d/third_party/blink/renderer/modules/sensor/sensor_proxy.cc