[M90] OOBE - Prevent Renderer Crashes
Prevent crashes by checking if Javascript is allowed in
all call sites in ErrorScreenHandler.
Replace IsJavascriptAllowed() checks with IsSafeToCallJavascript()
in all call sites that were added to mitigate Issue 1174691.
The new method allows JS requests to to be stored in the JS calls
container if the renderer message wasn't received yet.
Synchronise LoginOfflineTest with the OOBE initialisation.
Bug: 1174691, 1189787
Change-Id: Ia8a8373570221556677714617e526499ff023548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2831521
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/branch-heads/4430@{#1298}
Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
diff --git a/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
index c2a75715..c0da934 100644
--- a/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
@@ -84,7 +84,7 @@
return;
const std::string country_code = base::CountryCodeForCurrentTimezone();
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("login.ArcTermsOfServiceScreen.loadPlayStoreToS", country_code);
} else {
LOG(ERROR) << "Silently dropping MaybeLoadPlayStoreToS request.";
diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
index 1cf50cc..9a750f5d 100644
--- a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
@@ -20,8 +20,8 @@
void BaseWebUIHandler::InitializeBase() {
page_is_ready_ = true;
- Initialize();
AllowJavascript();
+ Initialize();
}
void BaseWebUIHandler::GetLocalizedStrings(base::DictionaryValue* dict) {
diff --git a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h
index ef48235..e9207e8 100644
--- a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h
+++ b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.h
@@ -88,6 +88,15 @@
function_name, ::login::MakeValue(args).Clone()...);
}
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed
+ // If the JS container hasn't been initialized yet, it is safe to call JS
+ // because the call will be postponed until we receive a message from the
+ // renderer.
+ bool IsSafeToCallJavascript() const {
+ return (js_calls_container_ && !js_calls_container_->is_initialized()) ||
+ IsJavascriptAllowed();
+ }
+
// Register WebUI callbacks. The callbacks will be recorded if recording is
// enabled.
template <typename T>
diff --git a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
index 356fbaa..ce6f701e 100644
--- a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
@@ -195,7 +195,7 @@
void CoreOobeHandler::ReloadEulaContent(
const base::DictionaryValue& dictionary) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.reloadEulaContent", dictionary);
} else {
LOG(ERROR) << "Silently dropping ReloadEulaContent request.";
@@ -208,7 +208,7 @@
void CoreOobeHandler::SetClientAreaSize(int width, int height) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setClientAreaSize", width, height);
} else {
LOG(ERROR) << "Silently dropping SetClientAreaSize request.";
@@ -217,7 +217,7 @@
void CoreOobeHandler::SetShelfHeight(int height) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setShelfHeight", height);
} else {
LOG(ERROR) << "Silently dropping SetShelfHeight request.";
@@ -226,7 +226,7 @@
void CoreOobeHandler::SetOrientation(bool is_horizontal) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setOrientation", is_horizontal);
} else {
LOG(ERROR) << "Silently dropping SetOrientation request.";
@@ -235,7 +235,7 @@
void CoreOobeHandler::SetDialogSize(int width, int height) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setDialogSize", width, height);
} else {
LOG(ERROR) << "Silently dropping SetDialogSize request.";
@@ -360,7 +360,7 @@
void CoreOobeHandler::UpdateLabel(const std::string& id,
const std::string& text) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setLabelText", id, text);
} else {
LOG(ERROR) << "Silently dropping UpdateLabel request.";
@@ -412,7 +412,7 @@
NOTREACHED();
}
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("cr.ui.Oobe.setDialogPaddingMode", padding);
} else {
LOG(ERROR) << "Silently dropping SetDialogPaddingMode request.";
diff --git a/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
index 1e939a5..2afa803 100644
--- a/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
@@ -62,7 +62,7 @@
void ErrorScreenHandler::SetErrorStateCode(
NetworkError::ErrorState error_state) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("login.ErrorMessageScreen.setErrorState",
static_cast<int>(error_state));
} else {
@@ -72,7 +72,7 @@
void ErrorScreenHandler::SetErrorStateNetwork(const std::string& network_name) {
// TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
- if (IsJavascriptAllowed()) {
+ if (IsSafeToCallJavascript()) {
CallJS("login.ErrorMessageScreen.setErrorStateNetwork", network_name);
} else {
LOG(ERROR) << "Silently dropping SetErrorStateNetwork request.";
@@ -80,33 +80,68 @@
}
void ErrorScreenHandler::SetGuestSigninAllowed(bool value) {
- CallJS("login.ErrorMessageScreen.allowGuestSignin", value);
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.ErrorMessageScreen.allowGuestSignin", value);
+ } else {
+ LOG(ERROR) << "Silently dropping SetGuestSigninAllowed request.";
+ }
}
void ErrorScreenHandler::SetOfflineSigninAllowed(bool value) {
- CallJS("login.ErrorMessageScreen.allowOfflineLogin", value);
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.ErrorMessageScreen.allowOfflineLogin", value);
+ } else {
+ LOG(ERROR) << "Silently dropping SetOfflineSigninAllowed request.";
+ }
}
void ErrorScreenHandler::SetShowConnectingIndicator(bool value) {
- CallJS("login.ErrorMessageScreen.showConnectingIndicator", value);
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.ErrorMessageScreen.showConnectingIndicator", value);
+ } else {
+ LOG(ERROR) << "Silently dropping SetShowConnectingIndicator request.";
+ }
}
void ErrorScreenHandler::SetIsPersistentError(bool is_persistent) {
- CallJS("login.ErrorMessageScreen.setIsPersistentError", is_persistent);
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.ErrorMessageScreen.setIsPersistentError", is_persistent);
+ } else {
+ LOG(ERROR) << "Silently dropping SetIsPersistentError request.";
+ }
}
void ErrorScreenHandler::SetUIState(NetworkError::UIState ui_state) {
- CallJS("login.ErrorMessageScreen.setUIState", static_cast<int>(ui_state));
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.ErrorMessageScreen.setUIState", static_cast<int>(ui_state));
+ } else {
+ LOG(ERROR) << "Silently dropping SetUIState request.";
+ }
}
// TODO (crbug.com/1168114): We need to handle that fully in C++ once
// all error screen logic is migrated to Screen object.
void ErrorScreenHandler::OnCancelButtonClicked() {
- CallJS("cr.ui.Oobe.showUserPods");
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("cr.ui.Oobe.showUserPods");
+ } else {
+ LOG(ERROR) << "Silently dropping OnCancelButtonClicked request.";
+ }
}
void ErrorScreenHandler::OnReloadGaiaClicked() {
- CallJS("login.GaiaSigninScreen.doReload");
+ // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+ if (IsSafeToCallJavascript()) {
+ CallJS("login.GaiaSigninScreen.doReload");
+ } else {
+ LOG(ERROR) << "Silently dropping OnReloadGaiaClicked request.";
+ }
}
void ErrorScreenHandler::DeclareLocalizedValues(