From 3e6a038af77cc01e16bf139dc68dd8eb45483c7b Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 25 Jan 2024 01:12:54 +0100 Subject: [PATCH] fix: draggable regions not working (#41030) * fix: draggable regions not working * fix: only support app regions for main frame --------- Co-authored-by: deepak1556 --- patches/chromium/.patches | 1 + ..._working_after_navigation_in_wco_app.patch | 205 ++++++++++++++++++ .../electron_render_frame_observer.cc | 6 + 3 files changed, 212 insertions(+) create mode 100644 patches/chromium/fix_drag_regions_not_working_after_navigation_in_wco_app.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 1ddb9f6d1568..681baab2eec8 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -128,3 +128,4 @@ fix_restore_original_resize_performance_on_macos.patch feat_allow_code_cache_in_custom_schemes.patch enable_partition_alloc_ref_count_size.patch build_run_reclient_cfg_generator_after_chrome.patch +fix_drag_regions_not_working_after_navigation_in_wco_app.patch diff --git a/patches/chromium/fix_drag_regions_not_working_after_navigation_in_wco_app.patch b/patches/chromium/fix_drag_regions_not_working_after_navigation_in_wco_app.patch new file mode 100644 index 000000000000..118c2082f340 --- /dev/null +++ b/patches/chromium/fix_drag_regions_not_working_after_navigation_in_wco_app.patch @@ -0,0 +1,205 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Amanda Baker +Date: Wed, 17 Jan 2024 21:07:24 +0000 +Subject: fix: drag regions not working after navigation in WCO app + +crrev.com/c/4814003 caused a regression where draggable regions (set +using the app-region CSS property) did not work after a Window +Controls Overlay or Borderless app navigated to another url. + +This change fixes this issue by setting SupportsAppRegion to true on +each navigation within the app window. + +Bug: 1516830, 1447586 +Change-Id: I98019070f13ccd93a0f6db57a187eed00b460e81 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5179536 +Commit-Queue: Amanda Baker +Reviewed-by: Phillis Tang +Reviewed-by: Dmitry Gozman +Cr-Commit-Position: refs/heads/main@{#1248354} + +diff --git a/third_party/blink/renderer/core/exported/web_view_impl.cc b/third_party/blink/renderer/core/exported/web_view_impl.cc +index 0550bf54088ba18311ea13cea1555d22e2976a20..9c1359730ae381037102d2fe9950f1a19a679e54 100644 +--- a/third_party/blink/renderer/core/exported/web_view_impl.cc ++++ b/third_party/blink/renderer/core/exported/web_view_impl.cc +@@ -4016,7 +4016,24 @@ bool WebViewImpl::IsFencedFrameRoot() const { + } + + void WebViewImpl::SetSupportsAppRegion(bool supports_app_region) { +- MainFrameImpl()->GetFrame()->SetSupportsAppRegion(supports_app_region); ++ supports_app_region_ = supports_app_region; ++ if (!MainFrameImpl() || !MainFrameImpl()->GetFrame()) { ++ return; ++ } ++ ++ LocalFrame* local_frame = MainFrameImpl()->GetFrame(); ++ ++ if (supports_app_region_) { ++ local_frame->View()->UpdateDocumentAnnotatedRegions(); ++ } else { ++ local_frame->GetDocument()->SetAnnotatedRegions( ++ Vector()); ++ local_frame->Client()->AnnotatedRegionsChanged(); ++ } ++} ++ ++bool WebViewImpl::SupportsAppRegion() { ++ return supports_app_region_; + } + + void WebViewImpl::MojoDisconnected() { +diff --git a/third_party/blink/renderer/core/exported/web_view_impl.h b/third_party/blink/renderer/core/exported/web_view_impl.h +index c227b904fef4acc76a4af50263ab9d4fa35472e2..8dfb3b5b9026df92e28271258870c9eb588a6526 100644 +--- a/third_party/blink/renderer/core/exported/web_view_impl.h ++++ b/third_party/blink/renderer/core/exported/web_view_impl.h +@@ -636,6 +636,9 @@ class CORE_EXPORT WebViewImpl final : public WebView, + + scheduler::WebAgentGroupScheduler& GetWebAgentGroupScheduler(); + ++ // Returns true if the page supports app-region: drag/no-drag. ++ bool SupportsAppRegion(); ++ + private: + FRIEND_TEST_ALL_PREFIXES(WebFrameTest, DivScrollIntoEditableTest); + FRIEND_TEST_ALL_PREFIXES(WebFrameTest, +@@ -998,6 +1001,10 @@ class CORE_EXPORT WebViewImpl final : public WebView, + absl::optional close_called_stack_trace_; + absl::optional close_window_called_stack_trace_; + ++ // Indicates whether the page supports draggable regions via the app-region ++ // CSS property. ++ bool supports_app_region_ = false; ++ + // All the registered observers. + base::ObserverList observers_; + +diff --git a/third_party/blink/renderer/core/frame/local_frame.cc b/third_party/blink/renderer/core/frame/local_frame.cc +index 8cd79df2968ca7e98761b5aa604fb0228cbeaa8d..3a948217adf7ddb5a846c06cd4c0c5a8d64ab87b 100644 +--- a/third_party/blink/renderer/core/frame/local_frame.cc ++++ b/third_party/blink/renderer/core/frame/local_frame.cc +@@ -3829,19 +3829,4 @@ const mojom::RendererContentSettingsPtr& LocalFrame::GetContentSettings() { + return loader_.GetDocumentLoader()->GetContentSettings(); + } + +-bool LocalFrame::SupportsAppRegion() { +- return supports_app_region_; +-} +- +-void LocalFrame::SetSupportsAppRegion(bool supports_app_region) { +- supports_app_region_ = supports_app_region; +- if (supports_app_region) { +- view_->UpdateDocumentAnnotatedRegions(); +- } else { +- CHECK(GetDocument()); +- GetDocument()->SetAnnotatedRegions(Vector()); +- Client()->AnnotatedRegionsChanged(); +- } +-} +- + } // namespace blink +diff --git a/third_party/blink/renderer/core/frame/local_frame.h b/third_party/blink/renderer/core/frame/local_frame.h +index eb5ec8620c583e8850ea40deca9d19b08a144323..4acd8d0df124e1c2d5a546294e7fae261baf3440 100644 +--- a/third_party/blink/renderer/core/frame/local_frame.h ++++ b/third_party/blink/renderer/core/frame/local_frame.h +@@ -926,10 +926,6 @@ class CORE_EXPORT LocalFrame final + // Can only be called while the frame is not detached. + const mojom::RendererContentSettingsPtr& GetContentSettings(); + +- // Returns true if the frame supports app-region: drag/no-drag. +- bool SupportsAppRegion(); +- void SetSupportsAppRegion(bool supports_app_region); +- + private: + friend class FrameNavigationDisabler; + // LocalFrameMojoHandler is a part of LocalFrame. +@@ -1191,8 +1187,6 @@ class CORE_EXPORT LocalFrame final + + Member + v8_local_compile_hints_producer_; +- +- bool supports_app_region_ = false; + }; + + inline FrameLoader& LocalFrame::Loader() const { +diff --git a/third_party/blink/renderer/core/frame/local_frame_view.cc b/third_party/blink/renderer/core/frame/local_frame_view.cc +index 8cb1426970f17a84f1012ab7520373648f40ea35..b5c034a451c860c2fd4ca6488a98b6c863691c2f 100644 +--- a/third_party/blink/renderer/core/frame/local_frame_view.cc ++++ b/third_party/blink/renderer/core/frame/local_frame_view.cc +@@ -1730,7 +1730,8 @@ void LocalFrameView::NotifyPageThatContentAreaWillPaint() const { + + void LocalFrameView::UpdateDocumentAnnotatedRegions() const { + Document* document = frame_->GetDocument(); +- if (!document->HasAnnotatedRegions() || !frame_->SupportsAppRegion()) { ++ if (!document->HasAnnotatedRegions() || ++ !frame_->GetPage()->GetChromeClient().SupportsAppRegion()) { + return; + } + +diff --git a/third_party/blink/renderer/core/loader/empty_clients.h b/third_party/blink/renderer/core/loader/empty_clients.h +index 47dab797a32b8832e9380c89cad92546233d9351..82bf787bc884a60ad49aeabecf8b62d2f72cd619 100644 +--- a/third_party/blink/renderer/core/loader/empty_clients.h ++++ b/third_party/blink/renderer/core/loader/empty_clients.h +@@ -106,6 +106,7 @@ class CORE_EXPORT EmptyChromeClient : public ChromeClient { + void DidFocusPage() override {} + bool CanTakeFocus(mojom::blink::FocusType) override { return false; } + void TakeFocus(mojom::blink::FocusType) override {} ++ bool SupportsAppRegion() override { return false; } + void Show(LocalFrame& frame, + LocalFrame& opener_frame, + NavigationPolicy navigation_policy, +diff --git a/third_party/blink/renderer/core/page/chrome_client.h b/third_party/blink/renderer/core/page/chrome_client.h +index 73cddb74781652fddae126f497bf8f76a2cd179c..07cc5202bd2fc38f5463a1ec651dc6dd3cc4b419 100644 +--- a/third_party/blink/renderer/core/page/chrome_client.h ++++ b/third_party/blink/renderer/core/page/chrome_client.h +@@ -177,6 +177,10 @@ class CORE_EXPORT ChromeClient : public GarbageCollected { + + virtual void SetKeyboardFocusURL(Element*) {} + ++ // Returns true if the page should support drag regions via the app-region ++ // CSS property. ++ virtual bool SupportsAppRegion() = 0; ++ + // Allow document lifecycle updates to be run in order to produce composited + // outputs. Updates are blocked from occurring during loading navigation in + // order to prevent contention and allow Blink to proceed more quickly. This +diff --git a/third_party/blink/renderer/core/page/chrome_client_impl.cc b/third_party/blink/renderer/core/page/chrome_client_impl.cc +index 55d3e4248f722c56951b9c0120f3ac4480f1b5fe..b047b993fa780b19e08fb9a5bd328f7e83cc5661 100644 +--- a/third_party/blink/renderer/core/page/chrome_client_impl.cc ++++ b/third_party/blink/renderer/core/page/chrome_client_impl.cc +@@ -291,6 +291,10 @@ void ChromeClientImpl::SetKeyboardFocusURL(Element* new_focus_element) { + web_view_->SetKeyboardFocusURL(focus_url); + } + ++bool ChromeClientImpl::SupportsAppRegion() { ++ return web_view_->SupportsAppRegion(); ++} ++ + void ChromeClientImpl::StartDragging(LocalFrame* frame, + const WebDragData& drag_data, + DragOperationsMask mask, +diff --git a/third_party/blink/renderer/core/page/chrome_client_impl.h b/third_party/blink/renderer/core/page/chrome_client_impl.h +index bc8f7fe91eaff2ee9a0676e91ddff806bcaee5ff..ad179bc9ea6290e7bbeba427f54f0ea6ab396338 100644 +--- a/third_party/blink/renderer/core/page/chrome_client_impl.h ++++ b/third_party/blink/renderer/core/page/chrome_client_impl.h +@@ -77,6 +77,7 @@ class CORE_EXPORT ChromeClientImpl final : public ChromeClient { + bool CanTakeFocus(mojom::blink::FocusType) override; + void TakeFocus(mojom::blink::FocusType) override; + void SetKeyboardFocusURL(Element* new_focus_element) override; ++ bool SupportsAppRegion() override; + void BeginLifecycleUpdates(LocalFrame& main_frame) override; + void RegisterForCommitObservation(CommitObserver*) override; + void UnregisterFromCommitObservation(CommitObserver*) override; +diff --git a/third_party/blink/renderer/core/testing/internals.cc b/third_party/blink/renderer/core/testing/internals.cc +index 23cf906dbba8136c604a1e3a59d27882710368c6..9ff8b22cf794cd58bff985e054df37636e34aabd 100644 +--- a/third_party/blink/renderer/core/testing/internals.cc ++++ b/third_party/blink/renderer/core/testing/internals.cc +@@ -3008,7 +3008,8 @@ DOMRectList* Internals::nonDraggableRegions(Document* document, + } + + void Internals::SetSupportsAppRegion(bool supports_app_region) { +- GetFrame()->SetSupportsAppRegion(supports_app_region); ++ document_->GetPage()->GetChromeClient().GetWebView()->SetSupportsAppRegion( ++ supports_app_region); + } + + DOMRectList* Internals::AnnotatedRegions(Document* document, diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc index 5ca94d49b358..eff8ab3e4f13 100644 --- a/shell/renderer/electron_render_frame_observer.cc +++ b/shell/renderer/electron_render_frame_observer.cc @@ -30,6 +30,7 @@ #include "third_party/blink/public/web/web_element.h" #include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/public/web/web_script_source.h" +#include "third_party/blink/public/web/web_view.h" #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" // nogncheck #include "ui/base/resource/resource_bundle.h" @@ -55,6 +56,11 @@ ElectronRenderFrameObserver::ElectronRenderFrameObserver( renderer_client_(renderer_client) { // Initialise resource for directory listing. net::NetModule::SetResourceProvider(NetResourceProvider); + + // App regions are only supported in the main frame. + auto* main_frame = frame->GetMainRenderFrame(); + if (main_frame && main_frame == frame) + render_frame_->GetWebView()->SetSupportsAppRegion(true); } void ElectronRenderFrameObserver::DidClearWindowObject() {