Skip to content

Commit

Permalink
CrOS Settings: Lazy load search settings page subpages
Browse files Browse the repository at this point in the history
All subpages should be included in the lazy load bundle and be
removed from the main bundle.

Also renames some files/elements and their respective browser tests
to include "subpage". Related browser tests will now import from
lazy_load.js.

Observed bundle size change:
os_settings.rollup.js: 1.46 MB -> 1.44 MB
lazy_load.rollup.js: 598.60 KB -> 613.94 KB

Bug: b/263414034
Test: browser_tests --gtest_filter="OSSettingsOsSearchPage*"
Test: browser_tests --gtest_filter="OSSettingsSearchSubpage*"
Test: browser_tests --gtest_filter="OSSettingsGoogleAssistant*"
Change-Id: Ia13b697a3fb9bc8ba5b7bf7aed524245dfd1384c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4330173
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Wes Okuhara <wesokuhara@google.com>
Cr-Commit-Position: refs/heads/main@{#1116039}
  • Loading branch information
Wes Okuhara authored and Chromium LUCI CQ committed Mar 11, 2023
1 parent a666881 commit a53f5e4
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 29 deletions.
4 changes: 4 additions & 0 deletions chrome/browser/resources/settings/chromeos/lazy_load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import './internet_page/internet_detail_subpage.js';
import './internet_page/internet_known_networks_subpage.js';
import './internet_page/internet_subpage.js';
import './os_about_page/detailed_build_info_subpage.js';
import './os_search_page/google_assistant_subpage.js';
import './os_search_page/search_subpage.js';
// TODO(b/263414034) Determine if elements below adhere to the lazy loading
// criteria and are needed here
import './crostini_page/bruschetta_subpage.js';
Expand Down Expand Up @@ -110,3 +112,5 @@ export {MetricsConsentBrowserProxy, MetricsConsentBrowserProxyImpl, MetricsConse
export {DataAccessPolicyState, PeripheralDataAccessBrowserProxy, PeripheralDataAccessBrowserProxyImpl} from './os_privacy_page/peripheral_data_access_browser_proxy.js';
export {PrivacyHubBrowserProxy, PrivacyHubBrowserProxyImpl} from './os_privacy_page/privacy_hub_browser_proxy.js';
export {OsResetBrowserProxyImpl} from './os_reset_page/os_reset_browser_proxy.js';
export {GoogleAssistantBrowserProxyImpl} from './os_search_page/google_assistant_browser_proxy.js';
export {ConsentStatus, DspHotwordState} from './os_search_page/google_assistant_subpage.js';
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

/**
* @fileoverview 'settings-google-assistant-page' is the settings page
* @fileoverview 'settings-google-assistant-subpage' is the settings page
* containing Google Assistant settings.
*/
import 'chrome://resources/cr_elements/cr_link_row/cr_link_row.js';
Expand Down Expand Up @@ -31,7 +31,7 @@ import {RouteObserverMixin} from '../route_observer_mixin.js';
import {Route} from '../router.js';

import {GoogleAssistantBrowserProxy, GoogleAssistantBrowserProxyImpl} from './google_assistant_browser_proxy.js';
import {getTemplate} from './google_assistant_page.html.js';
import {getTemplate} from './google_assistant_subpage.html.js';

/**
* The types of Hotword enable status without Dsp support.
Expand Down Expand Up @@ -63,14 +63,14 @@ export enum ConsentStatus {
NOT_FOUND = 3,
}

const SettingsGoogleAssistantPageElementBase =
const SettingsGoogleAssistantSubpageElementBase =
DeepLinkingMixin(RouteObserverMixin(
PrefsMixin(WebUiListenerMixin(I18nMixin(PolymerElement)))));

class SettingsGoogleAssistantPageElement extends
SettingsGoogleAssistantPageElementBase {
class SettingsGoogleAssistantSubpageElement extends
SettingsGoogleAssistantSubpageElementBase {
static get is() {
return 'settings-google-assistant-page';
return 'settings-google-assistant-subpage';
}

static get template() {
Expand Down Expand Up @@ -282,9 +282,10 @@ class SettingsGoogleAssistantPageElement extends

declare global {
interface HTMLElementTagNameMap {
'settings-google-assistant-page': SettingsGoogleAssistantPageElement;
'settings-google-assistant-subpage': SettingsGoogleAssistantSubpageElement;
}
}

customElements.define(
SettingsGoogleAssistantPageElement.is, SettingsGoogleAssistantPageElement);
SettingsGoogleAssistantSubpageElement.is,
SettingsGoogleAssistantSubpageElement);
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@
<template is="dom-if" if="[[isAssistantAllowed_]]">
<template is="dom-if" route-path="/googleAssistant">
<os-settings-subpage page-title="$i18n{googleAssistantPageTitle}">
<settings-google-assistant-page prefs="{{prefs}}">
</settings-google-assistant-page>
<settings-google-assistant-subpage prefs="{{prefs}}">
</settings-google-assistant-subpage>
</os-settings-subpage>
</template>
</template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ import 'chrome://resources/cr_elements/icons.html.js';
import 'chrome://resources/cr_elements/policy/cr_policy_pref_indicator.js';
import 'chrome://resources/cr_elements/cr_shared_style.css.js';
import 'chrome://resources/cr_elements/cr_shared_vars.css.js';
import './os_search_selection_dialog.js';
import '../../controls/extension_controlled_indicator.js';
import '../os_settings_page/os_settings_animated_pages.js';
import '../os_settings_page/os_settings_subpage.js';
import '../../settings_shared.css.js';
import '../../settings_vars.css.js';
import '../google_assistant_page/google_assistant_page.js';
import './search_subpage.js';
import '../os_settings_page/os_settings_animated_pages.js';
import '../os_settings_page/os_settings_subpage.js';
import './os_search_selection_dialog.js';
import './search_engine.js';

import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/resources/settings/chromeos/os_settings.gni
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ web_component_files = [
"chromeos/device_page/storage_external.ts",
"chromeos/device_page/storage_external_entry.ts",
"chromeos/device_page/stylus.ts",
"chromeos/google_assistant_page/google_assistant_page.ts",
"chromeos/guest_os/guest_os_container_select.ts",
"chromeos/guest_os/guest_os_shared_paths.ts",
"chromeos/guest_os/guest_os_shared_usb_devices.ts",
Expand Down Expand Up @@ -225,6 +224,7 @@ web_component_files = [
"chromeos/os_reset_page/os_powerwash_dialog.ts",
"chromeos/os_reset_page/os_powerwash_dialog_esim_item.ts",
"chromeos/os_reset_page/os_reset_page.ts",
"chromeos/os_search_page/google_assistant_subpage.ts",
"chromeos/os_search_page/os_search_page.ts",
"chromeos/os_search_page/os_search_selection_dialog.ts",
"chromeos/os_search_page/search_engine.ts",
Expand Down Expand Up @@ -320,7 +320,6 @@ non_web_component_files = [
"chromeos/device_page/input_device_mojo_interface_provider.ts",
"chromeos/device_page/input_device_settings_types.ts",
"chromeos/ensure_lazy_loaded.ts",
"chromeos/google_assistant_page/google_assistant_browser_proxy.ts",
"chromeos/guest_os/guest_os_browser_proxy.ts",
"chromeos/internet_page/cellular_setup_settings_delegate.ts",
"chromeos/internet_page/internet_page_browser_proxy.ts",
Expand Down Expand Up @@ -385,6 +384,7 @@ non_web_component_files = [
"chromeos/os_privacy_page/peripheral_data_access_browser_proxy.ts",
"chromeos/os_privacy_page/privacy_hub_browser_proxy.ts",
"chromeos/os_reset_page/os_reset_browser_proxy.ts",
"chromeos/os_search_page/google_assistant_browser_proxy.ts",
"chromeos/os_search_page/search_engines_browser_proxy.ts",
"chromeos/os_settings_routes.ts",
"chromeos/os_settings_search_box/os_settings_search_box_browser_proxy.ts",
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/resources/settings/chromeos/os_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import './device_page/storage.js';
import './device_page/storage_external.js';
import './device_page/storage_external_entry.js';
import './device_page/stylus.js';
import './google_assistant_page/google_assistant_page.js';
import './kerberos_page/kerberos_accounts.js';
import './kerberos_page/kerberos_page.js';
import './multidevice_page/multidevice_page.js';
Expand Down Expand Up @@ -132,8 +131,6 @@ export {SettingsPerDeviceKeyboardSubsectionElement} from './device_page/per_devi
export {SettingsPerDeviceMouseSubsectionElement} from './device_page/per_device_mouse_subsection.js';
export {SettingsPerDevicePointingStickSubsectionElement} from './device_page/per_device_pointing_stick_subsection.js';
export {SettingsPerDeviceTouchpadSubsectionElement} from './device_page/per_device_touchpad_subsection.js';
export {GoogleAssistantBrowserProxyImpl} from './google_assistant_page/google_assistant_browser_proxy.js';
export {ConsentStatus, DspHotwordState} from './google_assistant_page/google_assistant_page.js';
export {InternetPageBrowserProxy, InternetPageBrowserProxyImpl} from './internet_page/internet_page_browser_proxy.js';
export {KerberosAccountsBrowserProxyImpl, KerberosConfigErrorCode, KerberosErrorType} from './kerberos_page/kerberos_accounts_browser_proxy.js';
export {recordClick, recordNavigation, recordPageBlur, recordPageFocus, recordSearch, recordSettingChange, setUserActionRecorderForTesting} from './metrics_recorder.js';
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/webui/settings/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test_files = [
"hotspot_config_dialog_tests.js",
"hotspot_subpage_tests.js",
"hotspot_summary_item_tests.js",
"google_assistant_page_test.js",
"google_assistant_subpage_test.js",
"guest_os_shared_paths_test.js",
"guest_os_shared_usb_devices_test.js",
"input_device_mojo_interface_provider_test.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {ConsentStatus, CrSettingsPrefs, DspHotwordState, GoogleAssistantBrowserProxyImpl, Router, routes} from 'chrome://os-settings/chromeos/os_settings.js';
import 'chrome://os-settings/chromeos/lazy_load.js';

import {ConsentStatus, DspHotwordState, GoogleAssistantBrowserProxyImpl} from 'chrome://os-settings/chromeos/lazy_load.js';
import {CrSettingsPrefs, Router, routes} from 'chrome://os-settings/chromeos/os_settings.js';
import {getDeepActiveElement} from 'chrome://resources/ash/common/util.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';
import {TestMock} from 'chrome://webui-test/test_mock.js';

suite('GoogleAssistantHandler', function() {
/** @type {SettingsGoogleAssistantPageElement} */
/** @type {SettingsGoogleAssistantSubpageElement} */
let page = null;

let browserProxy = null;
Expand All @@ -32,7 +35,7 @@ suite('GoogleAssistantHandler', function() {
document.body.appendChild(prefElement);

return CrSettingsPrefs.initialized.then(function() {
page = document.createElement('settings-google-assistant-page');
page = document.createElement('settings-google-assistant-subpage');
page.prefs = prefElement.prefs;
document.body.appendChild(page);
});
Expand Down Expand Up @@ -260,7 +263,7 @@ suite('GoogleAssistantHandler', function() {
});

suite('GoogleAssistantHandlerWithNoDspHotword', function() {
/** @type {SettingsGoogleAssistantPageElement} */
/** @type {SettingsGoogleAssistantSubpageElement} */
let page = null;

let browserProxy = null;
Expand All @@ -282,7 +285,7 @@ suite('GoogleAssistantHandlerWithNoDspHotword', function() {
document.body.appendChild(prefElement);

return CrSettingsPrefs.initialized.then(function() {
page = document.createElement('settings-google-assistant-page');
page = document.createElement('settings-google-assistant-subpage');
page.prefs = prefElement.prefs;
document.body.appendChild(page);
flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ TEST_F('OSSettingsCrostiniExtraContainerPageV3Test', 'AllJsTests', () => {
['FakeInputDeviceSettings', 'fake_input_device_settings_provider_test.js'],
['FilesPage', 'os_files_page_test.js'],
['FingerprintPage', 'fingerprint_browsertest_chromeos.js'],
['GoogleAssistantPage', 'google_assistant_page_test.js'],
['GoogleAssistantSubpage', 'google_assistant_subpage_test.js'],
['GuestOsSharedPaths', 'guest_os_shared_paths_test.js'],
['GuestOsSharedUsbDevices', 'guest_os_shared_usb_devices_test.js'],
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'chrome://os-settings/chromeos/lazy_load.js';

import {CrSettingsPrefs, Router, routes} from 'chrome://os-settings/chromeos/os_settings.js';
import {getDeepActiveElement} from 'chrome://resources/ash/common/util.js';
import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';

import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';

suite('SearchSubpage', function() {
/** @type {SearchSubpageElement} */
Expand Down

0 comments on commit a53f5e4

Please sign in to comment.