Code cleanup

Author: mgurgel

injected/src/features.js

@@ -33,8 +33,16 @@ const otherFeatures = /** @type {const} */ ([
 /** @typedef {baseFeatures[number]|otherFeatures[number]} FeatureName */
 /** @type {Record<string, FeatureName[]>} */
 export const platformSupport = {
-    apple: ['webCompat', 'duckPlayerNative',...baseFeatures],
-    'apple-isolated': ['duckPlayer', 'duckPlayerNative', 'brokerProtection', 'performanceMetrics', 'clickToLoad', 'messageBridge', 'favicon'],
+    apple: ['webCompat', 'duckPlayerNative', ...baseFeatures],
+    'apple-isolated': [
+        'duckPlayer',
+        'duckPlayerNative',
+        'brokerProtection',
+        'performanceMetrics',
+        'clickToLoad',
+        'messageBridge',
+        'favicon',
+    ],
     android: [...baseFeatures, 'webCompat', 'breakageReporting', 'duckPlayer', 'messageBridge'],
     'android-broker-protection': ['brokerProtection'],
     'android-autofill-password-import': ['autofillPasswordImport'],

injected/src/features/duck-player-native.js

@@ -1,7 +1,6 @@
 import ContentFeature from '../content-feature.js';
 import { isBeingFramed } from '../utils.js';
 import { DuckPlayerNativeMessages } from './duckplayer-native/messages.js';
-import { mockTransport } from './duckplayer-native/mock-transport.js';
 import { setupDuckPlayerForNoCookie, setupDuckPlayerForSerp, setupDuckPlayerForYouTube } from './duckplayer-native/duckplayer-native.js';
 import { Environment } from './duckplayer-native/environment.js';
 
@@ -39,18 +38,13 @@ export class DuckPlayerNativeFeature extends ContentFeature {
 
         const locale = args?.locale || args?.language || 'en';
         const env = new Environment({
-            debug: this.isDebug || true, // TODO: REMOVE
+            debug: this.isDebug,
             injectName: import.meta.injectName,
             platform: this.platform,
             locale,
         });
 
-        if (env.isIntegrationMode()) {
-            // TODO: Better way than patching transport?
-            this.messaging.transport = mockTransport();
-        }
-
-        const messages = new DuckPlayerNativeMessages(this.messaging);
+        const messages = new DuckPlayerNativeMessages(this.messaging, env);
         messages.subscribeToURLChange(({ pageType }) => {
             const playbackPaused = false; // TODO: Get this from the native notification too?
             console.log('GOT PAGE TYPE', pageType);
@@ -60,7 +54,6 @@ export class DuckPlayerNativeFeature extends ContentFeature {
         /** @type {InitialSettings} */
         let initialSetup;
 
-        // TODO: This seems to get initted twice. Check with Daniel
         try {
             initialSetup = await messages.initialSetup();
         } catch (e) {

injected/src/features/duckplayer-native/duckplayer-native.js

@@ -18,7 +18,6 @@ import { Logger, SideEffects } from './util.js';
  * @typedef {(SideEffects, Logger) => void} CustomEventHandler
  * @typedef {DuckPlayerNativeSettings['selectors']} DuckPlayerNativeSelectors
  */
-// TODO: Abort controller?
 
 export class DuckPlayerNativePage {
     /** @type {SideEffects} */
@@ -49,8 +48,6 @@ export class DuckPlayerNativePage {
             throw new Error('Missing arguments');
         }
 
-        console.log('SELECTORS', selectors);
-
         this.setupLogger();
 
         this.onLoad = onLoad || (() => {});
@@ -108,7 +105,6 @@ export class DuckPlayerNativePage {
  */
 export function setupDuckPlayerForYouTube(selectors, playbackPaused, environment, messages) {
     const mediaControlHandler = (sideEffects, logger, pause) => {
-        console.log('MEDIA CONTROL', pause); // TODO: Remove
         logger.log('Running media control handler. Pause:', pause);
 
         const videoElement = selectors?.videoElement;
@@ -147,15 +143,13 @@ export function setupDuckPlayerForYouTube(selectors, playbackPaused, environment
         });
 
         if (playbackPaused) {
-            console.log('PAUSING VIDEO');
             mediaControlHandler(sideEffects, logger, !!playbackPaused);
         }
     };
 
     const onInit = (sideEffects, logger) => {
         sideEffects.add('subscribe to media control', () => {
             return messages.subscribeToMediaControl(({ pause }) => {
-                console.log('GOT MC SUB');
                 mediaControlHandler(sideEffects, logger, pause);
             });
         });
@@ -247,7 +241,6 @@ export function setupDuckPlayerForNoCookie(selectors, environment, messages) {
  */
 export function setupDuckPlayerForSerp(selectors, environment, messages) {
     const onLoad = () => {
-        console.log('SERP NOTIFY');
         serpNotify();
     };
 

injected/src/features/duckplayer-native/environment.js

@@ -1,4 +1,3 @@
-// TODO: Update locales
 import strings from '../../../../build/locales/duckplayer-locales.js';
 
 export class Environment {

injected/src/features/duckplayer-native/get-current-timestamp.js

@@ -5,41 +5,31 @@
  * @returns {number}
  */
 export function getCurrentTimestamp(selector) {
-    const video = /** @type {HTMLVideoElement|null} */ (document.querySelector("video")); // TODO: Return to remote config
+    const video = /** @type {HTMLVideoElement|null} */ (document.querySelector(selector));
     return video?.currentTime || 0;
 }
 
 /**
- * Sends the timestamp to the browser every 300ms
- * TODO: Can we not brute force this?
- */
-/**
- * Sends the timestamp to the browser every 300ms
- * TODO: Can we not brute force this?
+ * Sends the timestamp to the browser at an interval
+ *
  * @param {number} interval - Polling interval
  * @param {(number) => void} callback - Callback handler for polling event
  * @param {DuckPlayerNativeSelectors} selectors - Selectors for the player
  * @returns
  */
 export function pollTimestamp(interval = 300, callback, selectors) {
-    window.addEventListener("message", function(event) {
-        console.log('MESSAGE', event);
-    });
     if (!callback || !selectors) {
         console.error('Timestamp polling failed. No callback or selectors defined');
+        return;
     }
 
     const isShowingAd = () => {
         return selectors.adShowing && !!document.querySelector(selectors.adShowing);
     };
 
     const timestampPolling = setInterval(() => {
-        if (isShowingAd()) {
-            console.log('Ad showing. Not polling timestamp');
-            return;
-        }
+        if (isShowingAd()) return;
         const timestamp = getCurrentTimestamp(selectors.videoElement);
-        console.log('Polling timestamp', timestamp);
         callback(timestamp);
     }, interval);
 

injected/src/features/duckplayer-native/media-control.js

@@ -1,6 +1,8 @@
 import * as constants from './constants.js';
+import { mockTransport } from './mock-transport.js';
 
 /** @import {YouTubeError} from './error-detection.js' */
+/** @import {Environment} from './environment.js' */
 
 /**
  * @typedef {object} MuteSettings - Settings passed to the onMute callback
@@ -32,20 +34,19 @@ import * as constants from './constants.js';
 export class DuckPlayerNativeMessages {
     /**
      * @param {Messaging} messaging
+     * @param {Environment} environment
      * @internal
      */
-    constructor(messaging) {
+    constructor(messaging, environment) {
         /**
          * @internal
          */
         this.messaging = messaging;
-        // this.environment = environment;
-        // TODO: Replace with class if needed
-        this.environment = {
-            isIntegrationMode: function () {
-                return true;
-            },
-        };
+        this.environment = environment;
+
+        if (this.environment.isIntegrationMode()) {
+            this.messaging.transport = mockTransport(); // TODO: Better way than patching transport?
+        }
     }
 
     /**
@@ -84,7 +85,6 @@ export class DuckPlayerNativeMessages {
      * @param {(urlSettings: UrlChangeSettings) => void} callback
      */
     subscribeToURLChange(callback) {
-        console.log('SUBSCRIBING TO URL CHANGE');
         return this.messaging.subscribe(constants.MSG_NAME_URL_CHANGE, callback);
     }
 
@@ -100,23 +100,20 @@ export class DuckPlayerNativeMessages {
      * Notifies browser that the feature is ready
      */
     notifyFeatureIsReady() {
-        console.log('FIRING', constants.MSG_NAME_FEATURE_READY); // TODO: Remove
         this.messaging.notify(constants.MSG_NAME_FEATURE_READY, {});
     }
 
     /**
      * Notifies browser that scripts are ready to be acalled
      */
     notifyScriptIsReady() {
-        console.log('FIRING', constants.MSG_NAME_SCRIPTS_READY); // TODO: Remove
         this.messaging.notify(constants.MSG_NAME_SCRIPTS_READY, {});
     }
 
     /**
      * Notifies browser that the overlay was dismissed
      */
     notifyOverlayDismissed() {
-        console.log('FIRING', constants.MSG_NAME_DISMISS_OVERLAY); // TODO: Remove
         this.messaging.notify(constants.MSG_NAME_DISMISS_OVERLAY, {});
     }
 }

injected/src/features/duckplayer-native/messages.js

@@ -68,7 +68,6 @@ class TestTransport {
                 break;
             case constants.MSG_NAME_URL_CHANGE:
                 response = { pageType: this.getPageType() };
-                console.log('PAGE TYPE', response);
                 timeout = 100;
                 break;
         }

injected/src/features/duckplayer-native/mock-transport.js

@@ -1,5 +1,3 @@
-// TODO: This was copied from Duck Player. May need a cleanup
-
 /* eslint-disable promise/prefer-await-to-then */
 /**
  * Add an event listener to an element that is only executed if it actually comes from a user action