Commit da9586ce authored by David's avatar David Committed by Commit Bot

Add `LoadTimeData` to chrome://media-app.

Previously we were writing to loadTimeData in
ChromeMediaAppUIDelegate::PopulateLoadTimeData but it was not made
available via window.loadTimeData.

This change adds load_time_data.js to our untrusted context so we can
make use of it.

Note: "/strings.js" is required to ensure LoadTimeData is non empty.

Now loadTimeData looks like
```
{
   "data_":{
      "appLocale":"en-GB",
      "fontfamily":"Roboto, sans-serif",
      "fontsize":"75%",
      "language":"en",
      "textdirection":"ltr"
   }
}
```

Side note: also fixes html lint (don't close single tags) & sorts DEPS

Bug: b/170160681, b/170423915
Change-Id: I4d73037577534ef516641e2acf86c45126d444f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437582
Commit-Queue: David Lei <dlei@google.com>
Auto-Submit: David Lei <dlei@google.com>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815503}
parent eb498e09
...@@ -87,12 +87,11 @@ group("closure_compile") { ...@@ -87,12 +87,11 @@ group("closure_compile") {
mojom("mojo_bindings") { mojom("mojo_bindings") {
sources = [ "media_app_ui.mojom" ] sources = [ "media_app_ui.mojom" ]
} }
media_test_lib_closure_flags = system_app_closure_flags_strict + [ "hide_warnings_for=chromeos/components/media_app_ui/media_app_ui.mojom-lite-for-compile.js" ] + ignore_load_time_data_errors_closure_flags
media_test_lib_closure_flags = system_app_closure_flags_strict + [ "hide_warnings_for=chromeos/components/media_app_ui/media_app_ui.mojom-lite-for-compile.js" ]
# Use relaxed flags for the browsertest files themselves. This removes null # Use relaxed flags for the browsertest files themselves. This removes null
# checks and "could not determine type" errors which don't add a lot of value. # checks and "could not determine type" errors which don't add a lot of value.
media_browsertest_closure_flags = system_app_closure_flags + [ "hide_warnings_for=chromeos/components/media_app_ui/media_app_ui.mojom-lite-for-compile.js" ] media_browsertest_closure_flags = system_app_closure_flags + [ "hide_warnings_for=chromeos/components/media_app_ui/media_app_ui.mojom-lite-for-compile.js" ] + ignore_load_time_data_errors_closure_flags
js_type_check("closure_compile_test_lib") { js_type_check("closure_compile_test_lib") {
testonly = true testonly = true
...@@ -167,6 +166,7 @@ js_library("test_media_app_guest_ui_browsertest_js") { ...@@ -167,6 +166,7 @@ js_library("test_media_app_guest_ui_browsertest_js") {
[ "//chromeos/components/web_applications/js2gtest_support.externs.js" ] [ "//chromeos/components/web_applications/js2gtest_support.externs.js" ]
deps = [ deps = [
":test_driver_js", ":test_driver_js",
"//chromeos/components/media_app_ui/resources/js:receiver",
"//chromeos/components/system_apps/public/js:message_pipe", "//chromeos/components/system_apps/public/js:message_pipe",
] ]
} }
...@@ -3,8 +3,8 @@ include_rules = [ ...@@ -3,8 +3,8 @@ include_rules = [
"+chromeos/components/web_applications", "+chromeos/components/web_applications",
"+chromeos/grit/chromeos_media_app_bundle_resources.h", "+chromeos/grit/chromeos_media_app_bundle_resources.h",
"+chromeos/grit/chromeos_resources.h", "+chromeos/grit/chromeos_resources.h",
"+components/content_settings/core/common",
"+chromeos/strings/grit/chromeos_strings.h", "+chromeos/strings/grit/chromeos_strings.h",
"+components/content_settings/core/common",
"+content/public/browser", "+content/public/browser",
"+content/public/common", "+content/public/common",
"+ui/webui", "+ui/webui",
......
...@@ -3,13 +3,19 @@ ...@@ -3,13 +3,19 @@
found in the LICENSE file. --> found in the LICENSE file. -->
<!DOCTYPE html> <!DOCTYPE html>
<html dir="$i18n{textdirection}" lang="$i18n{appLocale}"> <html dir="$i18n{textdirection}" lang="$i18n{appLocale}">
<meta charset="utf-8"/> <meta charset="utf-8">
<style> <style>
body { body {
margin: 0; margin: 0;
overflow: hidden; overflow: hidden;
} }
</style> </style>
<script src="/js/app_main.js"></script> <script src="/js/app_main.js"></script>
<script src="/media_app_app_scripts.js"></script> <script src="/media_app_app_scripts.js"></script>
<!-- Populates `window.loadTimeData`, needs to be after
"media_app_app_scripts.js" which loads "load_time_data.js" -->
<script src="/strings.js"></script>
</html> </html>
...@@ -24,7 +24,8 @@ js_type_check("closure_compile_index") { ...@@ -24,7 +24,8 @@ js_type_check("closure_compile_index") {
} }
js_type_check("closure_compile_app") { js_type_check("closure_compile_app") {
closure_flags = media_closure_flags closure_flags =
media_closure_flags + ignore_load_time_data_errors_closure_flags
deps = [ ":receiver" ] deps = [ ":receiver" ]
} }
...@@ -60,6 +61,7 @@ js_library("receiver") { ...@@ -60,6 +61,7 @@ js_library("receiver") {
deps = [ deps = [
":message_types", ":message_types",
"//chromeos/components/system_apps/public/js:message_pipe", "//chromeos/components/system_apps/public/js:message_pipe",
"//ui/webui/resources/js:load_time_data",
] ]
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
/** @fileoverview Concatenation of the JS files we use in app.html. */ /** @fileoverview Concatenation of the JS files we use in app.html. */
// <include src="../../../../../ui/webui/resources/js/load_time_data.js">
// <include src="../../../system_apps/public/js/message_pipe.js"> // <include src="../../../system_apps/public/js/message_pipe.js">
// <include src="message_types.js"> // <include src="message_types.js">
// <include src="receiver.js"> // <include src="receiver.js">
......
...@@ -41,6 +41,14 @@ GUEST_TEST('GuestHasLang', () => { ...@@ -41,6 +41,14 @@ GUEST_TEST('GuestHasLang', () => {
assertEquals(document.documentElement.lang, 'en-US'); assertEquals(document.documentElement.lang, 'en-US');
}); });
GUEST_TEST('GuestLoadsLoadTimeData', () => {
// Check `LoadTimeData` exists.
chai.assert.isTrue(loadTimeData !== undefined);
// Check data loaded into `LoadTimeData` by "strings.js" via
// `source->UseStringsJs()` exists.
assertEquals(loadTimeData.getValue('appLocale'), 'en-US');
});
// Test can load files with CSP restrictions. We expect `error` to be called // Test can load files with CSP restrictions. We expect `error` to be called
// as these tests are loading resources that don't exist. Note: we can't violate // as these tests are loading resources that don't exist. Note: we can't violate
// CSP in tests or Js Errors will cause test failures. // CSP in tests or Js Errors will cause test failures.
......
...@@ -1144,6 +1144,11 @@ TEST_F('MediaAppUIBrowserTest', 'GuestHasLang', async () => { ...@@ -1144,6 +1144,11 @@ TEST_F('MediaAppUIBrowserTest', 'GuestHasLang', async () => {
testDone(); testDone();
}); });
TEST_F('MediaAppUIBrowserTest', 'GuestLoadsLoadTimeData', async () => {
await runTestInGuest('GuestLoadsLoadTimeData');
testDone();
});
TEST_F('MediaAppUIBrowserTest', 'GuestCanLoadWithCspRestrictions', async () => { TEST_F('MediaAppUIBrowserTest', 'GuestCanLoadWithCspRestrictions', async () => {
await runTestInGuest('GuestCanLoadWithCspRestrictions'); await runTestInGuest('GuestCanLoadWithCspRestrictions');
testDone(); testDone();
......
...@@ -30,3 +30,12 @@ system_app_closure_flags_strict = ...@@ -30,3 +30,12 @@ system_app_closure_flags_strict =
"jscomp_error=conformanceViolations", "jscomp_error=conformanceViolations",
"jscomp_error=reportUnknownTypes", "jscomp_error=reportUnknownTypes",
] ]
ignore_load_time_data_errors_closure_flags = [
"hide_warnings_for=ui/webui/resources/js/load_time_data.js",
# load_time_data.js has dependencies on these files that also throw closure errors.
"hide_warnings_for=ui/webui/resources/js/assert.js",
"hide_warnings_for=ui/webui/resources/js/parse_html_subset.js",
"hide_warnings_for=third_party/jstemplate/",
]
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