Skip to content

Commit

Permalink
Revert "[DTC] Add Device Signals Disclosure to chrome://management"
Browse files Browse the repository at this point in the history
This reverts commit 032d4ed.

Reason for revert: ManagementUIHandlerTests failing on MSan bots.

Bug:1449560

Original change's description:
> [DTC] Add Device Signals Disclosure to chrome://management
>
> Add a disclosure when device signals are being shared in the chrome://management page
>
> Bug:b:277901759
>
> Change-Id: I7bab126b3fbc79702f8088501452d2c153888bdd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4549539
> Reviewed-by: Sébastien Lalancette <seblalancette@chromium.org>
> Reviewed-by: Yann Dago <ydago@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Zonghan Xu <xzonghan@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1149839}

Bug: b:277901759
Change-Id: I3ea9ea63d155a2cfe3dc5df96ebf9182e1e80acb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4570944
Owners-Override: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1150191}
  • Loading branch information
horo-t authored and Chromium LUCI CQ committed May 29, 2023
1 parent 723d765 commit f0798ad
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 89 deletions.
4 changes: 0 additions & 4 deletions chrome/browser/ui/webui/management/management_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ content::WebUIDataSource* CreateAndAddManagementUIHtmlSource(Profile* profile) {
{kManagementScreenCaptureEvent, IDS_MANAGEMENT_SCREEN_CAPTURE_EVENT},
{kManagementScreenCaptureData, IDS_MANAGEMENT_SCREEN_CAPTURE_DATA},
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
{kManagementDeviceSignalsDisclosure,
IDS_MANAGEMENT_DEVICE_SIGNALS_DISCLOSURE},
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
{"browserReporting", IDS_MANAGEMENT_BROWSER_REPORTING},
{"browserReportingExplanation",
IDS_MANAGEMENT_BROWSER_REPORTING_EXPLANATION},
Expand Down
30 changes: 0 additions & 30 deletions chrome/browser/ui/webui/management/management_ui_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@
#include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
#include "chrome/browser/enterprise/signals/user_permission_service_factory.h"
#include "components/device_signals/core/browser/user_permission_service.h" // nogncheck
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#include "build/chromeos_buildflags.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/common/extensions/permissions/chrome_permission_message_provider.h"
Expand Down Expand Up @@ -183,11 +178,6 @@ const char kManagementScreenCaptureEvent[] = "managementScreenCaptureEvent";
const char kManagementScreenCaptureData[] = "managementScreenCaptureData";
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
const char kManagementDeviceSignalsDisclosure[] =
"managementDeviceSignalsDisclosure";
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_CHROMEOS)
const char kManagementLogUploadEnabled[] = "managementLogUploadEnabled";
const char kManagementReportActivityTimes[] = "managementReportActivityTimes";
Expand Down Expand Up @@ -738,18 +728,6 @@ void ManagementUIHandler::AddReportingInfo(base::Value::List* report_sources) {
GetReportingTypeValue(report_definition.reporting_type));
report_sources->Append(std::move(data));
}
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
// Insert the device signals consent disclosure at the end of browser
// reporting section.
auto* user_permission_service = GetUserPermissionService();
if (user_permission_service && user_permission_service->CanCollectSignals() ==
device_signals::UserPermission::kGranted) {
base::Value::Dict data;
data.Set("messageId", kManagementDeviceSignalsDisclosure);
data.Set("reportingType", GetReportingTypeValue(ReportingType::kDevice));
report_sources->Append(std::move(data));
}
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down Expand Up @@ -1047,14 +1025,6 @@ policy::PolicyService* ManagementUIHandler::GetPolicyService() {
->policy_service();
}

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
device_signals::UserPermissionService*
ManagementUIHandler::GetUserPermissionService() {
return enterprise_signals::UserPermissionServiceFactory::GetForProfile(
Profile::FromWebUI(web_ui()));
}
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

void ManagementUIHandler::AsyncUpdateLogo() {
#if BUILDFLAG(IS_CHROMEOS_ASH)
policy::BrowserPolicyConnectorAsh* connector =
Expand Down
13 changes: 0 additions & 13 deletions chrome/browser/ui/webui/management/management_ui_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ extern const char kManagementScreenCaptureEvent[];
extern const char kManagementScreenCaptureData[];
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
extern const char kManagementDeviceSignalsDisclosure[];
#endif // #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_CHROMEOS)
extern const char kManagementLogUploadEnabled[];
extern const char kManagementReportActivityTimes[];
Expand Down Expand Up @@ -114,12 +110,6 @@ class StatusCollector;
class SystemLogUploader;
} // namespace policy

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
namespace device_signals {
class UserPermissionService;
} // namespace device_signals
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

class Profile;

