From 38731b771e89f65a14ab665f25c030cb895bc2c3 Mon Sep 17 00:00:00 2001 From: Shivan Kaul Sahib Date: Wed, 17 Jul 2024 17:51:21 -0700 Subject: [PATCH 1/5] Re-enable ReportingObserver and make noop --- .../reporting_observer_browsertest.cc | 4 +-- .../context_feature_settings.h | 30 ------------------- ..._features-context_feature_settings.h.patch | 12 -------- ...rer-core-frame-reporting_observer.cc.patch | 12 ++++++++ ...er-core-frame-reporting_observer.idl.patch | 12 -------- test/data/reporting_observer.html | 22 ++++++++++---- 6 files changed, 30 insertions(+), 62 deletions(-) delete mode 100644 chromium_src/third_party/blink/renderer/core/context_features/context_feature_settings.h delete mode 100644 patches/third_party-blink-renderer-core-context_features-context_feature_settings.h.patch create mode 100644 patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch delete mode 100644 patches/third_party-blink-renderer-core-frame-reporting_observer.idl.patch diff --git a/browser/test/disabled_features/reporting_observer_browsertest.cc b/browser/test/disabled_features/reporting_observer_browsertest.cc index 0a763a463f54..1c7f24716753 100644 --- a/browser/test/disabled_features/reporting_observer_browsertest.cc +++ b/browser/test/disabled_features/reporting_observer_browsertest.cc @@ -32,11 +32,11 @@ class ReportingObserverTest : public InProcessBrowserTest { } }; -IN_PROC_BROWSER_TEST_F(ReportingObserverTest, IsDisabled) { +IN_PROC_BROWSER_TEST_F(ReportingObserverTest, IsNoop) { GURL url = embedded_test_server()->GetURL(kReportingObserver); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - EXPECT_EQ(true, EvalJs(contents, "isReportingObserverDisabled();")); + EXPECT_EQ(true, EvalJs(contents, "isReportingObserverNoop();")); } diff --git a/chromium_src/third_party/blink/renderer/core/context_features/context_feature_settings.h b/chromium_src/third_party/blink/renderer/core/context_features/context_feature_settings.h deleted file mode 100644 index 1a890f51f0cd..000000000000 --- a/chromium_src/third_party/blink/renderer/core/context_features/context_feature_settings.h +++ /dev/null @@ -1,30 +0,0 @@ -/* Copyright (c) 2020 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_THIRD_PARTY_BLINK_RENDERER_CORE_CONTEXT_FEATURES_CONTEXT_FEATURE_SETTINGS_H_ -#define BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_CONTEXT_FEATURES_CONTEXT_FEATURE_SETTINGS_H_ - -// ContextEnabled=ReportingObservers feature methods need to be added since we -// patched ReportingObservers as ContextEnabled into -// third_party/blink/renderer/core/frame/reporting_observer.idl -#define BRAVE_CONTEXT_FEATURE_SETTINGS_H_ \ - public: \ - void enableReportingObservers(bool enable) { \ - enable_reporting_observers_ = enable; \ - } \ - bool isReportingObserversEnabled() const { \ - return enable_reporting_observers_; \ - } \ - \ - private: \ - bool enable_reporting_observers_ = false; \ - \ - public: -// #define BRAVE_CONTEXT_FEATURE_SETTINGS_H_ - -#include "src/third_party/blink/renderer/core/context_features/context_feature_settings.h" // IWYU pragma: export -#undef BRAVE_CONTEXT_FEATURE_SETTINGS_H_ - -#endif // BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_CONTEXT_FEATURES_CONTEXT_FEATURE_SETTINGS_H_ diff --git a/patches/third_party-blink-renderer-core-context_features-context_feature_settings.h.patch b/patches/third_party-blink-renderer-core-context_features-context_feature_settings.h.patch deleted file mode 100644 index fd0574ea6140..000000000000 --- a/patches/third_party-blink-renderer-core-context_features-context_feature_settings.h.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/third_party/blink/renderer/core/context_features/context_feature_settings.h b/third_party/blink/renderer/core/context_features/context_feature_settings.h -index 096d49ecd53d7e17811ad7d70ea6a9b2af9acd6f..eed64a36e45779db4a16e75f40b02b76b480673a 100644 ---- a/third_party/blink/renderer/core/context_features/context_feature_settings.h -+++ b/third_party/blink/renderer/core/context_features/context_feature_settings.h -@@ -64,6 +64,7 @@ class CORE_EXPORT ContextFeatureSettings final - } - - void Trace(Visitor*) const override; -+ BRAVE_CONTEXT_FEATURE_SETTINGS_H_ - - private: - bool enable_mojo_js_ = false; diff --git a/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch b/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch new file mode 100644 index 000000000000..a08d007f6ce5 --- /dev/null +++ b/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch @@ -0,0 +1,12 @@ +diff --git a/third_party/blink/renderer/core/frame/reporting_observer.cc b/third_party/blink/renderer/core/frame/reporting_observer.cc +index 89c04a3d0f7261aeca47f10040bfaf55ce82dd32..e480a51973c72334a879eabaf71a3cb27b6bacd5 100644 +--- a/third_party/blink/renderer/core/frame/reporting_observer.cc ++++ b/third_party/blink/renderer/core/frame/reporting_observer.cc +@@ -46,6 +46,7 @@ void ReportingObserver::ReportToCallback() { + } + + void ReportingObserver::QueueReport(Report* report) { ++ if ((true)) return; + if (!ObservedType(report->type())) + return; + diff --git a/patches/third_party-blink-renderer-core-frame-reporting_observer.idl.patch b/patches/third_party-blink-renderer-core-frame-reporting_observer.idl.patch deleted file mode 100644 index 341fe1e7bac9..000000000000 --- a/patches/third_party-blink-renderer-core-frame-reporting_observer.idl.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/third_party/blink/renderer/core/frame/reporting_observer.idl b/third_party/blink/renderer/core/frame/reporting_observer.idl -index 90d04d7f35828629d71124cdd0024baebd7d4fb3..094d82aa0d3ba6b70c112907c94edc49f1003506 100644 ---- a/third_party/blink/renderer/core/frame/reporting_observer.idl -+++ b/third_party/blink/renderer/core/frame/reporting_observer.idl -@@ -7,6 +7,7 @@ - callback ReportingObserverCallback = void (sequence reports, ReportingObserver observer); - - [ -+ ContextEnabled=ReportingObservers, - ActiveScriptWrappable, - Exposed=(Window,Worker) - ] interface ReportingObserver { diff --git a/test/data/reporting_observer.html b/test/data/reporting_observer.html index b8aab550560e..9c98ecc89e06 100644 --- a/test/data/reporting_observer.html +++ b/test/data/reporting_observer.html @@ -1,11 +1,21 @@ From 5cd781ab475acc81a67d57d218c27871dcd9de7d Mon Sep 17 00:00:00 2001 From: Shivan Kaul Sahib Date: Thu, 18 Jul 2024 00:52:29 -0700 Subject: [PATCH 2/5] Remove patch --- .../renderer/core/frame/reporting_observer.cc | 18 ++++++++++++++++++ .../renderer/core/frame/reporting_observer.h | 17 +++++++++++++++++ ...erer-core-frame-reporting_observer.cc.patch | 12 ------------ 3 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc create mode 100644 chromium_src/third_party/blink/renderer/core/frame/reporting_observer.h delete mode 100644 patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch diff --git a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc new file mode 100644 index 000000000000..350cfc70025c --- /dev/null +++ b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc @@ -0,0 +1,18 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#include "third_party/blink/renderer/core/frame/reporting_observer.h" + +#define QueueReport QueueReport_Unused +#include "src/third_party/blink/renderer/core/frame/reporting_observer.cc" +#undef QueueReport + +namespace blink { + +void ReportingObserver::QueueReport(Report* report) { + return; +} + +} // namespace blink diff --git a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.h b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.h new file mode 100644 index 000000000000..c859462458fd --- /dev/null +++ b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_REPORTING_OBSERVER_H_ +#define BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_REPORTING_OBSERVER_H_ + +#define QueueReport \ + QueueReport_Unused(Report* report); \ + void QueueReport + +#include "src/third_party/blink/renderer/core/frame/reporting_observer.h" // IWYU pragma: export + +#undef QueueReport + +#endif // BRAVE_CHROMIUM_SRC_THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_REPORTING_OBSERVER_H_ diff --git a/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch b/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch deleted file mode 100644 index a08d007f6ce5..000000000000 --- a/patches/third_party-blink-renderer-core-frame-reporting_observer.cc.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/third_party/blink/renderer/core/frame/reporting_observer.cc b/third_party/blink/renderer/core/frame/reporting_observer.cc -index 89c04a3d0f7261aeca47f10040bfaf55ce82dd32..e480a51973c72334a879eabaf71a3cb27b6bacd5 100644 ---- a/third_party/blink/renderer/core/frame/reporting_observer.cc -+++ b/third_party/blink/renderer/core/frame/reporting_observer.cc -@@ -46,6 +46,7 @@ void ReportingObserver::ReportToCallback() { - } - - void ReportingObserver::QueueReport(Report* report) { -+ if ((true)) return; - if (!ObservedType(report->type())) - return; - From d61f38028d74cf09e2cbd51f20e4c3bfe51bdf56 Mon Sep 17 00:00:00 2001 From: Shivan Kaul Sahib Date: Thu, 18 Jul 2024 11:53:55 -0700 Subject: [PATCH 3/5] Add a comment --- .../blink/renderer/core/frame/reporting_observer.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc index 350cfc70025c..aa609c0c5aed 100644 --- a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc +++ b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc @@ -12,6 +12,9 @@ namespace blink { void ReportingObserver::QueueReport(Report* report) { + // Don't add reports. We previously used to disable ReportingObserver in Brave, + // but for webcompat reasons, we now just no-op it. + // This makes takeRecords() always return an empty list. return; } From 35fd324a7e4ccbffe4462c08953fd8400ca8984c Mon Sep 17 00:00:00 2001 From: Shivan Kaul Sahib Date: Thu, 18 Jul 2024 11:54:43 -0700 Subject: [PATCH 4/5] Presubmit fix for comment --- .../blink/renderer/core/frame/reporting_observer.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc index aa609c0c5aed..77e8c5ee9ce8 100644 --- a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc +++ b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc @@ -12,9 +12,9 @@ namespace blink { void ReportingObserver::QueueReport(Report* report) { - // Don't add reports. We previously used to disable ReportingObserver in Brave, - // but for webcompat reasons, we now just no-op it. - // This makes takeRecords() always return an empty list. + // Don't add reports. We previously used to disable ReportingObserver in + // Brave, but for webcompat reasons, we now just no-op it. This makes + // takeRecords() always return an empty list. return; } From 95110add0e106043575256600d3dd5201dcf9a78 Mon Sep 17 00:00:00 2001 From: Shivan Kaul Sahib Date: Thu, 18 Jul 2024 13:36:37 -0700 Subject: [PATCH 5/5] Remove return stmt --- .../blink/renderer/core/frame/reporting_observer.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc index 77e8c5ee9ce8..60708af53918 100644 --- a/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc +++ b/chromium_src/third_party/blink/renderer/core/frame/reporting_observer.cc @@ -11,11 +11,9 @@ namespace blink { -void ReportingObserver::QueueReport(Report* report) { - // Don't add reports. We previously used to disable ReportingObserver in - // Brave, but for webcompat reasons, we now just no-op it. This makes - // takeRecords() always return an empty list. - return; -} +// Don't add reports. We previously used to disable ReportingObserver in +// Brave, but for webcompat reasons, we now just no-op it. This makes +// takeRecords() always return an empty list. +void ReportingObserver::QueueReport(Report* report) {} } // namespace blink