[go: up one dir, main page]

[M90] OOBE - Improve Renderer Stability

> OOBE - Prevent Renderer Crashes - CoreOobeHandler
>
> Prevent crashes by checking if JavascriptIsAllowed in
> - CoreOobeHandler::SetDialogPaddingMode
> - CoreOobeHandler::SetClientAreaSize
> - CoreOobeHandler::SetShelfHeight
> - CoreOobeHandler::SetOrientation
> - CoreOobeHandler::SetDialogSize
> - CoreOobeHandler::UpdateLabel
> - CoreOobeHandler::ReloadEulaContent
>
> These checks should prevent the system from requesting OOBE to
> execute JavaScript when the renderer no longer exists. This is
> a temporary measure that will be addressed properly later.
>
> Bug: 1174691, 1180291
> Change-Id: Ic169bff529d11a505c26aec443f014921fa2158f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822163
> Commit-Queue: Renato Silva <rrsilva@google.com>
> Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#872751}
> (cherry picked from commit 5280824a09bb38ddf695cf2c067c326d8fdf89f8)
>
>
> OOBE - ARC Terms of Service - Prevent crashes
>
> Prevent the browser from crashing when trying to load the
> ARC terms of service caused by trying to execute JS on a non-
> existing renderer.
>
> Bug: 1174691, 1180291
> Change-Id: I94bb8b0a86a7db271b0a74579039388b56fbf611
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2814926
> Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Commit-Queue: Renato Silva <rrsilva@google.com>
> Cr-Commit-Position: refs/heads/master@{#872528}
> (cherry picked from commit 902faa69c58c4c883d44704623b602b2e04a642d)
>
> OOBE - Prevent Renderer Crashes - Error Screen
>
> Prevent crashes by checking if JavascriptIsAllowed in
> ErrorScreenHandler::SetErrorStateCode().
>
> Bug: 1174691, 1180291, 1189787
> Change-Id: I4eec3309d886641201f289939b4426d1d4540509
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2814842
> Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Commit-Queue: Renato Silva <rrsilva@google.com>
> Cr-Commit-Position: refs/heads/master@{#871945}
> (cherry picked from commit 0f75179e5cfcef899930d3b72de59c459ab911f0)
>
> OOBE - Prevent Renderer Crashes - CoreOobeHandler
>
> Prevent crashes by checking if JavascriptIsAllowed in
> CoreOobeHandler::OnEnterpriseInfoUpdated().
>
> Bug: 1174691
> Change-Id: I8fadc9c721f308ef6ce37a997f43a76c389b85a8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2817059
> Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Commit-Queue: Renato Silva <rrsilva@google.com>
> Cr-Commit-Position: refs/heads/master@{#871861}
> (cherry picked from commit e2ef40435b5088e2a2d73764aa1a4310742f6560)

Bug: 1174691, 1180291
Change-Id: Id6a6c3da09483ad0e8f81a345253004a75d84140
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2825879
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/branch-heads/4430@{#1281}
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 8200c8f..c2a75715 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
@@ -83,7 +83,12 @@
   if (!ignore_network_state && !default_network)
     return;
   const std::string country_code = base::CountryCodeForCurrentTimezone();
-  CallJS("login.ArcTermsOfServiceScreen.loadPlayStoreToS", country_code);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("login.ArcTermsOfServiceScreen.loadPlayStoreToS", country_code);
+  } else {
+    LOG(ERROR) << "Silently dropping MaybeLoadPlayStoreToS request.";
+  }
 }
 
 void ArcTermsOfServiceScreenHandler::OnCurrentScreenChanged(
@@ -296,6 +301,7 @@
 }
 
 void ArcTermsOfServiceScreenHandler::StartNetworkAndTimeZoneObserving() {
+  // TODO(crbug.com/1180291) - Clean up work. Fix this logic.
   if (network_time_zone_observing_)
     return;
 
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 f8cadf5..1cf50cc 100644
--- a/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/base_webui_handler.cc
@@ -21,6 +21,7 @@
 void BaseWebUIHandler::InitializeBase() {
   page_is_ready_ = true;
   Initialize();
+  AllowJavascript();
 }
 
 void BaseWebUIHandler::GetLocalizedStrings(base::DictionaryValue* dict) {
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 54be745c..356fbaa 100644
--- a/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
@@ -194,7 +194,12 @@
 
 void CoreOobeHandler::ReloadEulaContent(
     const base::DictionaryValue& dictionary) {
-  CallJS("cr.ui.Oobe.reloadEulaContent", dictionary);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.reloadEulaContent", dictionary);
+  } else {
+    LOG(ERROR) << "Silently dropping ReloadEulaContent request.";
+  }
 }
 
 void CoreOobeHandler::SetVirtualKeyboardShown(bool shown) {
@@ -202,25 +207,45 @@
 }
 
 void CoreOobeHandler::SetClientAreaSize(int width, int height) {
-  CallJS("cr.ui.Oobe.setClientAreaSize", width, height);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setClientAreaSize", width, height);
+  } else {
+    LOG(ERROR) << "Silently dropping SetClientAreaSize request.";
+  }
 }
 
 void CoreOobeHandler::SetShelfHeight(int height) {
-  CallJS("cr.ui.Oobe.setShelfHeight", height);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setShelfHeight", height);
+  } else {
+    LOG(ERROR) << "Silently dropping SetShelfHeight request.";
+  }
 }
 
 void CoreOobeHandler::SetOrientation(bool is_horizontal) {
-  CallJS("cr.ui.Oobe.setOrientation", is_horizontal);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setOrientation", is_horizontal);
+  } else {
+    LOG(ERROR) << "Silently dropping SetOrientation request.";
+  }
 }
 
 void CoreOobeHandler::SetDialogSize(int width, int height) {
-  CallJS("cr.ui.Oobe.setDialogSize", width, height);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setDialogSize", width, height);
+  } else {
+    LOG(ERROR) << "Silently dropping SetDialogSize request.";
+  }
 }
 
 void CoreOobeHandler::HandleInitialized() {
   VLOG(3) << "CoreOobeHandler::HandleInitialized";
-  GetOobeUI()->InitializeHandlers();
   AllowJavascript();
+  GetOobeUI()->InitializeHandlers();
 }
 
 void CoreOobeHandler::HandleUpdateCurrentScreen(
@@ -321,7 +346,7 @@
 
 void CoreOobeHandler::OnEnterpriseInfoUpdated(const std::string& message_text,
                                               const std::string& asset_id) {
-  CallJS("cr.ui.Oobe.setEnterpriseInfo", message_text, asset_id);
+  // Not relevant in OOBE mode.
 }
 
 void CoreOobeHandler::OnDeviceInfoUpdated(const std::string& bluetooth_name) {
@@ -334,7 +359,12 @@
 
 void CoreOobeHandler::UpdateLabel(const std::string& id,
                                   const std::string& text) {
-  CallJS("cr.ui.Oobe.setLabelText", id, text);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setLabelText", id, text);
+  } else {
+    LOG(ERROR) << "Silently dropping UpdateLabel request.";
+  }
 }
 
 void CoreOobeHandler::UpdateKeyboardState() {
@@ -381,7 +411,12 @@
     default:
       NOTREACHED();
   }
-  CallJS("cr.ui.Oobe.setDialogPaddingMode", padding);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("cr.ui.Oobe.setDialogPaddingMode", padding);
+  } else {
+    LOG(ERROR) << "Silently dropping SetDialogPaddingMode request.";
+  }
 }
 
 void CoreOobeHandler::OnOobeConfigurationChanged() {
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 af0674a..1e939a5 100644
--- a/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
@@ -61,12 +61,22 @@
 
 void ErrorScreenHandler::SetErrorStateCode(
     NetworkError::ErrorState error_state) {
-  CallJS("login.ErrorMessageScreen.setErrorState",
-         static_cast<int>(error_state));
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("login.ErrorMessageScreen.setErrorState",
+           static_cast<int>(error_state));
+  } else {
+    LOG(ERROR) << "Silently dropping SetErrorStateNetwork request.";
+  }
 }
 
 void ErrorScreenHandler::SetErrorStateNetwork(const std::string& network_name) {
-  CallJS("login.ErrorMessageScreen.setErrorStateNetwork", network_name);
+  // TODO(crbug.com/1180291) - Remove once OOBE JS calls are fixed.
+  if (IsJavascriptAllowed()) {
+    CallJS("login.ErrorMessageScreen.setErrorStateNetwork", network_name);
+  } else {
+    LOG(ERROR) << "Silently dropping SetErrorStateNetwork request.";
+  }
 }
 
 void ErrorScreenHandler::SetGuestSigninAllowed(bool value) {