[Merge 102] Privacy Sandbox: Defensively disallow Javascript in handler

CL adjusts the PrivacySandboxDialogHandler to be more defensive when
handling calls from the renderer. Specifically, any user initiated
action which would result in a call to a OnceCallback is made
idempotent.

This is achieved by disallowing Javascript on the first such call, and
confirming JavaScript is allowed in subsequent calls.

(cherry picked from commit 1a44290)

Bug: 1323611
Change-Id: I477bbbec6469dbd705afe2cdf7662f8603ca3970
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3634519
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1000996}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641763
Auto-Submit: Theodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Commit-Position: refs/branch-heads/5005@{#657}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}

chrome/browser/ui/webui/privacy_sandbox/privacy_sandbox_dialog_handler.cc

@@ -37,7 +37,12 @@ PrivacySandboxDialogHandler::PrivacySandboxDialogHandler(
       resize_callback_(std::move(resize_callback)),
       show_dialog_callback_(std::move(show_dialog_callback)),
       open_settings_callback_(std::move(open_settings_callback)),
-      dialog_type_(dialog_type) {}
+      dialog_type_(dialog_type) {
+  DCHECK(close_callback_);
+  DCHECK(resize_callback_);
+  DCHECK(show_dialog_callback_);
+  DCHECK(open_settings_callback_);
+}
 
 PrivacySandboxDialogHandler::~PrivacySandboxDialogHandler() {
   DisallowJavascript();
@@ -83,14 +88,15 @@ void PrivacySandboxDialogHandler::OnJavascriptDisallowed() {
 
 void PrivacySandboxDialogHandler::HandleDialogActionOccurred(
     const base::Value::List& args) {
+  if (!IsJavascriptAllowed())
+    return;
+
   CHECK_EQ(1U, args.size());
   auto action =
       static_cast<PrivacySandboxService::DialogAction>(args[0].GetInt());
 
-  if (action == PrivacySandboxService::DialogAction::kNoticeOpenSettings) {
-    DCHECK(open_settings_callback_);
+  if (action == PrivacySandboxService::DialogAction::kNoticeOpenSettings)
     std::move(open_settings_callback_).Run();
-  }
 
   bool covered_action = true;
   switch (action) {
@@ -131,7 +137,7 @@ void PrivacySandboxDialogHandler::HandleDialogActionOccurred(
 
   if (covered_action) {
     did_user_make_decision_ = true;
-    DCHECK(close_callback_);
+    DisallowJavascript();
     std::move(close_callback_).Run();
   }
 

chrome/browser/ui/webui/privacy_sandbox/privacy_sandbox_dialog_handler_unittest.cc

@@ -75,6 +75,21 @@ class PrivacySandboxDialogHandlerTest : public testing::Test {
     web_ui_.reset();
   }
 
+  void ShowDialog(PrivacySandboxService::DialogAction expected_action) {
+    EXPECT_CALL(*dialog_mock(), ShowNativeView());
+    EXPECT_CALL(*mock_privacy_sandbox_service(),
+                DialogActionOccurred(expected_action));
+    base::Value args(base::Value::Type::LIST);
+    handler()->HandleShowDialog(args.GetList());
+  }
+
+  void IdempotentDialogActionOccurred(const base::Value::List& args) {
+    // Inform the handler multiple times that a dialog action occurred. The test
+    // using this function expects the call to be idempotent.
+    handler()->HandleDialogActionOccurred(args);
+    handler()->HandleDialogActionOccurred(args);
+  }
+
   content::TestWebUI* web_ui() { return web_ui_.get(); }
   PrivacySandboxDialogHandler* handler() { return handler_.get(); }
   TestingProfile* profile() { return &profile_; }
@@ -138,22 +153,18 @@ TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleResizeDialog) {
 }
 
 TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleShowDialog) {
-  EXPECT_CALL(*dialog_mock(), ShowNativeView());
-  EXPECT_CALL(
-      *mock_privacy_sandbox_service(),
-      DialogActionOccurred(PrivacySandboxService::DialogAction::kConsentShown));
   EXPECT_CALL(
       *mock_privacy_sandbox_service(),
       DialogActionOccurred(
           PrivacySandboxService::DialogAction::kConsentClosedNoDecision));
 
-  base::Value args(base::Value::Type::LIST);
-  handler()->HandleShowDialog(args.GetList());
+  ShowDialog(PrivacySandboxService::DialogAction::kConsentShown);
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }
 
 TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleClickLearnMore) {
+  ShowDialog(PrivacySandboxService::DialogAction::kConsentShown);
   EXPECT_CALL(*mock_privacy_sandbox_service(),
               DialogActionOccurred(
                   PrivacySandboxService::DialogAction::kConsentMoreInfoOpened));
@@ -181,6 +192,7 @@ TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleClickLearnMore) {
 }
 
 TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleConsentAccepted) {
+  ShowDialog(PrivacySandboxService::DialogAction::kConsentShown);
   EXPECT_CALL(*dialog_mock(), Close());
   EXPECT_CALL(*mock_privacy_sandbox_service(),
               DialogActionOccurred(
@@ -200,12 +212,13 @@ TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleConsentAccepted) {
   base::Value args(base::Value::Type::LIST);
   args.Append(
       static_cast<int>(PrivacySandboxService::DialogAction::kConsentAccepted));
-  handler()->HandleDialogActionOccurred(args.GetList());
+  IdempotentDialogActionOccurred(args.GetList());
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }
 
 TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleConsentDeclined) {
+  ShowDialog(PrivacySandboxService::DialogAction::kConsentShown);
   EXPECT_CALL(*dialog_mock(), Close());
   EXPECT_CALL(*mock_privacy_sandbox_service(),
               DialogActionOccurred(
@@ -225,7 +238,7 @@ TEST_F(PrivacySandboxConsentDialogHandlerTest, HandleConsentDeclined) {
   base::Value args(base::Value::Type::LIST);
   args.Append(
       static_cast<int>(PrivacySandboxService::DialogAction::kConsentDeclined));
-  handler()->HandleDialogActionOccurred(args.GetList());
+  IdempotentDialogActionOccurred(args.GetList());
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }
@@ -267,22 +280,17 @@ TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleResizeDialog) {
 }
 
 TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleShowDialog) {
-  EXPECT_CALL(*dialog_mock(), ShowNativeView());
-  EXPECT_CALL(
-      *mock_privacy_sandbox_service(),
-      DialogActionOccurred(PrivacySandboxService::DialogAction::kNoticeShown));
   EXPECT_CALL(
       *mock_privacy_sandbox_service(),
       DialogActionOccurred(
           PrivacySandboxService::DialogAction::kNoticeClosedNoInteraction));
-
-  base::Value args(base::Value::Type::LIST);
-  handler()->HandleShowDialog(args.GetList());
+  ShowDialog(PrivacySandboxService::DialogAction::kNoticeShown);
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }
 
 TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleOpenSettings) {
+  ShowDialog(PrivacySandboxService::DialogAction::kNoticeShown);
   EXPECT_CALL(*dialog_mock(), OpenPrivacySandboxSettings());
   EXPECT_CALL(*dialog_mock(), Close());
   EXPECT_CALL(*mock_privacy_sandbox_service(),
@@ -303,12 +311,13 @@ TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleOpenSettings) {
   base::Value args(base::Value::Type::LIST);
   args.Append(static_cast<int>(
       PrivacySandboxService::DialogAction::kNoticeOpenSettings));
-  handler()->HandleDialogActionOccurred(args.GetList());
+  IdempotentDialogActionOccurred(args.GetList());
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }
 
 TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleNoticeAcknowledge) {
+  ShowDialog(PrivacySandboxService::DialogAction::kNoticeShown);
   EXPECT_CALL(*dialog_mock(), Close());
   EXPECT_CALL(*mock_privacy_sandbox_service(),
               DialogActionOccurred(
@@ -328,7 +337,7 @@ TEST_F(PrivacySandboxNoticeDialogHandlerTest, HandleNoticeAcknowledge) {
   base::Value args(base::Value::Type::LIST);
   args.Append(static_cast<int>(
       PrivacySandboxService::DialogAction::kNoticeAcknowledge));
-  handler()->HandleDialogActionOccurred(args.GetList());
+  IdempotentDialogActionOccurred(args.GetList());
 
   ASSERT_EQ(0U, web_ui()->call_data().size());
 }