Fix: Rename CloseWatcher.signalClose to CloseWatched.requestClose (#40213)

* 📦 Update dependency chromedriver to v131

* switch to realWin to resolve scheme-less url to the correct hostname

something changed recently with Url Parsing in Chrome or something in
the FakeWin implementation is causign it to not resolve properly.
Quick fix seems to be to switch it to a real window

* try out the disable feature

* rename to requestClose

* undo skip to test which tests are still failing

* add back skip

* try w/o closewatcher

* Update test/unit/utils/test-close-watcher-impl.js

Co-authored-by: Daniel Rozenberg <[email protected]>

* Update test/unit/utils/test-close-watcher-impl.js

Co-authored-by: Daniel Rozenberg <[email protected]>

* Update test/unit/utils/test-close-watcher-impl.js

Co-authored-by: Daniel Rozenberg <[email protected]>

* Update test/unit/utils/test-close-watcher-impl.js

Co-authored-by: Daniel Rozenberg <[email protected]>

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Daniel Rozenberg <[email protected]>

Author: 3 people

extensions/amp-lightbox/0.1/test/test-amp-lightbox.js

@@ -114,7 +114,7 @@ describes.realWin(
       const setupCloseSpy = env.sandbox.spy(impl, 'close');
 
       await impl.open_({caller: sourceElement});
-      impl.closeWatcher_.signalClosed();
+      impl.closeWatcher_.requestClose();
       expect(setupCloseSpy).to.be.called;
     });
 

src/utils/close-watcher-impl.js

@@ -67,11 +67,11 @@ export class CloseWatcherImpl {
 
   /**
    * Signals to the close watcher to close the modal.
-   * See `CloseWatcher.signalClosed`.
+   * See `CloseWatcher.requestClose`.
    */
-  signalClosed() {
+  requestClose() {
     if (this.watcher_) {
-      this.watcher_.signalClosed();
+      this.watcher_.requestClose();
     } else if (this.handler_) {
       const handler = this.handler_;
       handler();
@@ -106,7 +106,7 @@ export class CloseWatcherImpl {
   closeOnEscape_(event) {
     if (event.key == Keys_Enum.ESCAPE) {
       event.preventDefault();
-      this.signalClosed();
+      this.requestClose();
     }
   }
 }

src/utils/close-watcher.extern.js

@@ -12,7 +12,7 @@ function CloseWatcher() {}
 
 CloseWatcher.prototype.destroy = function () {};
 
-CloseWatcher.prototype.signalClosed = function () {};
+CloseWatcher.prototype.requestClose = function () {};
 
 /** @type {?function(!Event)} */
 CloseWatcher.prototype.onclose;

test/unit/utils/test-close-watcher-impl.js

@@ -24,46 +24,57 @@ describes.realWin('#CloseWatcherImpl', {amp: true}, (env) => {
     historyMock.verify();
   });
 
-  // TODO(#40214): fix flaky test.
-  it.skip('should push and pop history state', async () => {
-    historyMock.expects('push').resolves('H1').once();
-    historyMock.expects('pop').withArgs('H1').once();
-    const watcher = new CloseWatcherImpl(ampdoc, handler);
-    await Promise.resolve('H1');
-    watcher.signalClosed();
-    expect(handler).to.be.calledOnce;
-  });
+  it.configure()
+    .if(() => 'CloseWatcher' in window)
+    .run('should call the handler on requestClose', async () => {
+      const watcher = new CloseWatcherImpl(ampdoc, handler);
+      watcher.requestClose();
+      expect(handler).to.be.calledOnce;
+    });
 
-  // TODO(#40214): fix flaky test.
-  it.skip('should trigger on history pop', async () => {
-    let popHandler;
-    historyMock
-      .expects('push')
-      .withArgs(
-        env.sandbox.match((a) => {
-          popHandler = a;
-          return true;
-        })
-      )
-      .resolves('H1')
-      .once();
-    new CloseWatcherImpl(ampdoc, handler);
-    await Promise.resolve('H1');
-    expect(popHandler).to.exist;
-    popHandler();
-    expect(handler).to.be.calledOnce;
-  });
+  it.configure()
+    .if(() => !('CloseWatcher' in window))
+    .run('should push and pop history state', async () => {
+      historyMock.expects('push').resolves('H1').once();
+      historyMock.expects('pop').withArgs('H1').once();
+      const watcher = new CloseWatcherImpl(ampdoc, handler);
+      await Promise.resolve('H1');
+      watcher.requestClose();
+      expect(handler).to.be.calledOnce;
+    });
 
-  // TODO(#40214): fix flaky test.
-  it.skip('should trigger on ESC key', async () => {
-    historyMock.expects('push').resolves('H1').once();
-    historyMock.expects('pop').withArgs('H1').once();
-    new CloseWatcherImpl(ampdoc, handler);
-    await Promise.resolve('H1');
+  it.configure()
+    .if(() => !('CloseWatcher' in window))
+    .run('should trigger on history pop', async () => {
+      let popHandler;
+      historyMock
+        .expects('push')
+        .withArgs(
+          env.sandbox.match((a) => {
+            popHandler = a;
+            return true;
+          })
+        )
+        .resolves('H1')
+        .once();
+      new CloseWatcherImpl(ampdoc, handler);
+      await Promise.resolve('H1');
+      expect(popHandler).to.exist;
+      popHandler();
+      expect(handler).to.be.calledOnce;
+    });
 
-    doc.documentElement.dispatchEvent(
-      new KeyboardEvent('keydown', {key: Keys_Enum.ESCAPE})
-    );
-    expect(handler).to.be.calledOnce;
-  });
+  it.configure()
+    .if(() => !('CloseWatcher' in window))
+    .run('should trigger on ESC key', async () => {
+      historyMock.expects('push').resolves('H1').once();
+      historyMock.expects('pop').withArgs('H1').once();
+      new CloseWatcherImpl(ampdoc, handler);
+      await Promise.resolve('H1');
+
+      doc.documentElement.dispatchEvent(
+        new KeyboardEvent('keydown', {key: Keys_Enum.ESCAPE})
+      );
+      expect(handler).to.be.calledOnce;
+    });
 });