Skip to content

Commit

Permalink
shortcuts: Use OnAcceleratorsUpdated
Browse files Browse the repository at this point in the history
OnAcceleratorsUpdated is an observer method that gets called by
the browser mojo remote. This change will update the accelerators
whenever there is an observerable event related to changing
accelerators in the backend.

Bug: b:216049298
Test: browser_tests
Change-Id: I9ef699b1905baabdc4a9547eba4ed99cb715c1d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4257274
Reviewed-by: Michael Checo <michaelcheco@google.com>
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108501}
  • Loading branch information
Jimmy authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 2e3b663 commit 567b135
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ export class AcceleratorLookupManager {
}

setAcceleratorLookup(acceleratorConfig: MojoAcceleratorConfig): void {
// Reset the lookup maps every time we update the accelerator mappings.
this.reverseAcceleratorLookup.clear();
this.standardAcceleratorLookup.clear();

for (const [source, accelInfoMap] of Object.entries(acceleratorConfig)) {
// When calling Object.entries on an object with optional enum keys,
// TypeScript considers the values to be possibly undefined.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
// found in the LICENSE file.

import {FakeMethodResolver} from 'chrome://resources/ash/common/fake_method_resolver.js';
import {FakeObservables} from 'chrome://resources/ash/common/fake_observables.js';
import {assert} from 'chrome://resources/js/assert_ts.js';

import {AcceleratorsUpdatedObserverRemote} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';

import {AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';

Expand All @@ -12,8 +16,16 @@ import {AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoL
* Implements a fake version of the FakeShortcutProvider mojo interface.
*/

// Method names.
const ON_ACCELERATORS_UPDATED_METHOD_NAME =
'AcceleratorsUpdatedObserver_OnAcceleratorsUpdated';

export class FakeShortcutProvider implements ShortcutProviderInterface {
private methods: FakeMethodResolver;
private observables: FakeObservables = new FakeObservables();
private acceleratorsUpdatedRemote: AcceleratorsUpdatedObserverRemote|null =
null;
private acceleratorsUpdatedPromise: Promise<void>|null = null;

constructor() {
this.methods = new FakeMethodResolver();
Expand All @@ -27,6 +39,18 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
this.methods.register('removeAccelerator');
this.methods.register('restoreAllDefaults');
this.methods.register('restoreActionDefaults');
this.methods.register('addObserver');
this.registerObservables();
}

registerObservables(): void {
this.observables.register(ON_ACCELERATORS_UPDATED_METHOD_NAME);
}

// Disable all observers and reset provider to initial state.
reset(): void {
this.observables = new FakeObservables();
this.registerObservables();
}

getAcceleratorLayoutInfos(): Promise<{layoutInfos: MojoLayoutInfo[]}> {
Expand All @@ -43,8 +67,25 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
return this.methods.resolveMethod('isMutable');
}

// Return nothing because this method has a void return type.
addObserver(): void {}
addObserver(observer: AcceleratorsUpdatedObserverRemote): void {
this.acceleratorsUpdatedPromise = this.observe(
ON_ACCELERATORS_UPDATED_METHOD_NAME,
(config: MojoAcceleratorConfig) => {
observer.onAcceleratorsUpdated(config);
});
}

getAcceleratorsUpdatedPromiseForTesting(): Promise<void> {
assert(this.acceleratorsUpdatedPromise);
return this.acceleratorsUpdatedPromise;
}

// Set the value that will be retuned when `onAcceleratorsUpdated()` is
// called.
setFakeAcceleratorsUpdated(config: MojoAcceleratorConfig[]): void {
this.observables.setObservableData(
ON_ACCELERATORS_UPDATED_METHOD_NAME, config);
}

addUserAccelerator(): Promise<AcceleratorConfigResult> {
// Always return kSuccess in this fake.
Expand Down Expand Up @@ -96,4 +137,13 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
setFakeAcceleratorLayoutInfos(layoutInfos: MojoLayoutInfo[]): void {
this.methods.setResult('getAcceleratorLayoutInfos', {layoutInfos});
}

// Sets up an observer for methodName.
private observe(methodName: string, callback: (T: any) => void):
Promise<void> {
return new Promise((resolve) => {
this.observables.observe(methodName, callback);
resolve();
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ import '../strings.m.js';
import '../css/shortcut_customization_shared.css.js';
import 'chrome://resources/ash/common/navigation_view_panel.js';
import 'chrome://resources/ash/common/page_toolbar.js';
import 'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js';
import 'chrome://resources/polymer/v3_0/iron-icon/iron-icon.js';

import {NavigationViewPanelElement} from 'chrome://resources/ash/common/navigation_view_panel.js';
import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/interfaces.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {AcceleratorsUpdatedObserverInterface, AcceleratorsUpdatedObserverReceiver} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';

import {AcceleratorEditDialogElement} from './accelerator_edit_dialog.js';
import {RequestUpdateAcceleratorEvent} from './accelerator_edit_view.js';
import {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
Expand Down Expand Up @@ -48,7 +51,8 @@ declare global {
const ShortcutCustomizationAppElementBase = I18nMixin(PolymerElement);

export class ShortcutCustomizationAppElement extends
ShortcutCustomizationAppElementBase {
ShortcutCustomizationAppElementBase implements
AcceleratorsUpdatedObserverInterface {
static get is(): string {
return 'shortcut-customization-app';
}
Expand Down Expand Up @@ -96,6 +100,7 @@ export class ShortcutCustomizationAppElement extends
private shortcutProvider: ShortcutProviderInterface = getShortcutProvider();
private acceleratorlookupManager: AcceleratorLookupManager =
AcceleratorLookupManager.getInstance();
private acceleratorsUpdatedReceiver: AcceleratorsUpdatedObserverReceiver;

override connectedCallback(): void {
super.connectedCallback();
Expand All @@ -109,6 +114,7 @@ export class ShortcutCustomizationAppElement extends

override disconnectedCallback(): void {
super.disconnectedCallback();
this.acceleratorsUpdatedReceiver.$.close();
this.removeEventListener('show-edit-dialog', this.showDialog);
this.removeEventListener('edit-dialog-closed', this.onDialogClosed);
this.removeEventListener(
Expand All @@ -133,6 +139,18 @@ export class ShortcutCustomizationAppElement extends
this.acceleratorlookupManager.setAcceleratorLayoutLookup(layoutInfos);
// Notify pages to update their accelerators.
this.$.navigationPanel.notifyEvent('updateAccelerators');

// After fetching initial accelerators, start observing for any changes.
this.acceleratorsUpdatedReceiver =
new AcceleratorsUpdatedObserverReceiver(this);
this.shortcutProvider.addObserver(
this.acceleratorsUpdatedReceiver.$.bindNewPipeAndPassRemote());
}

// AcceleratorsUpdatedObserverInterface:
onAcceleratorsUpdated(config: MojoAcceleratorConfig): void {
this.acceleratorlookupManager.setAcceleratorLookup(config);
this.$.navigationPanel.notifyEvent('updateSubsections');
}

private addNavigationSelectors(layoutInfos: MojoLayoutInfo[]): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import 'chrome://webui-test/mojo_webui_test_support.js';

import {fakeAcceleratorConfig, fakeLayoutInfo} from 'chrome://shortcut-customization/js/fake_data.js';
import {FakeShortcutProvider} from 'chrome://shortcut-customization/js/fake_shortcut_provider.js';
import {AcceleratorConfigResult, AcceleratorSource, MojoLayoutInfo} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {AcceleratorsUpdatedObserverRemote} from 'chrome://shortcut-customization/mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
import {assertDeepEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';

suite('fakeShortcutProviderTest', function() {
Expand All @@ -20,6 +21,16 @@ suite('fakeShortcutProviderTest', function() {
provider = null;
});

// Fake class that overrides the `onAcceleratorsUpdated` function. This
// allows us to intercept the request send from the remote and validate
// the data received.
class FakeAcceleratorsUpdatedRemote extends
AcceleratorsUpdatedObserverRemote {
public override onAcceleratorsUpdated(config: MojoAcceleratorConfig) {
assertDeepEquals(fakeAcceleratorConfig, config);
}
}

function getProvider(): FakeShortcutProvider {
assertTrue(!!provider);
return provider as FakeShortcutProvider;
Expand Down Expand Up @@ -56,6 +67,17 @@ suite('fakeShortcutProviderTest', function() {
});
});

test('ObserveAcceleratorsUpdated', () => {
// Set the expected value to be returned when `onAcceleratorsUpdated()` is
// called.
getProvider().setFakeAcceleratorsUpdated([fakeAcceleratorConfig]);

const remote = new FakeAcceleratorsUpdatedRemote();
getProvider().addObserver(remote);
// Simulate `onAcceleratorsUpdated()` being called by an observer.
return getProvider().getAcceleratorsUpdatedPromiseForTesting();
});

test('IsMutableDefaultFake', () => {
// TODO(jimmyxgong): Remove this test once real data is ready.
// AcceleratorSource.kAsh is a mutable source.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import {AcceleratorLookupManager} from 'chrome://shortcut-customization/js/accel
import {AcceleratorRowElement} from 'chrome://shortcut-customization/js/accelerator_row.js';
import {AcceleratorSubsectionElement} from 'chrome://shortcut-customization/js/accelerator_subsection.js';
import {AcceleratorViewElement} from 'chrome://shortcut-customization/js/accelerator_view.js';
import {setShortcutProviderForTesting, setupFakeShortcutProvider} from 'chrome://shortcut-customization/js/mojo_interface_provider.js';
import {fakeAcceleratorConfig, fakeLayoutInfo} from 'chrome://shortcut-customization/js/fake_data.js';
import {FakeShortcutProvider} from 'chrome://shortcut-customization/js/fake_shortcut_provider.js';
import {setShortcutProviderForTesting} from 'chrome://shortcut-customization/js/mojo_interface_provider.js';
import {ShortcutCustomizationAppElement} from 'chrome://shortcut-customization/js/shortcut_customization_app.js';
import {AcceleratorCategory, AcceleratorSubcategory, LayoutInfo, Modifier, ShortcutProviderInterface} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {AcceleratorCategory, AcceleratorSubcategory, LayoutInfo, Modifier} from 'chrome://shortcut-customization/js/shortcut_types.js';
import {getCategoryNameStringId, getSubcategoryNameStringId} from 'chrome://shortcut-customization/js/shortcut_utils.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {flushTasks, waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';
Expand All @@ -37,15 +39,25 @@ suite('shortcutCustomizationAppTest', function() {

let manager: AcceleratorLookupManager|null = null;

let provider: ShortcutProviderInterface;
let provider: FakeShortcutProvider;

setup(() => {
manager = AcceleratorLookupManager.getInstance();
provider = setupFakeShortcutProvider();
provider = new FakeShortcutProvider();
provider.setFakeAcceleratorConfig(fakeAcceleratorConfig);
provider.setFakeAcceleratorLayoutInfos(fakeLayoutInfo);
// `onAcceleratorsUpdated` gets observed as soon as the layouts are
// initialized.
// TODO(jimmyxgong): Triggering the observer in tests is difficult
// with how Mojo handles union types, we will need to refactor
// the fake data to support the correct Mojo types for OnAceleratorsUpdated.
provider.setFakeAcceleratorsUpdated([fakeAcceleratorConfig]);

setShortcutProviderForTesting(provider);
});

teardown(() => {
provider.reset();
if (manager) {
manager.reset();
}
Expand Down

0 comments on commit 567b135

Please sign in to comment.