Skip to content

Commit

Permalink
[PM-3530][PM-8588] persist extension route history (#9556)
Browse files Browse the repository at this point in the history
Save the extension popup route history and restore it after closing and re-opening the popup.

---------

Co-authored-by: Justin Baur <[email protected]>
  • Loading branch information
willmartian and justindbaur authored Aug 12, 2024
1 parent b2db633 commit 295fb8f
Show file tree
Hide file tree
Showing 14 changed files with 356 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { UserId } from "@bitwarden/common/types/guid";
import { ButtonModule, I18nMockService } from "@bitwarden/components";

import { RegistrationCheckEmailIcon } from "../../../../../../libs/auth/src/angular/icons/registration-check-email.icon";
import { PopupRouterCacheService } from "../../../platform/popup/view-cache/popup-router-cache.service";

import { ExtensionAnonLayoutWrapperDataService } from "./extension-anon-layout-wrapper-data.service";
import {
Expand Down Expand Up @@ -145,7 +146,15 @@ const decorators = (options: {
],
}),
applicationConfig({
providers: [importProvidersFrom(RouterModule.forRoot(options.routes))],
providers: [
importProvidersFrom(RouterModule.forRoot(options.routes)),
{
provide: PopupRouterCacheService,
useValue: {
back() {},
} as Partial<PopupRouterCacheService>,
},
],
}),
];
};
Expand Down
11 changes: 11 additions & 0 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ import I18nService from "../platform/services/i18n.service";
import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service";
import { BackgroundPlatformUtilsService } from "../platform/services/platform-utils/background-platform-utils.service";
import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service";
import { PopupViewCacheBackgroundService } from "../platform/services/popup-view-cache-background.service";
import { BackgroundTaskSchedulerService } from "../platform/services/task-scheduler/background-task-scheduler.service";
import { ForegroundTaskSchedulerService } from "../platform/services/task-scheduler/foreground-task-scheduler.service";
import { BackgroundMemoryStorageService } from "../platform/storage/background-memory-storage.service";
Expand Down Expand Up @@ -371,6 +372,8 @@ export default class MainBackground {
private isSafari: boolean;
private nativeMessagingBackground: NativeMessagingBackground;

private popupViewCacheBackgroundService: PopupViewCacheBackgroundService;

constructor(public popupOnlyContext: boolean = false) {
// Services
const lockedCallback = async (userId?: string) => {
Expand Down Expand Up @@ -571,6 +574,10 @@ export default class MainBackground {
logoutCallback,
);

this.popupViewCacheBackgroundService = new PopupViewCacheBackgroundService(
this.globalStateProvider,
);

const migrationRunner = new MigrationRunner(
this.storageService,
this.logService,
Expand Down Expand Up @@ -1201,6 +1208,8 @@ export default class MainBackground {
await (this.i18nService as I18nService).init();
(this.eventUploadService as EventUploadService).init(true);

this.popupViewCacheBackgroundService.startObservingTabChanges();

if (this.popupOnlyContext) {
return;
}
Expand Down Expand Up @@ -1295,6 +1304,7 @@ export default class MainBackground {
}),
),
);
await this.popupViewCacheBackgroundService.clearState();
await this.accountService.switchAccount(userId);
await switchPromise;
// Clear sequentialized caches
Expand Down Expand Up @@ -1383,6 +1393,7 @@ export default class MainBackground {
this.vaultTimeoutSettingsService.clear(userBeingLoggedOut),
this.vaultFilterService.clear(),
this.biometricStateService.logout(userBeingLoggedOut),
this.popupViewCacheBackgroundService.clearState(),
/* We intentionally do not clear:
* - autofillSettingsService
* - badgeSettingsService
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BooleanInput, coerceBooleanProperty } from "@angular/cdk/coercion";
import { CommonModule, Location } from "@angular/common";
import { CommonModule } from "@angular/common";
import { Component, Input, Signal, inject } from "@angular/core";

import { JslibModule } from "@bitwarden/angular/jslib.module";
Expand All @@ -10,6 +10,8 @@ import {
TypographyModule,
} from "@bitwarden/components";

import { PopupRouterCacheService } from "../view-cache/popup-router-cache.service";

import { PopupPageComponent } from "./popup-page.component";

@Component({
Expand All @@ -19,6 +21,7 @@ import { PopupPageComponent } from "./popup-page.component";
imports: [TypographyModule, CommonModule, IconButtonModule, JslibModule, AsyncActionsModule],
})
export class PopupHeaderComponent {
private popupRouterCacheService = inject(PopupRouterCacheService);
protected pageContentScrolled: Signal<boolean> = inject(PopupPageComponent).isScrolled;

/** Background color */
Expand Down Expand Up @@ -46,8 +49,6 @@ export class PopupHeaderComponent {
**/
@Input()
backAction: FunctionReturningAwaitable = async () => {
this.location.back();
return this.popupRouterCacheService.back();
};

constructor(private location: Location) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
SectionComponent,
} from "@bitwarden/components";

import { PopupRouterCacheService } from "../view-cache/popup-router-cache.service";

import { PopupFooterComponent } from "./popup-footer.component";
import { PopupHeaderComponent } from "./popup-header.component";
import { PopupPageComponent } from "./popup-page.component";
Expand Down Expand Up @@ -334,6 +336,12 @@ export default {
{ useHash: true },
),
),
{
provide: PopupRouterCacheService,
useValue: {
back() {},
} as Partial<PopupRouterCacheService>,
},
],
}),
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import { Location } from "@angular/common";
import { Injectable, inject } from "@angular/core";
import {
ActivatedRouteSnapshot,
CanActivateFn,
NavigationEnd,
Router,
UrlSerializer,
} from "@angular/router";
import { filter, first, firstValueFrom, map, Observable, of, switchMap } from "rxjs";

import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { GlobalStateProvider } from "@bitwarden/common/platform/state";

import { POPUP_ROUTE_HISTORY_KEY } from "../../../platform/services/popup-view-cache-background.service";
import BrowserPopupUtils from "../browser-popup-utils";

/**
* Preserves route history when opening and closing the popup
*
* Routes marked with `doNotSaveUrl` will not be stored
**/
@Injectable({
providedIn: "root",
})
export class PopupRouterCacheService {
private router = inject(Router);
private state = inject(GlobalStateProvider).get(POPUP_ROUTE_HISTORY_KEY);
private location = inject(Location);

constructor() {
// init history with existing state
this.history$()
.pipe(first())
.subscribe((history) => history.forEach((location) => this.location.go(location)));

// update state when route change occurs
this.router.events
.pipe(
filter((event) => event instanceof NavigationEnd),
filter((_event: NavigationEnd) => {
const state: ActivatedRouteSnapshot = this.router.routerState.snapshot.root;

let child = state.firstChild;
while (child.firstChild) {
child = child.firstChild;
}

return !child?.data?.doNotSaveUrl ?? true;
}),
switchMap((event) => this.push(event.url)),
)
.subscribe();
}

history$(): Observable<string[]> {
return this.state.state$;
}

async setHistory(state: string[]): Promise<string[]> {
return this.state.update(() => state);
}

/** Get the last item from the history stack, or `null` if empty */
last$(): Observable<string | null> {
return this.history$().pipe(
map((history) => {
if (!history || history.length === 0) {
return null;
}
return history[history.length - 1];
}),
);
}

/**
* If in browser popup, push new route onto history stack
*/
private async push(url: string): Promise<boolean> {
if (!BrowserPopupUtils.inPopup(window) || url === (await firstValueFrom(this.last$()))) {
return;
}
await this.state.update((prevState) => (prevState == null ? [url] : prevState.concat(url)));
}

/**
* Navigate back in history
*/
async back() {
await this.state.update((prevState) => prevState.slice(0, -1));

const url = this.router.url;
this.location.back();
if (url !== this.router.url) {
return;
}

// if no history is present, fallback to vault page
await this.router.navigate([""]);
}
}

/**
* Redirect to the last visited route. Should be applied to root route.
*
* If `FeatureFlag.PersistPopupView` is disabled, do nothing.
**/
export const popupRouterCacheGuard = (() => {
const configService = inject(ConfigService);
const popupHistoryService = inject(PopupRouterCacheService);
const urlSerializer = inject(UrlSerializer);

return configService.getFeatureFlag$(FeatureFlag.PersistPopupView).pipe(
switchMap((featureEnabled) => {
if (!featureEnabled) {
return of(true);
}

return popupHistoryService.last$().pipe(
map((url: string) => {
if (!url) {
return true;
}

return urlSerializer.parse(url);
}),
);
}),
);
}) satisfies CanActivateFn;
113 changes: 113 additions & 0 deletions apps/browser/src/platform/popup/view-cache/popup-router-cache.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { Component } from "@angular/core";
import { TestBed } from "@angular/core/testing";
import { Router, UrlSerializer, UrlTree } from "@angular/router";
import { RouterTestingModule } from "@angular/router/testing";
import { mock } from "jest-mock-extended";
import { firstValueFrom, of } from "rxjs";

import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { GlobalStateProvider } from "@bitwarden/common/platform/state";
import { FakeGlobalStateProvider } from "@bitwarden/common/spec";

import { PopupRouterCacheService, popupRouterCacheGuard } from "./popup-router-cache.service";

const flushPromises = async () => await new Promise(process.nextTick);

@Component({ template: "" })
export class EmptyComponent {}

describe("Popup router cache guard", () => {
const configServiceMock = mock<ConfigService>();
const fakeGlobalStateProvider = new FakeGlobalStateProvider();

let testBed: TestBed;
let serializer: UrlSerializer;
let router: Router;

let service: PopupRouterCacheService;

beforeEach(async () => {
jest.spyOn(configServiceMock, "getFeatureFlag$").mockReturnValue(of(true));

testBed = TestBed.configureTestingModule({
imports: [
RouterTestingModule.withRoutes([
{ path: "a", component: EmptyComponent },
{ path: "b", component: EmptyComponent },
{
path: "c",
component: EmptyComponent,
data: { doNotSaveUrl: true },
},
]),
],
providers: [
{ provide: ConfigService, useValue: configServiceMock },
{ provide: GlobalStateProvider, useValue: fakeGlobalStateProvider },
],
});

await testBed.compileComponents();

router = testBed.inject(Router);
serializer = testBed.inject(UrlSerializer);

service = testBed.inject(PopupRouterCacheService);

await service.setHistory([]);
});

it("returns true if the history stack is empty", async () => {
const response = await firstValueFrom(
testBed.runInInjectionContext(() => popupRouterCacheGuard()),
);

expect(response).toBe(true);
});

it("redirects to the latest stored route", async () => {
await router.navigate(["a"]);
await router.navigate(["b"]);

const response = (await firstValueFrom(
testBed.runInInjectionContext(() => popupRouterCacheGuard()),
)) as UrlTree;

expect(serializer.serialize(response)).toBe("/b");
});

it("back method redirects to the previous route", async () => {
await router.navigate(["a"]);
await router.navigate(["b"]);

// wait for router events subscription
await flushPromises();

expect(await firstValueFrom(service.history$())).toEqual(["/a", "/b"]);

await service.back();

expect(await firstValueFrom(service.history$())).toEqual(["/a"]);
});

it("does not save ignored routes", async () => {
await router.navigate(["a"]);
await router.navigate(["b"]);
await router.navigate(["c"]);

const response = (await firstValueFrom(
testBed.runInInjectionContext(() => popupRouterCacheGuard()),
)) as UrlTree;

expect(serializer.serialize(response)).toBe("/b");
});

it("does not save duplicate routes", async () => {
await router.navigate(["a"]);
await router.navigate(["a"]);

await flushPromises();

expect(await firstValueFrom(service.history$())).toEqual(["/a"]);
});
});
Loading

0 comments on commit 295fb8f

Please sign in to comment.