Commit 0e972cf1 authored by Erik Chen's avatar Erik Chen Committed by Commit Bot

lacros: Fix crash when opening chrome://settings

Lacros uses the chromeOS user-agent string but does not display
ash-specific settings. Historically, webui relied on the user-agent to
determine what platform the browser runs on. The combination of the two
results in webUI settings trying to access ash-only private extension
APIs, causing a crash.

This CL updates the logic to explicitly check platform via gn args via
grit, and plumbs through lacros as a new platform.

This CL has one known problem: grit is not processed for js/cr.js for
some webUI tests. This results on isLacros and isChromeOS returning the
wrong values. This doesn't seem to affect any tests for now, but this is
a problem that should be fixed. This is tracked in
https://crbug.com/1118190.

Bug: 1111985
Change-Id: I038a533ccbc5018cd125a49e70320a36b5aa4d31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360816
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Auto-Submit: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800764}
parent 204dd2e8
...@@ -24,6 +24,8 @@ if (optimize_webui) { ...@@ -24,6 +24,8 @@ if (optimize_webui) {
":unpak", ":unpak",
"../../../../../ui/webui/resources:modulize", "../../../../../ui/webui/resources:modulize",
] ]
excludes = [ "chrome://resources/js/cr.m.js" ]
} }
unpak("unpak") { unpak("unpak") {
......
...@@ -18,7 +18,7 @@ function testGetFaviconForPageURL() { ...@@ -18,7 +18,7 @@ function testGetFaviconForPageURL() {
'&allow_google_server_fallback=0") ' + '&allow_google_server_fallback=0") ' +
window.devicePixelRatio + 'x)'; window.devicePixelRatio + 'x)';
var isDesktop = cr.isMac || cr.isChromeOS || cr.isWindows || cr.isLinux; var isDesktop = cr.isMac || cr.isChromeOS || cr.isWindows || cr.isLinux || cr.isLacros;
var expected = isDesktop ? expectedDesktop : expectedOther; var expected = isDesktop ? expectedDesktop : expectedOther;
assertEquals(expected, cr.icon.getFaviconForPageURL(url)); assertEquals(expected, cr.icon.getFaviconForPageURL(url));
} }
...@@ -35,7 +35,7 @@ function testGetFavicon() { ...@@ -35,7 +35,7 @@ function testGetFavicon() {
encodeURIComponent('http://foo.com/foo.ico')+'") ' + encodeURIComponent('http://foo.com/foo.ico')+'") ' +
window.devicePixelRatio + 'x)'; window.devicePixelRatio + 'x)';
var isDesktop = cr.isMac || cr.isChromeOS || cr.isWindows || cr.isLinux; var isDesktop = cr.isMac || cr.isChromeOS || cr.isWindows || cr.isLinux || cr.isLacros;
var expected = isDesktop ? expectedDesktop : expectedOther; var expected = isDesktop ? expectedDesktop : expectedOther;
assertEquals(expected, cr.icon.getFavicon(url)); assertEquals(expected, cr.icon.getFavicon(url));
} }
......
...@@ -436,9 +436,26 @@ var cr = cr || function(global) { ...@@ -436,9 +436,26 @@ var cr = cr || function(global) {
return /Win/.test(navigator.platform); return /Win/.test(navigator.platform);
}, },
/** Whether this is on chromeOS or not. */ /** Whether this is the ChromeOS/ash web browser. */
get isChromeOS() { get isChromeOS() {
return /CrOS/.test(navigator.userAgent); let returnValue = false;
// TODO(https://crbug.com/1118190): grit conditionals do not work in many
// WebUI tests.
// <if expr="chromeos">
returnValue = true;
// </if>
return returnValue;
},
/** Whether this is the ChromeOS/Lacros web browser. */
get isLacros() {
let returnValue = false;
// TODO(https://crbug.com/1118190): grit conditionals do not work in many
// WebUI tests.
// <if expr="lacros">
returnValue = true;
// </if>
return returnValue;
}, },
/** Whether this is on vanilla Linux (not chromeOS). */ /** Whether this is on vanilla Linux (not chromeOS). */
......
...@@ -184,8 +184,23 @@ export const isMac = /Mac/.test(navigator.platform); ...@@ -184,8 +184,23 @@ export const isMac = /Mac/.test(navigator.platform);
/** Whether this is on the Windows platform or not. */ /** Whether this is on the Windows platform or not. */
export const isWindows = /Win/.test(navigator.platform); export const isWindows = /Win/.test(navigator.platform);
/** Whether this is on chromeOS or not. */ /** Whether this is the ChromeOS/ash web browser. */
export const isChromeOS = /CrOS/.test(navigator.userAgent); export const isChromeOS = (() => {
let returnValue = false;
// <if expr="chromeos">
returnValue = true;
// </if>
return returnValue;
})();
/** Whether this is the ChromeOS/Lacros web browser. */
export const isLacros = (() => {
let returnValue = false;
// <if expr="lacros">
returnValue = true;
// </if>
return returnValue;
})();
/** Whether this is on vanilla Linux (not chromeOS). */ /** Whether this is on vanilla Linux (not chromeOS). */
export const isLinux = /Linux/.test(navigator.userAgent); export const isLinux = /Linux/.test(navigator.userAgent);
......
...@@ -162,7 +162,8 @@ cr.define('cr.ui', function() { ...@@ -162,7 +162,8 @@ cr.define('cr.ui', function() {
case 'mousedown': case 'mousedown':
if (!this.menu.contains(e.target)) { if (!this.menu.contains(e.target)) {
this.hideMenu(); this.hideMenu();
if (e.button === 0 /* Left button */ && (cr.isLinux || cr.isMac)) { if (e.button === 0 /* Left button */ &&
(cr.isLinux || cr.isMac || cr.isLacros)) {
// Emulate Mac and Linux, which swallow native 'mousedown' events // Emulate Mac and Linux, which swallow native 'mousedown' events
// that close menus. // that close menus.
e.preventDefault(); e.preventDefault();
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// #import {isChromeOS, isIOS, isLinux, isMac, isWindows} from './cr.m.js'; // #import {isAndroid, isIOS} from './cr.m.js';
cr.define('cr.icon', function() { cr.define('cr.icon', function() {
/** /**
...@@ -16,7 +16,7 @@ cr.define('cr.icon', function() { ...@@ -16,7 +16,7 @@ cr.define('cr.icon', function() {
// supports SCALE_FACTOR_100P on all non-iOS platforms. // supports SCALE_FACTOR_100P on all non-iOS platforms.
supportedScaleFactors.push(1); supportedScaleFactors.push(1);
} }
if (cr.isMac || cr.isChromeOS || cr.isWindows || cr.isLinux) { if (!cr.isIOS && !cr.isAndroid) {
// All desktop platforms support zooming which also updates the renderer's // All desktop platforms support zooming which also updates the renderer's
// device scale factors (a.k.a devicePixelRatio), and these platforms have // device scale factors (a.k.a devicePixelRatio), and these platforms have
// high DPI assets for 2x. Let the renderer pick the closest image for // high DPI assets for 2x. Let the renderer pick the closest image for
......
...@@ -12,6 +12,7 @@ common_namespace_rewrites = [ ...@@ -12,6 +12,7 @@ common_namespace_rewrites = [
"cr.isAndroid|isAndroid", "cr.isAndroid|isAndroid",
"cr.isChromeOS|isChromeOS", "cr.isChromeOS|isChromeOS",
"cr.isIOS|isIOS", "cr.isIOS|isIOS",
"cr.isLacros|isLacros",
"cr.isLinux|isLinux", "cr.isLinux|isLinux",
"cr.isMac|isMac", "cr.isMac|isMac",
"cr.isWindows|isWindows", "cr.isWindows|isWindows",
......
...@@ -295,9 +295,11 @@ without changes to the corresponding grd file. --> ...@@ -295,9 +295,11 @@ without changes to the corresponding grd file. -->
file="js/promise_resolver.js" type="chrome_html" /> file="js/promise_resolver.js" type="chrome_html" />
<structure name="IDR_WEBUI_JS_CR" <structure name="IDR_WEBUI_JS_CR"
file="js/cr.js" type="chrome_html" /> file="js/cr.js" type="chrome_html"
preprocess="true" />
<structure name="IDR_WEBUI_JS_CR_M_JS" <structure name="IDR_WEBUI_JS_CR_M_JS"
file="js/cr.m.js" type="chrome_html" /> file="js/cr.m.js" type="chrome_html"
preprocess="true" />
<structure name="IDR_WEBUI_JS_CR_EVENT_TARGET" <structure name="IDR_WEBUI_JS_CR_EVENT_TARGET"
file="js/cr/event_target.js" type="chrome_html" /> file="js/cr/event_target.js" type="chrome_html" />
<structure name="IDR_WEBUI_JS_CR_UI" <structure name="IDR_WEBUI_JS_CR_UI"
......
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