From f9c64b21b70aef7daed69ec7966fd7a7c87506eb Mon Sep 17 00:00:00 2001 From: mkarolin Date: Wed, 20 Jan 2021 18:36:10 -0500 Subject: [PATCH] PermissionRequestType enum was replaced. Split into RequestType and UMA. Chromium change: https://source.chromium.org/chromium/chromium/src/+/362cce49672ec40f804268bb817085125f31dc39 commit 362cce49672ec40f804268bb817085125f31dc39 Author: Bret Sepulveda Date: Wed Jan 13 18:47:54 2021 +0000 Add new enum permissions::RequestType, replacing PermissionRequestType. RequestType is a new enum in the permissions package that is not burdened by UMA backwards-compatibility like PermissionRequestType is. PermissionRequestType is renamed to RequestTypeForUma and is now only used inside permission_uma_util.h/cc (and related tests). RequestType also replaces most usages of ContentSettingsType within the permission package. Prompt icons are now fetched statically via the RequestType. RequestType will eventually also represent combined requests (for example, camera and microphone together) but this will be done in a future patch. Bug: 1110905 --- .../widevine/widevine_permission_request.cc | 11 +++------ .../widevine/widevine_permission_request.h | 3 +-- .../permission_prompt_bubble_view.cc | 4 ++-- .../permissions/permission_uma_util.cc | 23 ++++++++++++++----- ..._request_enums.h => permission_uma_util.h} | 15 +++++------- .../components/permissions/request_type.cc | 13 +++++++++++ .../components/permissions/request_type.h | 23 +++++++++++++++++++ ...rmissions-permission_request_enums.h.patch | 12 ---------- ...s-permissions-permission_uma_util.cc.patch | 16 +++++++++---- ...ts-permissions-permission_uma_util.h.patch | 12 ++++++++++ ...mponents-permissions-request_type.cc.patch | 12 ++++++++++ ...omponents-permissions-request_type.h.patch | 12 ++++++++++ 12 files changed, 112 insertions(+), 44 deletions(-) rename chromium_src/components/permissions/{permission_request_enums.h => permission_uma_util.h} (52%) create mode 100644 chromium_src/components/permissions/request_type.cc create mode 100644 chromium_src/components/permissions/request_type.h delete mode 100644 patches/components-permissions-permission_request_enums.h.patch create mode 100644 patches/components-permissions-permission_uma_util.h.patch create mode 100644 patches/components-permissions-request_type.cc.patch create mode 100644 patches/components-permissions-request_type.h.patch diff --git a/browser/widevine/widevine_permission_request.cc b/browser/widevine/widevine_permission_request.cc index 991bfcbfb05..5f78661e71f 100644 --- a/browser/widevine/widevine_permission_request.cc +++ b/browser/widevine/widevine_permission_request.cc @@ -8,6 +8,7 @@ #include "brave/browser/widevine/widevine_utils.h" #include "brave/grit/brave_generated_resources.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "components/permissions/request_type.h" #include "components/vector_icons/vector_icons.h" #include "content/public/browser/web_contents.h" #include "ui/base/l10n/l10n_util.h" @@ -22,11 +23,6 @@ WidevinePermissionRequest::WidevinePermissionRequest( WidevinePermissionRequest::~WidevinePermissionRequest() = default; -permissions::PermissionRequest::IconId WidevinePermissionRequest::GetIconId() - const { - return vector_icons::kExtensionIcon; -} - base::string16 WidevinePermissionRequest::GetMessageTextFragment() const { return l10n_util::GetStringUTF16( GetWidevinePermissionRequestTextFrangmentResourceId(for_restart_)); @@ -60,9 +56,8 @@ void WidevinePermissionRequest::RequestFinished() { delete this; } -permissions::PermissionRequestType -WidevinePermissionRequest::GetPermissionRequestType() const { - return permissions::PermissionRequestType::PERMISSION_WIDEVINE; +permissions::RequestType WidevinePermissionRequest::GetRequestType() const { + return permissions::RequestType::kWidevine; } base::string16 WidevinePermissionRequest::GetExplanatoryMessageText() const { diff --git a/browser/widevine/widevine_permission_request.h b/browser/widevine/widevine_permission_request.h index f7b5e33be94..5fedbc5361a 100644 --- a/browser/widevine/widevine_permission_request.h +++ b/browser/widevine/widevine_permission_request.h @@ -32,14 +32,13 @@ class WidevinePermissionRequest : public permissions::PermissionRequest { static bool is_test_; // PermissionRequest overrides: - permissions::PermissionRequest::IconId GetIconId() const override; base::string16 GetMessageTextFragment() const override; GURL GetOrigin() const override; void PermissionGranted(bool is_one_time) override; void PermissionDenied() override; void Cancelled() override; void RequestFinished() override; - permissions::PermissionRequestType GetPermissionRequestType() const override; + permissions::RequestType GetRequestType() const override; // It's safe to use this raw |web_contents_| because this request is deleted // by PermissionManager that is tied with this |web_contents_|. diff --git a/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.cc b/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.cc index 5f880412ddc..a5ae9d5d1a9 100644 --- a/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.cc +++ b/chromium_src/chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.cc @@ -14,6 +14,7 @@ #include "brave/browser/widevine/widevine_permission_request.h" #include "brave/grit/brave_generated_resources.h" #include "chrome/browser/ui/views/chrome_layout_provider.h" +#include "components/permissions/request_type.h" #include "ui/gfx/text_constants.h" #include "ui/base/l10n/l10n_util.h" #include "ui/views/controls/button/checkbox.h" @@ -52,8 +53,7 @@ bool HasWidevinePermissionRequest( // When widevine permission is requested, |requests| only includes Widevine // permission because it is not a candidate for grouping. if (requests.size() == 1 && - requests[0]->GetPermissionRequestType() == - permissions::PermissionRequestType::PERMISSION_WIDEVINE) + requests[0]->GetRequestType() == permissions::RequestType::kWidevine) return true; return false; diff --git a/chromium_src/components/permissions/permission_uma_util.cc b/chromium_src/components/permissions/permission_uma_util.cc index 4e13f97bacd..612dac849be 100644 --- a/chromium_src/components/permissions/permission_uma_util.cc +++ b/chromium_src/components/permissions/permission_uma_util.cc @@ -3,26 +3,37 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "components/permissions/permission_request.h" +#include "components/permissions/permission_uma_util.h" + +#include "build/build_config.h" namespace permissions { namespace { -std::string GetPermissionRequestString(PermissionRequestType type); -std::string GetPermissionRequestString_ChromiumImpl(PermissionRequestType type); +std::string GetPermissionRequestString(RequestTypeForUma type); +std::string GetPermissionRequestString_ChromiumImpl(RequestTypeForUma type); } // namespace } // namespace permissions +#if !defined(OS_ANDROID) && !defined(OS_IOS) +#define BRAVE_GET_UMA_VALUE_FOR_REQUEST_TYPE \ + case RequestType::kWidevine: \ + return RequestTypeForUma::PERMISSION_WIDEVINE; +#else +#define BRAVE_GET_UMA_VALUE_FOR_REQUEST_TYPE +#endif + #include "../../../../components/permissions/permission_uma_util.cc" +#undef BRAVE_GET_UMA_VALUE_FOR_REQUEST_TYPE namespace permissions { namespace { -std::string GetPermissionRequestString(PermissionRequestType type) { - if (type == PermissionRequestType::PERMISSION_WIDEVINE) +std::string GetPermissionRequestString(RequestTypeForUma type) { + if (type == RequestTypeForUma::PERMISSION_WIDEVINE) return "Widevine"; - if (type == PermissionRequestType::PERMISSION_WALLET) + if (type == RequestTypeForUma::PERMISSION_WALLET) return "Wallet"; return GetPermissionRequestString_ChromiumImpl(type); } diff --git a/chromium_src/components/permissions/permission_request_enums.h b/chromium_src/components/permissions/permission_uma_util.h similarity index 52% rename from chromium_src/components/permissions/permission_request_enums.h rename to chromium_src/components/permissions/permission_uma_util.h index 853c7f335f2..718e4218177 100644 --- a/chromium_src/components/permissions/permission_request_enums.h +++ b/chromium_src/components/permissions/permission_uma_util.h @@ -3,19 +3,16 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_ENUMS_H_ -#define BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_ENUMS_H_ +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_UMA_UTIL_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_UMA_UTIL_H_ -#if defined(BRAVE_CHROMIUM_BUILD) // clang-format off -#define BRAVE_PERMISSION_REQUEST_TYPES \ +#define BRAVE_PERMISSION_REQUEST_TYPES_FOR_UMA \ PERMISSION_WIDEVINE, \ PERMISSION_WALLET, // clang-format on -#else -#define BRAVE_PERMISSION_REQUEST_TYPES -#endif -#include "../../../../components/permissions/permission_request_enums.h" +#include "../../../../components/permissions/permission_uma_util.h" +#undef BRAVE_PERMISSION_REQUEST_TYPES_FOR_UMA -#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_ENUMS_H_ +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_PERMISSION_UMA_UTIL_H_ diff --git a/chromium_src/components/permissions/request_type.cc b/chromium_src/components/permissions/request_type.cc new file mode 100644 index 00000000000..3c61565231c --- /dev/null +++ b/chromium_src/components/permissions/request_type.cc @@ -0,0 +1,13 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "components/permissions/request_type.h" + +#define BRAVE_GET_ICON_ID_DESKTOP \ + case RequestType::kWidevine: \ + return vector_icons::kExtensionIcon; + +#include "../../../../components/permissions/request_type.cc" +#undef BRAVE_GET_ICON_ID_DESKTOP diff --git a/chromium_src/components/permissions/request_type.h b/chromium_src/components/permissions/request_type.h new file mode 100644 index 00000000000..5e8fb0f4a87 --- /dev/null +++ b/chromium_src/components/permissions/request_type.h @@ -0,0 +1,23 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_REQUEST_TYPE_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_REQUEST_TYPE_H_ + +#include "build/build_config.h" + +// clang-format off +#if !defined(OS_ANDROID) && !defined(OS_IOS) +#define BRAVE_REQUEST_TYPES \ + kWidevine, +#else +#define BRAVE_REQUEST_TYPES +#endif +// clang-format on + +#include "../../../../components/permissions/request_type.h" +#undef BRAVE_REQUEST_TYPES + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_PERMISSIONS_REQUEST_TYPE_H_ diff --git a/patches/components-permissions-permission_request_enums.h.patch b/patches/components-permissions-permission_request_enums.h.patch deleted file mode 100644 index f9f45c7f014..00000000000 --- a/patches/components-permissions-permission_request_enums.h.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/components/permissions/permission_request_enums.h b/components/permissions/permission_request_enums.h -index 8bc20927e8518d212e033655a1a7a87c9a293580..6528d9381b3017a321519b3e8c19ae89cf6ebfd0 100644 ---- a/components/permissions/permission_request_enums.h -+++ b/components/permissions/permission_request_enums.h -@@ -48,6 +48,7 @@ enum class PermissionRequestType { - PERMISSION_WINDOW_PLACEMENT = 25, - PERMISSION_FONT_ACCESS = 26, - PERMISSION_IDLE_DETECTION = 27, -+ BRAVE_PERMISSION_REQUEST_TYPES - // NUM must be the last value in the enum. - NUM - }; diff --git a/patches/components-permissions-permission_uma_util.cc.patch b/patches/components-permissions-permission_uma_util.cc.patch index 8cfb6a2c007..9451f0fbaca 100644 --- a/patches/components-permissions-permission_uma_util.cc.patch +++ b/patches/components-permissions-permission_uma_util.cc.patch @@ -1,13 +1,19 @@ diff --git a/components/permissions/permission_uma_util.cc b/components/permissions/permission_uma_util.cc -index 64042b1fc8cf906789d526d6c7a99dbc2ded9d76..4d3bc36bc3b50b2df0872365ef2c61ea88f4cc78 100644 +index 68828c19b1660fcded7984869c7f33b7f447e5c4..65faa52335b4f0d3e928356c157c2d1ac4d74b8a 100644 --- a/components/permissions/permission_uma_util.cc +++ b/components/permissions/permission_uma_util.cc -@@ -50,7 +50,7 @@ namespace { +@@ -103,12 +103,13 @@ RequestTypeForUma GetUmaValueForRequestType(RequestType request_type) { + case RequestType::kWindowPlacement: + return RequestTypeForUma::PERMISSION_WINDOW_PLACEMENT; + #endif ++ BRAVE_GET_UMA_VALUE_FOR_REQUEST_TYPE + } + } const int kPriorCountCap = 10; --std::string GetPermissionRequestString(PermissionRequestType type) { -+std::string GetPermissionRequestString_ChromiumImpl(PermissionRequestType type) { +-std::string GetPermissionRequestString(RequestTypeForUma type) { ++std::string GetPermissionRequestString_ChromiumImpl(RequestTypeForUma type) { switch (type) { - case PermissionRequestType::MULTIPLE: + case RequestTypeForUma::MULTIPLE: return "AudioAndVideoCapture"; diff --git a/patches/components-permissions-permission_uma_util.h.patch b/patches/components-permissions-permission_uma_util.h.patch new file mode 100644 index 00000000000..030b5ced8bb --- /dev/null +++ b/patches/components-permissions-permission_uma_util.h.patch @@ -0,0 +1,12 @@ +diff --git a/components/permissions/permission_uma_util.h b/components/permissions/permission_uma_util.h +index f2fe29c36726b73a92c8693466240e92340415c3..5b730ad861a80b9b315350e58bf47b6a4f8764aa 100644 +--- a/components/permissions/permission_uma_util.h ++++ b/components/permissions/permission_uma_util.h +@@ -66,6 +66,7 @@ enum class RequestTypeForUma { + PERMISSION_WINDOW_PLACEMENT = 25, + PERMISSION_FONT_ACCESS = 26, + PERMISSION_IDLE_DETECTION = 27, ++ BRAVE_PERMISSION_REQUEST_TYPES_FOR_UMA + // NUM must be the last value in the enum. + NUM + }; diff --git a/patches/components-permissions-request_type.cc.patch b/patches/components-permissions-request_type.cc.patch new file mode 100644 index 00000000000..b88bed2fc4d --- /dev/null +++ b/patches/components-permissions-request_type.cc.patch @@ -0,0 +1,12 @@ +diff --git a/components/permissions/request_type.cc b/components/permissions/request_type.cc +index c0d6d512e08bf2d9382f562592b1f9bbef63c439..d2a4eb628c33826fc2a93a2e0ed78f2bdf4c67d9 100644 +--- a/components/permissions/request_type.cc ++++ b/components/permissions/request_type.cc +@@ -102,6 +102,7 @@ const gfx::VectorIcon& GetIconIdDesktop(RequestType type) { + return vector_icons::kCookieIcon; + case RequestType::kWindowPlacement: + return vector_icons::kSelectWindowIcon; ++ BRAVE_GET_ICON_ID_DESKTOP + } + NOTREACHED(); + return gfx::kNoneIcon; diff --git a/patches/components-permissions-request_type.h.patch b/patches/components-permissions-request_type.h.patch new file mode 100644 index 00000000000..2e418f76bfd --- /dev/null +++ b/patches/components-permissions-request_type.h.patch @@ -0,0 +1,12 @@ +diff --git a/components/permissions/request_type.h b/components/permissions/request_type.h +index c560723b184f2cc114f3c26787867c5ed6d73104..247511538d2a043d713541900e039de1afa7684e 100644 +--- a/components/permissions/request_type.h ++++ b/components/permissions/request_type.h +@@ -52,6 +52,7 @@ enum class RequestType { + #if !defined(OS_ANDROID) + kWindowPlacement, + #endif ++ BRAVE_REQUEST_TYPES + }; + + #if defined(OS_ANDROID)