Commit 0d0342c1 authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Files app: Refactor externs for background page

This CL fixes some inconsistencies that become larger issues for running
with JS modules and within SWA container.

Issues:
1. The `window.background` was defined using `var background =`, however
for JS Modules this doesn't set in the global `window`.

2. The interface `FileBrowserBackground` is actually used by other apps
other than Files app, despite the prefix `FileBrowser`. Fix
by renaming the interface to `BackgroundBase` and in turn rename the
implementation to `BackgroundBaseImpl`.

3. The implementation `BackgroundBase` doesn't have a `@implements` to
its interface, which was confusingly named `FileBrowserBackground`. Fix
by adding the `@implements` accordingly.

4. The methods `ready()` and `setLauncherHandler()` were the defined in
the wrong interfaces.  Fix by moving `ready()` to
`FileBrowserBackgroundFull` because it's used only in Files app and move
`setLauncherHandler()` to `BackgroundBase` because it's used in all
apps.

No change in behavior, Closure type-checking and current tests cover all
changes.

Bug: 1148545, 1133186, 1113981
Change-Id: I4ca90e826faf88b7def3b12d89ae249c091ec973
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2534551
Auto-Submit: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827162}
parent e03792f4
......@@ -3,7 +3,7 @@
// found in the LICENSE file.
// clang-format off
// #import {BackgroundBase} from '../../file_manager/background/js/background_base.m.js';
// #import {BackgroundBaseImpl} from '../../file_manager/background/js/background_base.m.js';
// #import {SingletonAppWindowWrapper} from '../../file_manager/background/js/app_window_wrapper.m.js';
// #import * as wrappedUtil from '../../file_manager/common/js/util.m.js'; const {util} = wrappedUtil;
// #import {FileType} from '../../file_manager/common/js/file_type.m.js';
......@@ -39,7 +39,7 @@ const audioPlayerCreateOptions = {
frame: {color: '#fafafa'}
};
class AudioPlayerBackground extends BackgroundBase {
class AudioPlayerBackground extends BackgroundBaseImpl {
constructor() {
super();
}
......@@ -62,8 +62,7 @@ class AudioPlayerBackground extends BackgroundBase {
* Backgound object. This is necessary for AppWindowWrapper.
* @type {!AudioPlayerBackground}
*/
// eslint-disable-next-line no-var
/* #export */ var background = new AudioPlayerBackground();
window.background = new AudioPlayerBackground();
/**
* Audio player app window wrapper.
......@@ -139,4 +138,4 @@ const audioPlayer = new SingletonAppWindowWrapper(
});
}
background.setLaunchHandler(open);
window.background.setLaunchHandler(open);
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//third_party/closure_compiler/compile_js.gni")
import("//ui/webui/resources/tools/js_modulizer.gni")
js_library("file_browser_background_full") {
sources = []
externs_list = [
"background_base.js",
"crostini.js",
"drive_sync_handler.js",
"duplicate_finder.js",
"file_browser_background_full.js",
"file_operation_manager.js",
"import_history.js",
"import_runner.js",
"media_import_handler.js",
"media_scanner.js",
"progress_center.js",
"../progress_center_panel.js",
"task_queue.js",
]
}
js_library("background_base.m") {
sources = [
"$root_gen_dir/ui/file_manager/externs/background/background_base.m.js",
]
deps = [ "..:volume_manager.m" ]
extra_deps = [ ":modulize" ]
}
js_modulizer("modulize") {
input_files = [ "background_base.js" ]
}
......@@ -2,16 +2,30 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
/** @typedef {function(!Array<string>):!Promise} */
/* #export */ let LaunchHandler;
/**
* Interface exposed in window.background in the background page. Used for
* Audio, Video and Gallery.
*
* Files app uses a larger interface: `FileBrowserBackgroundFull`.
* Interface exposed in window.background in the background page. Used for
* Audio, Video and Gallery.
*
* Files app uses a larger interface: `FileBrowserBackgroundFull`.
*
* @interface
*/
class FileBrowserBackground {
/* #export */ class BackgroundBase {
constructor() {
/** @type {!Object<!Window>} */
this.dialogs;
}
/**
* @param {function()} callback
* Set a handler which is called when an app is launched.
* @param {!LaunchHandler} handler Function to be called.
*/
ready(callback) {}
setLaunchHandler(handler) {}
}
......@@ -5,7 +5,7 @@
/**
* @interface
*/
class FileBrowserBackgroundFull extends FileBrowserBackground {
class FileBrowserBackgroundFull extends BackgroundBase {
constructor() {
/**
* @type {!DriveSyncHandler}
......@@ -48,4 +48,13 @@ class FileBrowserBackgroundFull extends FileBrowserBackground {
*/
this.crostini;
}
/**
* Register callback to be invoked after initialization of the background
* page. If the initialization is already done, the callback is invoked
* immediately.
*
* @param {function()} callback
*/
ready(callback) {}
}
......@@ -9,7 +9,13 @@
class BackgroundWindow {
constructor() {
/**
* @type {FileBrowserBackground}
* For File Manager it uses FileBrowserBackgroundFull.
* For all other apps it uses BackgroundBase.
*
* TODO(crbug.com/1148545): Add `FileBrowserBackgroundFull` together with
* `BackgroundBase` below.
*
* @type {!BackgroundBase}
*/
this.background;
......
......@@ -110,7 +110,7 @@ js_library("closure_compile_externs") {
"$externs_path/metrics_private.js",
"//ui/file_manager/externs/background/crostini.js",
"//ui/file_manager/externs/background/drive_sync_handler.js",
"//ui/file_manager/externs/background/file_browser_background.js",
"//ui/file_manager/externs/background/background_base.js",
"//ui/file_manager/externs/background/file_browser_background_full.js",
"//ui/file_manager/externs/background/file_operation_manager.js",
"//ui/file_manager/externs/background/import_history.js",
......@@ -195,6 +195,7 @@ js_library("background_base") {
"//ui/webui/resources/js:assert",
"//ui/webui/resources/js:load_time_data",
]
externs_list = [ "//ui/file_manager/externs/background/background_base.js" ]
}
js_library("background_base.m") {
......@@ -202,6 +203,7 @@ js_library("background_base.m") {
sources = [ "$root_gen_dir/ui/file_manager/file_manager/background/js/background_base.m.js" ]
deps = [
":volume_manager_factory.m",
"//ui/file_manager/externs/background:background_base.m",
"//ui/file_manager/file_manager/common/js:util.m",
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:load_time_data.m",
......@@ -583,7 +585,7 @@ js_library("runtime_loaded_test_util") {
"//ui/file_manager/externs/background/file_operation_manager.js",
"//ui/file_manager/externs/background/import_history.js",
"//ui/file_manager/externs/background/import_runner.js",
"//ui/file_manager/externs/background/file_browser_background.js",
"//ui/file_manager/externs/background/background_base.js",
"//ui/file_manager/externs/background/file_browser_background_full.js",
]
}
......
......@@ -6,7 +6,7 @@
* Root class of the background page.
* @implements {FileBrowserBackgroundFull}
*/
class FileBrowserBackgroundImpl extends BackgroundBase {
class FileBrowserBackgroundImpl extends BackgroundBaseImpl {
constructor() {
super();
this.setLaunchHandler(this.launch_);
......@@ -306,7 +306,7 @@ class FileBrowserBackgroundImpl extends BackgroundBase {
this.launch_(undefined);
return;
}
BackgroundBase.prototype.onLaunched_.apply(this, [launchData]);
BackgroundBaseImpl.prototype.onLaunched_.apply(this, [launchData]);
}
/**
......@@ -532,7 +532,7 @@ const GPLUS_PHOTOS_APP_ORIGIN =
/**
* Singleton instance of Background object.
* @type {!FileBrowserBackgroundImpl}
* @type {!FileBrowserBackgroundFull}
*/
window.background = new FileBrowserBackgroundImpl();
......
......@@ -3,19 +3,18 @@
// found in the LICENSE file.
// clang-format off
// #import {BackgroundBase, LaunchHandler} from '../../../externs/background/background_base.m.js';
// #import * as wrappedVolumeManagerFactory from './volume_manager_factory.m.js'; const {volumeManagerFactory} = wrappedVolumeManagerFactory;
// #import * as wrappedUtil from '../../common/js/util.m.js'; const {util} = wrappedUtil;
// #import {assert} from 'chrome://resources/js/assert.m.js';
// #import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
// clang-format on
/** @typedef {function(!Array<string>):!Promise} */
let LaunchHandler;
/**
* Root class of the background page.
* @implements {BackgroundBase}
*/
/* #export */ class BackgroundBase {
/* #export */ class BackgroundBaseImpl {
constructor() {
/**
* Map of all currently open file dialogs. The key is an app ID.
......
......@@ -89,7 +89,7 @@ js_library("closure_compile_externs") {
"$externs_path/metrics_private.js",
"//ui/file_manager/externs/background/crostini.js",
"//ui/file_manager/externs/background/drive_sync_handler.js",
"//ui/file_manager/externs/background/file_browser_background.js",
"//ui/file_manager/externs/background/background_base.js",
"//ui/file_manager/externs/background/file_browser_background_full.js",
"//ui/file_manager/externs/background/file_operation_manager.js",
"//ui/file_manager/externs/background/import_runner.js",
......
......@@ -68,7 +68,7 @@ js_library("closure_compile_externs") {
"$externs_path/metrics_private.js",
"//ui/file_manager/externs/background/crostini.js",
"//ui/file_manager/externs/background/drive_sync_handler.js",
"//ui/file_manager/externs/background/file_browser_background.js",
"//ui/file_manager/externs/background/background_base.js",
"//ui/file_manager/externs/background/file_operation_manager.js",
"//ui/file_manager/externs/background/import_history.js",
"//ui/file_manager/externs/background_window.js",
......
......@@ -54,7 +54,7 @@ js_library("closure_compile_externs") {
"js/externs.js",
"$externs_path/command_line_private.js",
"$externs_path/metrics_private.js",
"//ui/file_manager/externs/background/file_browser_background.js",
"//ui/file_manager/externs/background/background_base.js",
"//ui/file_manager/externs/background/progress_center.js",
"//ui/file_manager/externs/progress_center_panel.js",
"//ui/file_manager/externs/background/crostini.js",
......
......@@ -15,12 +15,7 @@ let FILE_MANAGER_ROOT;
/** @type {!FileManagerTestDeps} */
let fileManager;
/**
* Declare as FileBrowserBackground rather than FileBrowserBackgroundFull to
* simplify deps. Individual fields such as progressCenter below can be
* defined as needed.
* @type {!FileBrowserBackground}}
*/
/** @type {!FileBrowserBackgroundFull} */
fileManager.fileBrowserBackground_;
/**
......@@ -33,5 +28,5 @@ class ProgressCenterPanel {}
/** @type {!ProgressCenter} */
fileManager.fileBrowserBackground_.progressCenter;
/** @type {!FileBrowserBackground} */
/** @type {!FileBrowserBackgroundFull} */
window.background;
......@@ -23,8 +23,7 @@ const windowCreateOptions = {
* Backgound object. This is necessary for AppWindowWrapper.
* @type {!BackgroundBase}
*/
// eslint-disable-next-line no-var
var background = new BackgroundBase();
window.background = new BackgroundBaseImpl();
/**
* Gallery app window wrapper.
......@@ -82,4 +81,4 @@ function openGalleryWindow(urls) {
});
}
background.setLaunchHandler(openGalleryWindow);
window.background.setLaunchHandler(openGalleryWindow);
......@@ -27,8 +27,7 @@ const windowCreateOptions = {
* Backgound object.
* @type {!BackgroundBase}
*/
// eslint-disable-next-line no-var
var background = new BackgroundBase();
window.background = new BackgroundBaseImpl();
/**
* Creates a unique windowId string. Each call increments the sequence number
......@@ -103,4 +102,4 @@ function openVideoPlayerWindow(urls) {
}.wrap());
}
background.setLaunchHandler(openVideoPlayerWindow);
window.background.setLaunchHandler(openVideoPlayerWindow);
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment