Commit 6ca273c1 authored by Nick Stenning's avatar Nick Stenning

Ensure that frame-rpc event listeners are correctly torn down

When the client is destroyed, it's important that we correctly tear down
the frame-rpc event listeners (bound to the document's `onmessage`
event).

These event listeners are responsible for relaying messages from the
frame-rpc mechanism to various parts of the code on either side of the
frame boundary, including the `AnnotationSync` component. If the
frame-rpc channels aren't correctly torn down, they will hold a
reference to the `Bridge` object, which in turn holds a reference to the
`AnnotationSync` object, and thus old copies of `AnnotationSync` which
should have been garbage-collected will continue to receive events from
the brand new Hypothesis client.

The most visible result of this problem was that repeated activation and
deactivation of the client would result in increasing numbers of
highlights being drawn on annotated text, as reported in
hypothesis/h#3096.

Fixes hypothesis/h#3096.
parent 429009ca
...@@ -31,6 +31,7 @@ module.exports = class CrossFrame extends Annotator.Plugin ...@@ -31,6 +31,7 @@ module.exports = class CrossFrame extends Annotator.Plugin
this.destroy = -> this.destroy = ->
# super doesnt work here :( # super doesnt work here :(
Annotator.Plugin::destroy.apply(this, arguments) Annotator.Plugin::destroy.apply(this, arguments)
bridge.destroy()
discovery.stopDiscovery() discovery.stopDiscovery()
this.sync = (annotations, cb) -> this.sync = (annotations, cb) ->
......
...@@ -19,6 +19,7 @@ describe 'Annotator.Plugin.CrossFrame', -> ...@@ -19,6 +19,7 @@ describe 'Annotator.Plugin.CrossFrame', ->
stopDiscovery: sandbox.stub() stopDiscovery: sandbox.stub()
fakeBridge = fakeBridge =
destroy: sandbox.stub()
createChannel: sandbox.stub() createChannel: sandbox.stub()
onConnect: sandbox.stub() onConnect: sandbox.stub()
call: sandbox.stub() call: sandbox.stub()
...@@ -75,10 +76,15 @@ describe 'Annotator.Plugin.CrossFrame', -> ...@@ -75,10 +76,15 @@ describe 'Annotator.Plugin.CrossFrame', ->
describe '.destroy', -> describe '.destroy', ->
it 'stops the discovery of new frames', -> it 'stops the discovery of new frames', ->
bridge = createCrossFrame() cf = createCrossFrame()
bridge.destroy() cf.destroy()
assert.called(fakeDiscovery.stopDiscovery) assert.called(fakeDiscovery.stopDiscovery)
it 'destroys the bridge object', ->
cf = createCrossFrame()
cf.destroy()
assert.called(fakeBridge.destroy)
describe '.sync', -> describe '.sync', ->
it 'syncs the annotations with the other frame', -> it 'syncs the annotations with the other frame', ->
bridge = createCrossFrame() bridge = createCrossFrame()
......
...@@ -14,6 +14,13 @@ module.exports = class Bridge ...@@ -14,6 +14,13 @@ module.exports = class Bridge
@channelListeners = {} @channelListeners = {}
@onConnectListeners = [] @onConnectListeners = []
# Tear down the bridge. We destroy each RPC "channel" object we know about.
# This removes the `onmessage` event listeners, thus removing references to
# any listeners and allowing them to be garbage collected.
destroy: ->
for link in @links
link.channel.destroy()
createChannel: (source, origin, token) -> createChannel: (source, origin, token) ->
channel = null channel = null
connected = false connected = false
......
...@@ -190,3 +190,15 @@ describe 'Bridge', -> ...@@ -190,3 +190,15 @@ describe 'Bridge', ->
bridge.onConnect(callback) bridge.onConnect(callback)
bridge.onConnect(callback) bridge.onConnect(callback)
channel = createChannel() channel = createChannel()
describe '.destroy', ->
it 'destroys all opened channels', ->
channel1 = bridge.createChannel(fakeWindow, 'http://example.com', 'foo')
channel2 = bridge.createChannel(fakeWindow, 'http://example.com', 'bar')
sinon.spy(channel1, 'destroy')
sinon.spy(channel2, 'destroy')
bridge.destroy()
assert.called(channel1.destroy)
assert.called(channel2.destroy)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment