Skip to content

Commit

Permalink
os_languages_page: Use onLanguagePackStatusChanged instead of polling
Browse files Browse the repository at this point in the history
This event completely eliminates the need for polling, but we still need
to request the initial state of language pack statuses.

Bug: b:298128915
Change-Id: I407b52438d57129f41018abd57d2af9ccdf8e7db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942716
Commit-Queue: Michael Cui <mlcui@google.com>
Reviewed-by: Claudio M <claudiomagni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1211265}
  • Loading branch information
mlcui-corp authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent b35b100 commit 5ee8bbc
Showing 1 changed file with 31 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ const kArcImeLanguage = '_arc_ime_language_';
export const ACCESSIBILITY_COMMON_IME_ID =
'_ext_ime_egfdjlfmgnehecnclamagfafdccgfndpdictation';

// How often to poll language packs for pack updates.
const LANGUAGE_PACKS_POLLING_RATE_MS = 1000;

interface ModelArgs {
// Unused.
supportedLanguages: chrome.languageSettingsPrivate.Language[];
Expand Down Expand Up @@ -234,7 +231,6 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
Map<string, chrome.languageSettingsPrivate.InputMethod[]>;
private enabledInputMethodSet_: Set<string>;
private originalProspectiveUILanguage_?: string;
private languagePackPollIntervalId?: number = undefined;

// Bound methods.
// Instances of SettingsLanguagesElement below should be replaced with
Expand All @@ -254,6 +250,8 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
private boundOnInputMethodChanged_:
OmitThisParameter<SettingsLanguagesElement['onInputMethodChanged_']>|
null = null;
private boundOnLanguagePackStatusChanged_: OmitThisParameter<
SettingsLanguagesElement['onLanguagePackStatusChanged_']>|null = null;

override connectedCallback(): void {
super.connectedCallback();
Expand Down Expand Up @@ -331,12 +329,15 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
this.boundOnSpellcheckDictionariesChanged_);

if (loadTimeData.getBoolean('languagePacksInSettingsEnabled')) {
// Poll language packs now to prevent test flakiness.
// Poll language packs once to get the initial state of language pack
// statuses.
// Do so in the next microtask to prevent `connectedCallback()` from
// failing and stalling tests.
Promise.resolve().then(() => this.pollLanguagePacks_());
this.languagePackPollIntervalId = setInterval(
() => this.pollLanguagePacks_(), LANGUAGE_PACKS_POLLING_RATE_MS);
this.boundOnLanguagePackStatusChanged_ =
this.onLanguagePackStatusChanged_.bind(this);
this.inputMethodPrivate_.onLanguagePackStatusChanged.addListener(
this.boundOnLanguagePackStatusChanged_);
}

this.resolver_.resolve(undefined);
Expand Down Expand Up @@ -373,9 +374,10 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
.removeListener(this.boundOnSpellcheckDictionariesChanged_);
this.boundOnSpellcheckDictionariesChanged_ = null;
}

if (this.languagePackPollIntervalId !== undefined) {
clearInterval(this.languagePackPollIntervalId);
if (this.boundOnLanguagePackStatusChanged_) {
this.inputMethodPrivate_.onLanguagePackStatusChanged.removeListener(
this.boundOnLanguagePackStatusChanged_);
this.boundOnLanguagePackStatusChanged_ = null;
}
}

Expand Down Expand Up @@ -1315,6 +1317,17 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
return inputMethod.displayName;
}

private setLanguagePackStatus_(
id: string, status: chrome.inputMethodPrivate.LanguagePackStatus): void {
this.set(
['languages', 'inputMethods', 'imeLanguagePackStatus', id], status);
}

/**
* Manually poll language packs for the status of enabled input methods.
* Required to get the initial language pack status of input methods - further
* updates will be done via a listener to changes.
*/
private pollLanguagePacks_(): void {
if (!this.languages) {
return;
Expand All @@ -1323,18 +1336,18 @@ export class SettingsLanguagesElement extends SettingsLanguagesElementBase
for (const inputMethod of this.languages.inputMethods!.enabled) {
void this.inputMethodPrivate_.getLanguagePackStatus(inputMethod.id)
.then((status) => {
this.set(
[
'languages',
'inputMethods',
'imeLanguagePackStatus',
inputMethod.id,
],
status);
this.setLanguagePackStatus_(inputMethod.id, status);
});
}
}

private onLanguagePackStatusChanged_(
change: chrome.inputMethodPrivate.LanguagePackStatusChange): void {
for (const engineId of change.engineIds) {
this.setLanguagePackStatus_(engineId, change.status);
}
}

getImeLanguagePackStatus(id: string):
chrome.inputMethodPrivate.LanguagePackStatus {
// Safety: `LanguagesModel.inputMethods` is always defined on CrOS.
Expand Down

0 comments on commit 5ee8bbc

Please sign in to comment.