// The JavaScript message handler for the chrome://management page.
Expand Down Expand Up @@ -173,9 +163,6 @@ class ManagementUIHandler : public content::WebUIMessageHandler,
base::Value::Dict GetThreatProtectionInfo(Profile* profile);
base::Value::List GetManagedWebsitesInfo(Profile* profile) const;
virtual policy::PolicyService* GetPolicyService();
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
virtual device_signals::UserPermissionService* GetUserPermissionService();
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_CHROMEOS_ASH)
// Protected for testing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/webui/management/management_ui_handler.h"
#include <map>
#include <memory>
#include <set>
Expand All @@ -21,6 +20,7 @@
#include "build/chromeos_buildflags.h"
#include "chrome/browser/policy/dm_token_utils.h"
#include "chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.h"
#include "chrome/browser/ui/webui/management/management_ui_handler.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_profile.h"
#include "chromeos/ash/components/settings/cros_settings_names.h"
Expand Down Expand Up @@ -104,10 +104,6 @@
#include "services/network/test/test_network_connection_tracker.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
#include "components/device_signals/core/browser/mock_user_permission_service.h" // nogncheck
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chromeos/lacros/lacros_test_helper.h"
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
Expand Down Expand Up @@ -240,17 +236,6 @@ class TestManagementUIHandler : public ManagementUIHandler {

policy::PolicyService* GetPolicyService() override { return policy_service_; }

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
device_signals::UserPermissionService* GetUserPermissionService() override {
return user_permission_service_;
}

void SetUserPermissionService(
device_signals::UserPermissionService* user_permission_service) {
user_permission_service_ = user_permission_service;
}
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

#if BUILDFLAG(IS_CHROMEOS_ASH)
MOCK_METHOD(policy::DeviceCloudPolicyManagerAsh*,
GetDeviceCloudPolicyManager,
Expand All @@ -266,9 +251,6 @@ class TestManagementUIHandler : public ManagementUIHandler {
raw_ptr<policy::PolicyService> policy_service_ = nullptr;
bool update_required_eol_ = false;
std::string device_domain = "devicedomain.com";
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
raw_ptr<device_signals::UserPermissionService> user_permission_service_;
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
};

// We need to use a different base class for ChromeOS and non ChromeOS case.
Expand Down Expand Up @@ -529,9 +511,6 @@ class ManagementUIHandlerTests : public TestingBaseClass {
base::Value::Dict data =
handler_.GetContextualManagedDataForTesting(profile_.get());
ExtractContextualSourceUpdate(data);
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
handler_.SetUserPermissionService(&mock_user_permission_service_);
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
}

bool GetManaged() const { return extracted_.managed; }
Expand Down Expand Up @@ -640,9 +619,6 @@ class ManagementUIHandlerTests : public TestingBaseClass {
#if BUILDFLAG(IS_CHROMEOS_LACROS)
chromeos::ScopedLacrosServiceTestHelper scoped_lacros_test_helper_;
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
device_signals::MockUserPermissionService mock_user_permission_service_;
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
TestManagementUIHandler handler_;
};

Expand Down Expand Up @@ -1312,25 +1288,18 @@ TEST_F(ManagementUIHandlerTests, ExtensionReportingInfoNoPolicySetNoMessage) {
}

TEST_F(ManagementUIHandlerTests, CloudReportingPolicy) {
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
SetUpProfileAndHandler();
EXPECT_CALL(mock_user_permission_service_, CanCollectSignals())
.WillOnce(Return(device_signals::UserPermission::kGranted));
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
policy::PolicyMap chrome_policies;
const policy::PolicyNamespace chrome_policies_namespace =
policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string());
EXPECT_CALL(policy_service_, GetPolicies(_))
.WillRepeatedly(ReturnRef(chrome_policies));
SetPolicyValue(policy::key::kCloudReportingEnabled, true, chrome_policies);

std::set<std::string> expected_messages = {
const std::set<std::string> expected_messages = {
kManagementExtensionReportMachineName, kManagementExtensionReportUsername,
kManagementExtensionReportVersion,
kManagementExtensionReportExtensionsPlugin};
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
expected_messages.insert(kManagementDeviceSignalsDisclosure);
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)

ASSERT_PRED_FORMAT2(MessagesToBeEQ, handler_.GetExtensionReportingInfo(),
expected_messages);
}
Expand Down
7 changes: 0 additions & 7 deletions components/management_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -319,11 +319,4 @@
URLs of pages you visit are sent to Google Cloud or third parties for analysis. For example, they might be scanned to detect unsafe websites or filter websites based on rules set by the administrator.
</message>
</if>

<!-- Strings related to Chrome Enterprise Device Signals Sharing -->
<if expr="is_win or is_linux or is_macosx">
<message name="IDS_MANAGEMENT_DEVICE_SIGNALS_DISCLOSURE" desc="Disclosure message explaining that device signals can be shared.">
Information about your browser, OS, device, installed software, and files
</message>
</if>
</grit-part>

This file was deleted.

0 comments on commit f0798ad

Please sign in to comment.