Manifest v3 #54

tryasko · 2025-11-28T20:05:52Z

No description provided.

Copilot

Pull request overview

This PR migrates the Radio Wave Chrome extension from Manifest v2 to Manifest v3, a significant architectural change required for modern Chrome extensions. The migration replaces the persistent background page with a service worker, transitions from localStorage to chrome.storage.local for data persistence, and implements message-based communication patterns. Audio playback is moved to an offscreen document since service workers cannot directly play audio.

Key changes:

Architecture shift: Background page replaced with service worker + offscreen document pattern
Communication model: Direct function calls replaced with chrome.runtime.sendMessage-based messaging
Storage migration: localStorage replaced with chrome.storage.local API

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
manifest.json	Updated to Manifest v3 with service worker, new permissions (storage, offscreen), and host_permissions
background/service-worker.js	New service worker handling message routing and state management via chrome.storage.local
background/offscreen.js	New offscreen document script for audio playback (service workers can't play audio)
background/offscreen.html	HTML container for offscreen audio player
background/player.js	Removed - replaced by service worker + offscreen document pattern
background/updater.js	Removed - migration logic moved into service worker's onInstalled handler
background/index.html	Removed - no longer needed with service worker architecture
popup/application.js	Refactored to use message passing for all background communication and local state management
shared/stations.js	Updated to support both window (popup/offscreen) and self (service worker) contexts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-28T20:11:41Z

background/service-worker.js

+      case 'GET_STATE':
+        const state = await chrome.storage.local.get(['state', 'volume', 'station']);
+        sendResponse(state);
+        break;


[nitpick] Missing default case: The switch statement doesn't handle unknown message types. Add a default case to log or handle unexpected message types, which can help with debugging and prevent silent failures.

Suggested change

break;

break;

default:

console.warn("Unknown message type received:", message.type, message);

sendResponse({ success: false, error: "Unknown message type" });

break;

Copilot · 2025-11-28T20:11:41Z

background/offscreen.js

+
+    case 'AUDIO_VOLUME':
+      audio.volume = message.volume;
+      break;


[nitpick] Missing default case: The switch statement doesn't handle unknown message types. Consider adding a default case to log unexpected messages for debugging purposes.

Suggested change

break;

break;

default:

console.warn('Unknown message type received:', message.type, message);

Copilot · 2025-11-28T20:11:42Z

popup/application.js

-  }
-})();
+// Initialize on load
+initializePopup();


Missing error handling: If initializePopup() fails (e.g., service worker not responding), the popup will not initialize properly and will remain in an unusable state. Wrap the call in a try-catch or add .catch() to provide fallback behavior or user feedback.

Suggested change

initializePopup();

initializePopup().catch((err) => {

// Provide fallback behavior or user feedback

// For example, display an error message in the popup

playList.innerHTML = '<li class="error">Failed to initialize popup. Please try again later.</li>';

console.error("Failed to initialize popup:", err);

});

Copilot · 2025-11-28T20:11:42Z

popup/application.js

+  await chrome.runtime.sendMessage({
+    type: 'SET_VOLUME',
+    volume: parseInt(volume)
+  });


Missing error handling: If the SET_VOLUME message fails, the volume slider will show an incorrect value that doesn't match the actual audio volume. Add error handling to revert the UI state on failure.

Copilot · 2025-11-28T20:11:42Z

background/service-worker.js

+        chrome.runtime.sendMessage({
+          type: 'AUDIO_PLAY',
+          url: playUrl,
+          volume: playData.volume / 100
+        });


Potential race condition: chrome.runtime.sendMessage is called without ensuring the offscreen document is fully ready to receive messages. The offscreen document creation is async, but there's no guarantee it has loaded and registered its message listener before this message is sent. Consider adding a small delay or a handshake mechanism to ensure the offscreen document is ready.

Copilot · 2025-11-28T20:11:43Z

popup/application.js

+  const stationId = element.getAttribute("data-id");
+  currentState.station = stationId;
+  currentState.state = "played";


State synchronization issue: Setting currentState.station and currentState.state before the message operations complete (lines 88-93) can lead to inconsistent state if the operations fail. The local state should only be updated after verifying the operations succeeded.

Copilot · 2025-11-28T20:11:43Z

background/service-worker.js

