Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: chrome bugs #30

Merged
merged 4 commits into from
Apr 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions js/packages/extension/src/Connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,39 @@ import { PlayInfo } from '../../shared/types';
import { IncomingMessages, OutgoingMessages, ReconnectingWebsocket } from '../../shared/reconnecting-websocket';

type ConnectionActiveMessage = LegacyEventData | PlayInfo;
type Port = number | undefined | null;

export class Connection {
private sock?: ReconnectingWebsocket<
IncomingMessages,
OutgoingMessages<{ Active: ConnectionActiveMessage; Inactive: undefined }>
>;
private isLegacy = false;
private port = DEFAULT_CURRENT_PORT;
private port: Port = null;

private lastMessage: null | ConnectionActiveMessage = null;

constructor() {
// although this may not seem like it,
// this will call the callback at the start.
listenOption<boolean>(Option.UseLegacyApi, v => this.handleOptionChange(!!v, this.port));
listenOption<string>(Option.ApiPort, v =>
this.handleOptionChange(
this.isLegacy,
v ? Number(v) : this.isLegacy ? DEFAULT_LEGACY_PORT : DEFAULT_CURRENT_PORT,
),
);
listenOption<string>(Option.ApiPort, v => this.handleOptionChange(this.isLegacy, v ? Number(v) : null));
}

handleOptionChange(isLegacy: boolean, port: number) {
handleOptionChange(isLegacy: boolean, port: Port) {
const legacyChanged = this.isLegacy !== isLegacy;
const portChanged = this.port !== port;
if (!legacyChanged && !portChanged) return;
if (!legacyChanged && !portChanged && this.sock) return;

if (this.sock) this.sock.close();
if (legacyChanged) this.lastMessage = null;

this.port = port;
this.isLegacy = isLegacy;

const actualPort = this.port ?? (this.isLegacy ? DEFAULT_LEGACY_PORT : DEFAULT_CURRENT_PORT);
this.sock = new ReconnectingWebsocket(
this.isLegacy ? `ws://127.0.0.1:${this.port}` : formatLocalUrl('/api/ws/extension', this.port, 'ws'),
this.isLegacy ? `ws://127.0.0.1:${actualPort}` : formatLocalUrl('/api/ws/extension', actualPort, 'ws'),
);
this.sock.connect().then(() => {
if (this.lastMessage) {
Expand Down
10 changes: 7 additions & 3 deletions js/packages/extension/src/background/BrowserInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface IBrowserInterface {
addTabRemovedListener(cb: (tabId: TabId) => void): void;
addTabUpdatedListener(cb: (tabId: TabId) => void): void;
addTabActivatedListener(cb: (info: TabActivateInfo) => void): void;
addWindowFocusChangedListener(cb: (windowId: WindowId) => void): void;
addWindowFocusChangedListener(cb: (windowId: WindowId) => void): boolean;
addWindowRemovedListener(cb: (windowId: WindowId) => void): void;
}

Expand All @@ -35,9 +35,13 @@ export const DefaultBrowserInterface: IBrowserInterface = {
},
// do not use windows api on chrome see: https://bugs.chromium.org/p/chromium/issues/detail?id=387377
addWindowFocusChangedListener(cb) {
if (!isChromeLike()) browser.windows.onFocusChanged.addListener(cb);
if (isChromeLike()) {
return false;
}
browser.windows.onFocusChanged.addListener(cb);
return true;
},
addWindowRemovedListener(cb) {
if (!isChromeLike()) browser.windows.onRemoved.addListener(cb);
browser.windows.onRemoved.addListener(cb);
},
};
36 changes: 29 additions & 7 deletions js/packages/extension/src/background/TabManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ interface FindAndEmitOptions {
export class TabManager extends EventTarget {
/** This includes tabs that are audible OR have metadata */
readonly tabs = new Map<TabId, TabModel>();
/** Since Chrome doesn't send the `previousTabId` when a tab is activated, we need to track all windows ourselves. */
readonly knownWindows = new Map<WindowId, { activeTabId: TabId }>();

readonly blockedWindows = new Set<WindowId>();
activeWindowId: number | null = null;

sentTabId: number | null = null;

private shouldTrackActiveWindow = false;

private readonly updateCallback: (message: TabModel | null) => void;
private readonly filterManager: FilterManager;
private readonly browser: IBrowserInterface;
Expand Down Expand Up @@ -85,6 +89,9 @@ export class TabManager extends EventTarget {
// This won't throw since we know there's a valid tab-id
const model = new TabModel(tab);
this.tabs.set(tab.id, model);

if (tab.active) this.activateTabOnWindow(model.windowId, model.id);

return model;
}

Expand All @@ -95,12 +102,24 @@ export class TabManager extends EventTarget {
else this.blockedWindows.delete(window.id);
}

private activateTabOnWindow(windowId: WindowId, tabId: TabId) {
const info = this.knownWindows.get(windowId);
if (info) {
this.tabs.get(info.activeTabId)?.setActive(false);
info.activeTabId = tabId;
} else {
this.knownWindows.set(windowId, { activeTabId: tabId });
}

this.tabs.get(tabId)?.setActive(true);
}

private initListeners() {
this.browser.addTabCreatedListener(this.tabCreated.bind(this));
this.browser.addTabRemovedListener(this.tabRemoved.bind(this));
this.browser.addTabUpdatedListener(this.tabUpdated.bind(this));
this.browser.addTabActivatedListener(this.tabActivated.bind(this));
this.browser.addWindowFocusChangedListener(this.windowFocused.bind(this));
this.shouldTrackActiveWindow = this.browser.addWindowFocusChangedListener(this.windowFocused.bind(this));
this.browser.addWindowRemovedListener(this.windowRemoved.bind(this));
}

Expand All @@ -119,12 +138,9 @@ export class TabManager extends EventTarget {
}

private tabActivated(info: TabActivateInfo) {
this.activeWindowId = info.windowId;

if (info.previousTabId !== undefined) this.tabs.get(info.previousTabId)?.setActive(false);

this.tabs.get(info.tabId)?.setActive(true);
if (this.shouldTrackActiveWindow) this.activeWindowId = info.windowId;

this.activateTabOnWindow(info.windowId, info.tabId);
this.findAndEmitActiveTab();
}

Expand Down Expand Up @@ -156,6 +172,7 @@ export class TabManager extends EventTarget {
private windowRemoved(windowId: WindowId) {
if (this.activeWindowId === windowId) this.activeWindowId = null;
this.blockedWindows.delete(windowId);
this.knownWindows.delete(windowId);

// don't update tabs here since there will be separate events for them
}
Expand Down Expand Up @@ -184,7 +201,12 @@ export class TabManager extends EventTarget {
}

private isValidTab(tab: TabModel): boolean {
if (!this.filterManager.includeFocusedTabs && tab.active && this.activeWindowId === tab.windowId) return false;
if (
!this.filterManager.includeFocusedTabs &&
tab.active &&
(!this.shouldTrackActiveWindow || this.activeWindowId === tab.windowId)
)
return false;
if (!tab.audible || tab.muted) return false;
if (this.blockedWindows.has(tab.windowId)) return false;

Expand Down
4 changes: 3 additions & 1 deletion js/packages/extension/src/chrome-fix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
tryPromisify(browser.tabs, 'get');
tryPromisify(browser.windows, 'getCurrent');
tryPromisify(browser.windows, 'getAll');
tryPromisify(browser.storage.local, 'get');
tryPromisify(browser.storage.local, 'set');
})();

export function tryPromisify<K extends string, T extends { [x in K]: (arg: any) => Promise<any> }>(obj: T, key: K) {
Expand All @@ -14,6 +16,6 @@ export function tryPromisify<K extends string, T extends { [x in K]: (arg: any)
// assume this is chrome
const base = obj[key];
// @ts-ignore -- wrong types or something, this is fine
obj[key] = arg1 => new Promise(resolve => base(arg1, resolve));
obj[key] = arg1 => new Promise(resolve => base.call(obj, arg1, resolve));
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import '../chrome-fix';
import { ContentEventHandler } from 'beaverjs';

if (document.documentElement instanceof HTMLElement) {
Expand Down
1 change: 1 addition & 0 deletions js/packages/extension/src/content-scripts/youtube.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import '../chrome-fix';
import { ContentEventHandler } from 'beaverjs';
import { InternalMessageMap } from '../messages';
import { VideoPlayPosition } from '../types/video.types';
Expand Down
10 changes: 5 additions & 5 deletions js/packages/extension/src/options-page/PortInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class PortInput extends LitElement {
`;

@state()
private _port: string | undefined = undefined;
private _port: string | undefined | null = undefined;
@state()
private _useLegacyApi = false;

Expand Down Expand Up @@ -48,10 +48,10 @@ export class PortInput extends LitElement {

private _updatePort(e: Event) {
const target = e.target as HTMLInputElement;
if (typeof this._port === 'undefined' && target.value === this._getCurrentDefaultPort()) {
if (!this._port && target.value === this._getCurrentDefaultPort()) {
return;
}
this._port = target.value || undefined;
this._port = target.value || null;
this._trySetPort();
this.requestUpdate();
}
Expand All @@ -61,7 +61,7 @@ export class PortInput extends LitElement {
}

private _getActualPort() {
return typeof this._port === 'undefined' ? this._getCurrentDefaultPort() : this._port;
return this._port ?? this._getCurrentDefaultPort();
}

private _trySetPort() {
Expand All @@ -70,7 +70,7 @@ export class PortInput extends LitElement {
setOption(Option.ApiPort, Number(this._port)).catch(console.error);
}
} else {
setOption(Option.ApiPort, undefined).catch(console.error);
setOption(Option.ApiPort, null).catch(console.error);
}
}
}
2 changes: 2 additions & 0 deletions js/packages/extension/src/options-page/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import '../chrome-fix';

export * from './ToggleOption';
export * from './FilterList';
export * from './FilterModeToggle';
Expand Down
2 changes: 1 addition & 1 deletion js/packages/extension/test/background/TabManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ describe('TabManager', function () {
addTabRemovedListener: () => undefined,
addTabUpdatedListener: cb => (onTabUpdated = cb),
addTabActivatedListener: () => undefined,
addWindowFocusChangedListener: () => undefined,
addWindowFocusChangedListener: () => false,
addWindowRemovedListener: () => undefined,
};
const { nextUpdate } = mockTabManager({ browser, initialTabs: [invalidTab], initialWindows: [invalidWindow] });
Expand Down
6 changes: 5 additions & 1 deletion js/packages/extension/test/mock/browser-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ export function mockBrowser(initialWindows: BrowserWindow[] = []) {
addTabRemovedListener: (cb: (tabId: TabId) => void) => (onTabRemoved = cb),
addTabUpdatedListener: (cb: (tabId: TabId) => void) => (onTabUpdated = cb),
addTabActivatedListener: (cb: (info: TabActivateInfo) => void) => (onTabActivated = cb),
addWindowFocusChangedListener: (cb: (windowId: WindowId) => void) => (onWindowFocusChanged = cb),
addWindowFocusChangedListener: (cb: (windowId: WindowId) => void) => {
onWindowFocusChanged = cb;
// we can always listen to window events
return true;
},
addWindowRemovedListener: (cb: (windowId: WindowId) => void) => (onWindowRemoved = cb),
} as IBrowserInterface,
initialTabs: [...tabs.values()],
Expand Down
6 changes: 5 additions & 1 deletion js/packages/shared/reconnecting-websocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ export class ReconnectingWebsocket<EventMap extends MinEventMap, SendMap extends
}

public trySend<K extends keyof SendMap>(type: K, data: SendMap[K]) {
this.ws?.send(JSON.stringify({ type, data }));
try {
this.ws?.send(JSON.stringify({ type, data }));
} catch (e) {
console.warn('Error sending websocket message:', e);
}
}

public close() {
Expand Down