Skip to content

Commit

Permalink
App Management: Migrate some mojo bindings to TS
Browse files Browse the repository at this point in the history
- Migrating ui/webui/resources/cr_components/app_management mojo
  bindings to TS.
- Note: Not migrating dependency from components/ as the closure target
  is still used on ChromeOS Ash.
- Fixing use of relative paths within
  ui/webui/resources/cr_components/app_management; since this is
  compiled as a separate ts_library from ui/webui/resources, it should
  not use other ui/webui/resources files from relative paths.
- Fixing unnecessary copying of app_management.mojom-webui.js in
  chrome/browser/resources/app_settings/; this file is imported from
  chrome://resources/cr_components/app_management so copying it into
  the app settings ts_library is unnecessary.
- Also using one of the new TypeScript bindings in OS Settings. Since
  the os apps page is already in TypeScript, this makes sense for OS
  settings as well.

Bug: 1002798
Change-Id: I55a226eb6554ee2ad6bff80dd03f681990e6e9d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4103181
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084593}
  • Loading branch information
Rebekah Potter authored and Chromium LUCI CQ committed Dec 16, 2022
1 parent 528d481 commit d7709f5
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 55 deletions.
3 changes: 0 additions & 3 deletions chrome/browser/resources/app_settings/BUILD.gn
Expand Up @@ -16,9 +16,6 @@ build_webui("build") {

non_web_component_files = [ "web_app_settings.ts" ]

mojo_files_deps = [ "//ui/webui/resources/cr_components/app_management:mojo_bindings_js__generator" ]
mojo_files = [ "$root_gen_dir/mojom-webui/ui/webui/resources/cr_components/app_management/app_management.mojom-webui.js" ]

ts_composite = true
ts_deps = [
"//third_party/polymer/v3_0:library",
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/resources/app_settings/tsconfig_base.json
@@ -1,7 +1,6 @@
{
"extends": "../../../../tools/typescript/tsconfig_base.json",
"compilerOptions": {
"allowJs": true,
"noUnusedLocals": false,
"noUnusedParameters": false,
"strictPropertyInitialization": false
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/resources/settings/chromeos/BUILD.gn
Expand Up @@ -187,10 +187,8 @@ copy("generate_ash_search_mojo_webui") {
}

copy("generate_app_notification_mojo_webui") {
deps = [
"//chrome/browser/ui/webui/settings/ash/os_apps_page/mojom:mojom_webui_js",
]
sources = [ "$root_gen_dir/mojom-webui/chrome/browser/ui/webui/settings/ash/os_apps_page/mojom/app_notification_handler.mojom-webui.js" ]
deps = [ "//chrome/browser/ui/webui/settings/ash/os_apps_page/mojom:mojom_ts__generator" ]
sources = [ "$root_gen_dir/chrome/browser/ui/webui/settings/ash/os_apps_page/mojom/app_notification_handler.mojom-webui.ts" ]
outputs = [ "$tsc_input_dir/mojom-webui/os_apps_page/{{source_file_part}}" ]
}

Expand Down
Expand Up @@ -40,15 +40,17 @@ export class AppManagementBrowserProxy {
new FakePageHandler(this.callbackRouter.$.bindNewPipeAndPassRemote());
this.handler = this.fakeHandler.getRemote();

const permissionOptions: Record<PermissionType, PermissionOption> = {};
permissionOptions[PermissionType.kLocation] = {
permissionValue: TriState.kAllow,
isManaged: true,
};
permissionOptions[PermissionType.kCamera] = {
permissionValue: TriState.kBlock,
isManaged: true,
};
const permissionOptions:
Partial<Record<PermissionType, PermissionOption>> = {
[PermissionType.kLocation]: {
permissionValue: TriState.kAllow,
isManaged: true,
},
[PermissionType.kCamera]: {
permissionValue: TriState.kBlock,
isManaged: true,
},
};

const appList: App[] = [
FakePageHandler.createApp(
Expand Down
Expand Up @@ -8,30 +8,31 @@ import {InstallReason, InstallSource} from 'chrome://resources/cr_components/app
import {createBoolPermission, createTriStatePermission, getTriStatePermissionValue} from 'chrome://resources/cr_components/app_management/permission_util.js';
import {assert, assertNotReached} from 'chrome://resources/js/assert_ts.js';

import {PermissionOption} from './browser_proxy';
import {PermissionOption} from './browser_proxy.js';
import {AppManagementStore} from './store.js';

type AppConfig = Partial<App>;
type PermissionMap = Partial<Record<PermissionType, Permission>>;

export class FakePageHandler implements PageHandlerInterface {
static createWebPermissions(options?:
Record<PermissionType, PermissionOption>):
Record<PermissionType, Permission> {
static createWebPermissions(
options?: Partial<Record<PermissionType, PermissionOption>>):
PermissionMap {
const permissionTypes = [
PermissionType.kLocation,
PermissionType.kNotifications,
PermissionType.kMicrophone,
PermissionType.kCamera,
];

const permissions: Record<PermissionType, Permission> = {};
const permissions: PermissionMap = {};

for (const permissionType of permissionTypes) {
let permissionValue = TriState.kAllow;
let isManaged = false;

if (options && options[permissionType]) {
const opts = options[permissionType];
const opts = options[permissionType]!;
permissionValue = opts.value ? getTriStatePermissionValue(opts.value) :
permissionValue;
isManaged = opts.isManaged || isManaged;
Expand All @@ -43,8 +44,7 @@ export class FakePageHandler implements PageHandlerInterface {
return permissions;
}

static createArcPermissions(optIds?: number[]):
Record<PermissionType, Permission> {
static createArcPermissions(optIds?: PermissionType[]): PermissionMap {
const permissionTypes = optIds || [
PermissionType.kCamera,
PermissionType.kLocation,
Expand All @@ -54,7 +54,7 @@ export class FakePageHandler implements PageHandlerInterface {
PermissionType.kStorage,
];

const permissions: Record<PermissionType, Permission> = {};
const permissions: PermissionMap = {};

for (const permissionType of permissionTypes) {
permissions[permissionType] =
Expand All @@ -64,8 +64,7 @@ export class FakePageHandler implements PageHandlerInterface {
return permissions;
}

static createPermissions(appType: AppType):
Record<PermissionType, Permission> {
static createPermissions(appType: AppType): PermissionMap {
switch (appType) {
case (AppType.kWeb):
return FakePageHandler.createWebPermissions();
Expand Down
Expand Up @@ -29,7 +29,7 @@ import {App} from 'chrome://resources/cr_components/app_management/app_managemen
import {AppManagementEntryPoint, AppManagementEntryPointsHistogramName} from 'chrome://resources/cr_components/app_management/constants.js';
import {getAppIcon, getSelectedApp} from 'chrome://resources/cr_components/app_management/util.js';
import {I18nMixin, I18nMixinInterface} from 'chrome://resources/cr_elements/i18n_mixin.js';
import {assert, assertNotReached} from 'chrome://resources/js/assert_ts.js';
import {assert} from 'chrome://resources/js/assert_ts.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
import {mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

Expand Down Expand Up @@ -61,7 +61,6 @@ export function isAppInstalled(app: AppWithNotifications): boolean {
case Readiness.kUnknown:
return false;
}
assertNotReached();
}

const OsSettingsAppsPageElementBase =
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/resources/settings/chromeos/os_settings.gni
Expand Up @@ -420,7 +420,7 @@ generated_web_component_files = [

mojom_webui_files = [
"mojom-webui/audio/cros_audio_config.mojom-webui.js",
"mojom-webui/os_apps_page/app_notification_handler.mojom-webui.js",
"mojom-webui/os_apps_page/app_notification_handler.mojom-webui.ts",
"mojom-webui/personalization/search.mojom-webui.js",
"mojom-webui/routes.mojom-webui.js",
"mojom-webui/search/search.mojom-webui.js",
Expand Down
Expand Up @@ -11,6 +11,7 @@ mojom("mojom") {
sources = [ "app_notification_handler.mojom" ]

webui_module_path = "/"
use_typescript_sources = true

public_deps = [
"//mojo/public/mojom/base",
Expand Down
6 changes: 6 additions & 0 deletions ui/webui/resources/cr_components/README.md
Expand Up @@ -10,3 +10,9 @@ These components may also use chrome and extension APIs, e.g. chrome.send
component is expected to handle these calls.

For simpler components with no I18n or chrome dependencies, see cr_elements.

Note that some cr_components also have independent ts_library()s. These can be
identified by looking for a ts_library() rule in the BUILD.gn file for the
component. Components like this should not use relative paths to other files
in ui/webui/resources/, and users of these components must add a dependency on
the ts_library target to use the component.
29 changes: 13 additions & 16 deletions ui/webui/resources/cr_components/app_management/BUILD.gn
Expand Up @@ -16,6 +16,7 @@ assert(!is_android && !is_ios)
mojom("mojo_bindings") {
sources = [ "app_management.mojom" ]
webui_module_path = "chrome://resources/cr_components/app_management/"
use_typescript_sources = true

public_deps = [
"//mojo/public/mojom/base",
Expand Down Expand Up @@ -75,7 +76,6 @@ mojom("mojo_bindings") {
]
}

preprocess_folder_tmp = "$root_gen_dir/ui/webui/resources/preprocessed/cr_components/app_management_tmp"
preprocess_folder =
"$root_gen_dir/ui/webui/resources/preprocessed/cr_components/app_management"

Expand All @@ -86,40 +86,35 @@ preprocess_if_expr("preprocess") {
":html_wrapper_files",
]
in_folder = "."
out_folder = preprocess_folder_tmp
out_folder = target_gen_dir
in_files = ts_files + html_files + html_icons_files + css_files
}

html_to_wrapper("html_wrapper_files") {
deps = [ ":preprocess" ]
in_folder = preprocess_folder_tmp
out_folder = preprocess_folder_tmp
in_folder = target_gen_dir
out_folder = target_gen_dir
in_files = html_files + html_icons_files
minify = optimize_webui
}

css_to_wrapper("css_wrapper_files") {
deps = [ ":preprocess" ]
in_folder = preprocess_folder_tmp
out_folder = preprocess_folder_tmp
in_folder = target_gen_dir
out_folder = target_gen_dir
in_files = css_files
minify = optimize_webui
}

copy("copy_mojo") {
deps = [
":mojo_bindings_js__generator",
"//components/services/app_service/public/mojom:types_js__generator",
]
sources = [
"$root_gen_dir/mojom-webui/components/services/app_service/public/mojom/types.mojom-webui.js",
"$root_gen_dir/mojom-webui/ui/webui/resources/cr_components/app_management/app_management.mojom-webui.js",
]
outputs = [ "$preprocess_folder_tmp/{{source_file_part}}" ]
deps =
[ "//components/services/app_service/public/mojom:types_js__generator" ]
sources = [ "$root_gen_dir/mojom-webui/components/services/app_service/public/mojom/types.mojom-webui.js" ]
outputs = [ "$target_gen_dir/{{source_file_part}}" ]
}

ts_library("build_ts") {
root_dir = preprocess_folder_tmp
root_dir = target_gen_dir
out_dir = preprocess_folder
composite = true
tsconfig_base = "tsconfig_base.json"
Expand All @@ -130,11 +125,13 @@ ts_library("build_ts") {
deps = [
"//third_party/polymer/v3_0:library",
"//ui/webui/resources:library",
"//ui/webui/resources/mojo:library",
]
extra_deps = [
":copy_mojo",
":css_wrapper_files",
":html_wrapper_files",
":mojo_bindings_ts__generator",
":preprocess",
]
}
Expand Down
Expand Up @@ -35,7 +35,9 @@ _non_web_component_files = [
]

mojo_files = [
"app_management.mojom-webui.js",
"app_management.mojom-webui.ts",

# This file is stuck in JS due to dependencies on ChromeOS Ash.
"types.mojom-webui.js",
]

Expand Down
Expand Up @@ -6,12 +6,11 @@ import './app_management_shared_style.css.js';
import './toggle_row.js';

import {assert} from '//resources/js/assert_ts.js';
import {CrDialogElement} from 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';
import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
import {focusWithoutInk} from 'chrome://resources/js/focus_without_ink.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {CrDialogElement} from '../../cr_elements/cr_dialog/cr_dialog.js';
import {I18nMixin} from '../../cr_elements/i18n_mixin.js';

import {App} from './app_management.mojom-webui.js';
import {BrowserProxy} from './browser_proxy.js';
import {AppManagementUserAction} from './constants.js';
Expand Down
Expand Up @@ -99,6 +99,7 @@ export class AppManagementPermissionItemElement extends PolymerElement {
}

const permission = getPermission(app, permissionType);
assert(permission);
return permission.isManaged;
}

Expand Down Expand Up @@ -136,8 +137,10 @@ export class AppManagementPermissionItemElement extends PolymerElement {
syncPermission() {
let newPermission: Permission|undefined = undefined;

let newBoolState = false; // to keep the closure compiler happy.
const permissionValue = getPermission(this.app, this.permissionType).value;
let newBoolState = false;
const permission = getPermission(this.app, this.permissionType);
assert(permission);
const permissionValue = permission.value;
if (isBoolValue(permissionValue)) {
newPermission =
this.getUiPermissionBoolean_(this.app, this.permissionType);
Expand Down Expand Up @@ -167,6 +170,7 @@ export class AppManagementPermissionItemElement extends PolymerElement {
private getUiPermissionBoolean_(
app: App, permissionType: PermissionTypeIndex): Permission {
const currentPermission = getPermission(app, permissionType);
assert(currentPermission);

assert(isBoolValue(currentPermission.value));

Expand All @@ -184,6 +188,7 @@ export class AppManagementPermissionItemElement extends PolymerElement {
app: App, permissionType: PermissionTypeIndex): Permission {
let newPermissionValue;
const currentPermission = getPermission(app, permissionType);
assert(currentPermission);

assert(isTriStateValue(currentPermission.value));

Expand Down
5 changes: 3 additions & 2 deletions ui/webui/resources/cr_components/app_management/util.ts
Expand Up @@ -4,7 +4,7 @@

import {assert, assertNotReached} from 'chrome://resources/js/assert_ts.js';

import {App, PermissionType} from './app_management.mojom-webui.js';
import {App, Permission, PermissionType} from './app_management.mojom-webui.js';
import {BrowserProxy} from './browser_proxy.js';
import {AppManagementUserAction, AppType, OptionalBool} from './constants.js';
import {PermissionTypeIndex} from './permission_constants.js';
Expand Down Expand Up @@ -51,7 +51,8 @@ export function getPermissionValueBool(
/**
* Undefined is returned when the app does not request a permission.
*/
export function getPermission(app: App, permissionType: PermissionTypeIndex) {
export function getPermission(
app: App, permissionType: PermissionTypeIndex): Permission|undefined {
return app.permissions[PermissionType[permissionType]];
}

Expand Down

0 comments on commit d7709f5

Please sign in to comment.