Skip to content

Commit 999403f

Browse files
calixtemancalixteman
authored
Merge pull request #18579 from calixteman/cancel_ai_requests_2
Dispatch changes in prefs enableAltTextModelDownload and enableGuessAltText to the viewer (bug 1912024)
2 parents e099840 + 1d51c3e commit 999403f

File tree

6 files changed

+162
-51
lines changed

6 files changed

+162
-51
lines changed

‎src/display/editor/stamp.js

+20-6
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,11 @@ class StampEditor extends AnnotationEditor {
141141
this._uiManager.useNewAltTextFlow &&
142142
this.#bitmap
143143
) {
144-
// The alt-text dialog isn't opened but we still want to guess the alt
145-
// text.
146-
this.mlGuessAltText();
144+
try {
145+
// The alt-text dialog isn't opened but we still want to guess the alt
146+
// text.
147+
this.mlGuessAltText();
148+
} catch {}
147149
}
148150

149151
this.div.focus();
@@ -155,8 +157,11 @@ class StampEditor extends AnnotationEditor {
155157
}
156158

157159
const { mlManager } = this._uiManager;
158-
if (!mlManager || !(await mlManager.isEnabledFor("altText"))) {
159-
return null;
160+
if (!mlManager) {
161+
throw new Error("No ML.");
162+
}
163+
if (!(await mlManager.isEnabledFor("altText"))) {
164+
throw new Error("ML isn't enabled for alt text.");
160165
}
161166
const { data, width, height } =
162167
imageData ||
@@ -170,9 +175,18 @@ class StampEditor extends AnnotationEditor {
170175
channels: data.length / (width * height),
171176
},
172177
});
173-
if (!response || response.error || !response.output) {
178+
if (!response) {
179+
throw new Error("No response from the AI service.");
180+
}
181+
if (response.error) {
182+
throw new Error("Error from the AI service.");
183+
}
184+
if (response.cancel) {
174185
return null;
175186
}
187+
if (!response.output) {
188+
throw new Error("No valid response from the AI service.");
189+
}
176190
const altText = response.output;
177191
await this.setGuessedAltText(altText);
178192
if (updateAltTextData && !this.hasAltTextData()) {

‎web/app.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,7 @@ const PDFViewerApplication = {
404404
} else {
405405
eventBus = new EventBus();
406406
}
407-
if (this.mlManager) {
408-
this.mlManager.eventBus = eventBus;
409-
}
407+
this.mlManager?.setEventBus(eventBus, this._globalAbortController.signal);
410408
this.eventBus = eventBus;
411409

412410
this.overlayManager = new OverlayManager();

‎web/app_options.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,12 @@ const defaultOptions = {
193193
enableAltTextModelDownload: {
194194
/** @type {boolean} */
195195
value: true,
196-
kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
196+
kind: OptionKind.VIEWER + OptionKind.PREFERENCE + OptionKind.EVENT_DISPATCH,
197197
},
198198
enableGuessAltText: {
199199
/** @type {boolean} */
200200
value: true,
201-
kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
201+
kind: OptionKind.VIEWER + OptionKind.PREFERENCE + OptionKind.EVENT_DISPATCH,
202202
},
203203
enableHighlightEditor: {
204204
// We'll probably want to make some experiments before enabling this

‎web/firefoxcom.js

+80-13
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,15 @@ class FirefoxScripting {
307307
}
308308

309309
class MLManager {
310+
#abortSignal = null;
311+
310312
#enabled = null;
311313

314+
#eventBus = null;
315+
312316
#ready = null;
313317

314-
eventBus = null;
318+
#requestResolvers = null;
315319

316320
hasProgress = false;
317321

@@ -330,6 +334,32 @@ class MLManager {
330334
this.enableGuessAltText = enableGuessAltText;
331335
}
332336

337+
setEventBus(eventBus, abortSignal) {
338+
this.#eventBus = eventBus;
339+
this.#abortSignal = abortSignal;
340+
eventBus._on(
341+
"enablealttextmodeldownload",
342+
({ value }) => {
343+
if (this.enableAltTextModelDownload === value) {
344+
return;
345+
}
346+
if (value) {
347+
this.downloadModel("altText");
348+
} else {
349+
this.deleteModel("altText");
350+
}
351+
},
352+
{ signal: abortSignal }
353+
);
354+
eventBus._on(
355+
"enableguessalttext",
356+
({ value }) => {
357+
this.toggleService("altText", value);
358+
},
359+
{ signal: abortSignal }
360+
);
361+
}
362+
333363
async isEnabledFor(name) {
334364
return this.enableGuessAltText && !!(await this.#enabled?.get(name));
335365
}
@@ -339,16 +369,17 @@ class MLManager {
339369
}
340370

341371
async deleteModel(name) {
342-
if (name !== "altText") {
372+
if (name !== "altText" || !this.enableAltTextModelDownload) {
343373
return;
344374
}
345375
this.enableAltTextModelDownload = false;
346376
this.#ready?.delete(name);
347377
this.#enabled?.delete(name);
348-
await Promise.all([
349-
this.toggleService("altText", false),
350-
FirefoxCom.requestAsync("mlDelete", MLManager.#AI_ALT_TEXT_MODEL_NAME),
351-
]);
378+
await this.toggleService("altText", false);
379+
await FirefoxCom.requestAsync(
380+
"mlDelete",
381+
MLManager.#AI_ALT_TEXT_MODEL_NAME
382+
);
352383
}
353384

354385
async loadModel(name) {
@@ -358,7 +389,7 @@ class MLManager {
358389
}
359390

360391
async downloadModel(name) {
361-
if (name !== "altText") {
392+
if (name !== "altText" || this.enableAltTextModelDownload) {
362393
return null;
363394
}
364395
this.enableAltTextModelDownload = true;
@@ -369,18 +400,52 @@ class MLManager {
369400
if (data?.name !== "altText") {
370401
return null;
371402
}
403+
const resolvers = (this.#requestResolvers ||= new Set());
404+
const resolver = Promise.withResolvers();
405+
resolvers.add(resolver);
406+
372407
data.service = MLManager.#AI_ALT_TEXT_MODEL_NAME;
373-
return FirefoxCom.requestAsync("mlGuess", data);
408+
409+
FirefoxCom.requestAsync("mlGuess", data)
410+
.then(response => {
411+
if (resolvers.has(resolver)) {
412+
resolver.resolve(response);
413+
resolvers.delete(resolver);
414+
}
415+
})
416+
.catch(reason => {
417+
if (resolvers.has(resolver)) {
418+
resolver.reject(reason);
419+
resolvers.delete(resolver);
420+
}
421+
});
422+
423+
return resolver.promise;
424+
}
425+
426+
async #cancelAllRequests() {
427+
if (!this.#requestResolvers) {
428+
return;
429+
}
430+
for (const resolver of this.#requestResolvers) {
431+
resolver.resolve({ cancel: true });
432+
}
433+
this.#requestResolvers.clear();
434+
this.#requestResolvers = null;
374435
}
375436

376437
async toggleService(name, enabled) {
377-
if (name !== "altText") {
438+
if (name !== "altText" || this.enableGuessAltText === enabled) {
378439
return;
379440
}
380441

381442
this.enableGuessAltText = enabled;
382-
if (enabled && this.enableAltTextModelDownload) {
383-
await this.#loadAltTextEngine(false);
443+
if (enabled) {
444+
if (this.enableAltTextModelDownload) {
445+
await this.#loadAltTextEngine(false);
446+
}
447+
} else {
448+
this.#cancelAllRequests();
384449
}
385450
}
386451

@@ -403,7 +468,7 @@ class MLManager {
403468
if (listenToProgress) {
404469
this.hasProgress = true;
405470
const callback = ({ detail }) => {
406-
this.eventBus.dispatch("loadaiengineprogress", {
471+
this.#eventBus.dispatch("loadaiengineprogress", {
407472
source: this,
408473
detail,
409474
});
@@ -412,7 +477,9 @@ class MLManager {
412477
window.removeEventListener("loadAIEngineProgress", callback);
413478
}
414479
};
415-
window.addEventListener("loadAIEngineProgress", callback);
480+
window.addEventListener("loadAIEngineProgress", callback, {
481+
signal: this.#abortSignal,
482+
});
416483
promise.then(ok => {
417484
if (!ok) {
418485
this.hasProgress = false;

‎web/genericcom.js

+4
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ class FakeMLManager {
7979
this.enableAltTextModelDownload = enableAltTextModelDownload;
8080
}
8181

82+
setEventBus(eventBus, abortSignal) {
83+
this.eventBus = eventBus;
84+
}
85+
8286
async isEnabledFor(_name) {
8387
return this.enableGuessAltText;
8488
}

‎web/new_alt_text_manager.js

+55-27
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ class NewAltTextManager {
137137
this.#toggleDisclaimer();
138138
});
139139

140+
eventBus._on("enableguessalttext", ({ value }) => {
141+
this.#toggleGuessAltText(value, /* isInitial = */ false);
142+
});
143+
140144
this.#overlayManager.register(dialog);
141145
}
142146

@@ -247,13 +251,12 @@ class NewAltTextManager {
247251
this.#imageData,
248252
/* updateAltTextData = */ false
249253
);
250-
if (altText === null) {
251-
throw new Error("No valid response from the AI service.");
252-
}
253-
this.#guessedAltText = altText;
254-
this.#wasAILoading = this.#isAILoading;
255-
if (this.#isAILoading) {
256-
this.#addAltText(altText);
254+
if (altText) {
255+
this.#guessedAltText = altText;
256+
this.#wasAILoading = this.#isAILoading;
257+
if (this.#isAILoading) {
258+
this.#addAltText(altText);
259+
}
257260
}
258261
} catch (e) {
259262
console.error(e);
@@ -458,6 +461,8 @@ class ImageAltTextSettings {
458461

459462
#createModelButton;
460463

464+
#downloadModelButton;
465+
461466
#dialog;
462467

463468
#eventBus;
@@ -486,6 +491,7 @@ class ImageAltTextSettings {
486491
this.#dialog = dialog;
487492
this.#aiModelSettings = aiModelSettings;
488493
this.#createModelButton = createModelButton;
494+
this.#downloadModelButton = downloadModelButton;
489495
this.#showAltTextDialogButton = showAltTextDialogButton;
490496
this.#overlayManager = overlayManager;
491497
this.#eventBus = eventBus;
@@ -508,40 +514,62 @@ class ImageAltTextSettings {
508514
this.#togglePref.bind(this, "enableNewAltTextWhenAddingImage")
509515
);
510516

511-
deleteModelButton.addEventListener("click", async () => {
512-
await mlManager.deleteModel("altText");
517+
deleteModelButton.addEventListener("click", this.#delete.bind(this, true));
518+
downloadModelButton.addEventListener(
519+
"click",
520+
this.#download.bind(this, true)
521+
);
522+
523+
closeButton.addEventListener("click", this.#finish.bind(this));
513524

514-
aiModelSettings.classList.toggle("download", true);
515-
createModelButton.disabled = true;
516-
createModelButton.setAttribute("aria-pressed", false);
517-
this.#setPref("enableGuessAltText", false);
518-
this.#setPref("enableAltTextModelDownload", false);
525+
eventBus._on("enablealttextmodeldownload", ({ value }) => {
526+
if (value) {
527+
this.#download(false);
528+
} else {
529+
this.#delete(false);
530+
}
519531
});
520532

521-
downloadModelButton.addEventListener("click", async () => {
522-
downloadModelButton.disabled = true;
523-
downloadModelButton.firstChild.setAttribute(
533+
this.#overlayManager.register(dialog);
534+
}
535+
536+
async #download(isFromUI = false) {
537+
if (isFromUI) {
538+
this.#downloadModelButton.disabled = true;
539+
const span = this.#downloadModelButton.firstChild;
540+
span.setAttribute(
524541
"data-l10n-id",
525542
"pdfjs-editor-alt-text-settings-downloading-model-button"
526543
);
527544

528-
await mlManager.downloadModel("altText");
545+
await this.#mlManager.downloadModel("altText");
529546

530-
aiModelSettings.classList.toggle("download", false);
531-
downloadModelButton.firstChild.setAttribute(
547+
span.setAttribute(
532548
"data-l10n-id",
533549
"pdfjs-editor-alt-text-settings-download-model-button"
534550
);
535-
createModelButton.disabled = false;
536-
createModelButton.setAttribute("aria-pressed", true);
551+
552+
this.#createModelButton.disabled = false;
537553
this.#setPref("enableGuessAltText", true);
538-
mlManager.toggleService("altText", true);
554+
this.#mlManager.toggleService("altText", true);
539555
this.#setPref("enableAltTextModelDownload", true);
540-
downloadModelButton.disabled = false;
541-
});
556+
this.#downloadModelButton.disabled = false;
557+
}
542558

543-
closeButton.addEventListener("click", this.#finish.bind(this));
544-
this.#overlayManager.register(dialog);
559+
this.#aiModelSettings.classList.toggle("download", false);
560+
this.#createModelButton.setAttribute("aria-pressed", true);
561+
}
562+
563+
async #delete(isFromUI = false) {
564+
if (isFromUI) {
565+
await this.#mlManager.deleteModel("altText");
566+
this.#setPref("enableGuessAltText", false);
567+
this.#setPref("enableAltTextModelDownload", false);
568+
}
569+
570+
this.#aiModelSettings.classList.toggle("download", true);
571+
this.#createModelButton.disabled = true;
572+
this.#createModelButton.setAttribute("aria-pressed", false);
545573
}
546574

547575
async open({ enableGuessAltText, enableNewAltTextWhenAddingImage }) {

0 commit comments

Comments
 (0)