Stop polling for navigations once a frame is disconnected

Fix a bug in `onDocumentReady` where the callback would fire continuously with
an error after the frame either navigated to a cross-origin URL, or was removed
from its parent frame. Also emit a more appropriate error message in the case
where the frame is disconnected.

In the VitalSource integration this issue manifested in an `onDocumentReady`
callback firing continuously for the previous chapter's iframe when it was
removed from the document. This didn't cause an error because the callback
registered by the VS integration does nothing in that case, but it wasted
resources.

src/annotator/frame-observer.js

@@ -186,8 +186,15 @@ export function onDocumentReady(frame, callback, { pollInterval = 10 } = {}) {
 
   const checkForDocumentChange = () => {
     const currentDocument = frame.contentDocument;
+
+    // `contentDocument` may be null if the frame navigated to a URL that is
+    // cross-origin, or if the `<iframe>` was removed from the document.
     if (!currentDocument) {
-      callback(new Error('Frame is cross-origin'));
+      cancelPoll();
+      const errorMessage = frame.isConnected
+        ? 'Frame is cross-origin'
+        : 'Frame is disconnected';
+      callback(new Error(errorMessage));
       return;
     }
 

src/annotator/test/frame-observer-test.js

@@ -164,6 +164,13 @@ describe('annotator/frame-observer', () => {
   // listening on the port.
   const crossOriginURL = 'http://localhost:12345/test.html';
 
+  function assertCalledOnceWithError(callback, message) {
+    assert.calledOnce(callback);
+    assert.calledWith(callback, sinon.match.instanceOf(Error));
+    const error = callback.args[0][0];
+    assert.equal(error.message, message);
+  }
+
   describe('onDocumentReady', () => {
     it('invokes callback with current document if it is already ready', async () => {
       const callback = sinon.stub();
@@ -219,13 +226,55 @@ describe('annotator/frame-observer', () => {
       const frame = createFrame(crossOriginURL);
       await waitForEvent(frame, 'load');
 
-      onDocumentReady(frame, callback);
+      const pollInterval = 1;
+      onDocumentReady(frame, callback, { pollInterval });
       await waitForCall(callback);
 
+      assertCalledOnceWithError(callback, 'Frame is cross-origin');
+
+      await delay(pollInterval + 10);
       assert.calledOnce(callback);
-      assert.calledWith(callback, sinon.match.instanceOf(Error));
-      const error = callback.args[0][0];
-      assert.equal(error.message, 'Frame is cross-origin');
+    });
+
+    it('invokes callback with error for subsequent navigations to cross-origin document', async () => {
+      const callback = sinon.stub();
+      const frame = createFrame(sameOriginURL);
+      await waitForEvent(frame, 'load');
+
+      const pollInterval = 1;
+      onDocumentReady(frame, callback, { pollInterval });
+      await waitForCall(callback);
+      callback.resetHistory();
+
+      frame.src = crossOriginURL;
+      await waitForEvent(frame, 'load');
+      assertCalledOnceWithError(callback, 'Frame is cross-origin');
+
+      // Wait a moment to check that callback was only invoked once.
+      await delay(pollInterval + 1);
+      assertCalledOnceWithError(callback, 'Frame is cross-origin');
+    });
+
+    it('invokes callback with error if frame is disconnected', async () => {
+      const callback = sinon.stub();
+      const frame = createFrame(sameOriginURL);
+      await waitForEvent(frame, 'load');
+
+      const pollInterval = 1;
+      onDocumentReady(frame, callback, { pollInterval });
+      await waitForCall(callback);
+      callback.resetHistory();
+
+      const frameUnloaded = waitForEvent(frame.contentWindow, 'unload');
+      frame.remove();
+      await frameUnloaded;
+
+      await delay(pollInterval);
+      assertCalledOnceWithError(callback, 'Frame is disconnected');
+
+      // Wait a moment to check that callback was only invoked once.
+      await delay(pollInterval);
+      assertCalledOnceWithError(callback, 'Frame is disconnected');
     });
 
     it('stops polling when subscription is canceled', async () => {