+chrome.runtime.onInstalled.addListener(async () => {
+  const data = await chrome.storage.local.get(['version', 'volume', 'station', 'state']);
+
+  if (!data.version || data.version !== currentVersion) {


[nitpick] Logic issue: The condition checks if data.version exists before comparing it to currentVersion, but if this is a fresh install, data.version will be undefined and the check will proceed to verify if a station exists using data.station, which will also be undefined. The some() comparison will always fail for undefined, but it would be clearer to explicitly handle the fresh install case separately.

Suggested change

if (!data.version || data.version !== currentVersion) {

if (typeof data.version === "undefined") {

// Fresh install: set default values

await chrome.storage.local.set({

version: currentVersion,

volume: 30,

state: "paused",

station: "TVR.KissFM"

});

} else if (data.version !== currentVersion) {

// Upgrade: preserve station if it exists in the new station list

Copilot · 2025-11-28T20:11:43Z

popup/application.js

+  await chrome.runtime.sendMessage({
+    type: 'SET_STATION',
+    station: stationId
+  });

-// Render station list
-(() => {
-  controlPlay.setAttribute("class", localStorage.state);
-  controlVolume.value = localStorage.volume;
-
-  playList.innerHTML = window.stationList
-    .map(({ name, group, station }) => {
-      const stationId = getStationId(group, station);
-
-      return `<li class="${localStorage.station === stationId ? "selected" : ""}" data-id="${stationId}">
-          <span class="group">${group}</span>
-          <span class="name">${name}</span>
-        </li>`;
-    })
-    .join("");
+  await chrome.runtime.sendMessage({ type: 'PLAY' });


Missing error handling: The message sending operations (lines 88-93) should include error handling. If either SET_STATION or PLAY fails, the UI will be in an inconsistent state showing the station as selected and playing when it may not be.

Copilot · 2025-11-28T20:11:44Z

popup/application.js

+  await chrome.runtime.sendMessage({
+    type: 'SET_VOLUME',
+    volume: parseInt(volume)
+  });


Missing error handling: If the SET_VOLUME message fails, the volume slider and state will be inconsistent. Add error handling to revert the UI on failure.

Copilot · 2025-11-28T20:11:44Z

background/offscreen.js

+    case 'AUDIO_PLAY':
+      audio.volume = message.volume || 0.3;
+      audio.src = message.url;
+      audio.play().catch(err => console.error('Audio play error:', err));


Incomplete error handling: audio.play() returns a Promise that can be rejected, but the error is only logged to console. Consider notifying the service worker of playback failures so it can update the state to "paused" and inform the user, rather than leaving the UI in an inconsistent "played" state when audio actually failed.

Suggested change

audio.play().catch(err => console.error('Audio play error:', err));

audio.play().catch(err => {

console.error('Audio play error:', err);

chrome.runtime.sendMessage({

type: 'AUDIO_PLAY_FAILED',

error: err && err.message ? err.message : String(err),

url: message.url

});

});

tryasko added 2 commits November 28, 2025 20:48

Bump extension to support manifest v3

c1bd74c

Remove redundant local variable

043165f

tryasko requested a review from Copilot November 28, 2025 20:06

Copilot started reviewing on behalf of tryasko November 28, 2025 20:06 View session

Copilot finished reviewing on behalf of tryasko November 28, 2025 20:09

Copilot AI reviewed Nov 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Manifest v3 #54

Manifest v3 #54

Uh oh!

tryasko commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        break;
+        break;
+      default:
+        console.warn("Unknown message type received:", message.type, message);
+        sendResponse({ success: false, error: "Unknown message type" });
+        break;

-initializePopup();
+initializePopup().catch((err) => {
+  // Provide fallback behavior or user feedback
+  // For example, display an error message in the popup
+  playList.innerHTML = '<li class="error">Failed to initialize popup. Please try again later.</li>';
+  console.error("Failed to initialize popup:", err);
+});

-  if (!data.version || data.version !== currentVersion) {
+  if (typeof data.version === "undefined") {
+    // Fresh install: set default values
+    await chrome.storage.local.set({
+      version: currentVersion,
+      volume: 30,
+      state: "paused",
+      station: "TVR.KissFM"
+    });
+  } else if (data.version !== currentVersion) {
+    // Upgrade: preserve station if it exists in the new station list

-      audio.play().catch(err => console.error('Audio play error:', err));
+      audio.play().catch(err => {
+        console.error('Audio play error:', err);
+        chrome.runtime.sendMessage({
+          type: 'AUDIO_PLAY_FAILED',
+          error: err && err.message ? err.message : String(err),
+          url: message.url
+        });
+      });

Manifest v3 #54

Are you sure you want to change the base?

Manifest v3 #54

Uh oh!

Conversation

tryasko commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants