fix: draggable regions exclusively on BrowserViews (#26145)

filenames.gni

@@ -368,6 +368,7 @@ filenames = {
     "shell/browser/electron_web_ui_controller_factory.h",
     "shell/browser/event_emitter_mixin.cc",
     "shell/browser/event_emitter_mixin.h",
+    "shell/browser/extended_web_contents_observer.h",
     "shell/browser/feature_list.cc",
     "shell/browser/feature_list.h",
     "shell/browser/font_defaults.cc",
@@ -452,6 +453,7 @@ filenames = {
     "shell/browser/ui/devtools_manager_delegate.h",
     "shell/browser/ui/devtools_ui.cc",
     "shell/browser/ui/devtools_ui.h",
+    "shell/browser/ui/drag_util.cc",
     "shell/browser/ui/drag_util.h",
     "shell/browser/ui/electron_menu_model.cc",
     "shell/browser/ui/electron_menu_model.h",

shell/browser/api/electron_api_browser_view.cc

@@ -4,9 +4,12 @@
 
 #include "shell/browser/api/electron_api_browser_view.h"
 
+#include <vector>
+
 #include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/browser.h"
 #include "shell/browser/native_browser_view.h"
+#include "shell/browser/ui/drag_util.h"
 #include "shell/common/color_util.h"
 #include "shell/common/gin_converters/gfx_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
@@ -80,6 +83,7 @@ BrowserView::BrowserView(gin::Arguments* args,
 
   web_contents_.Reset(isolate, web_contents.ToV8());
   api_web_contents_ = web_contents.get();
+  api_web_contents_->AddObserver(this);
   Observe(web_contents->web_contents());
 
   view_.reset(
@@ -90,6 +94,7 @@ BrowserView::~BrowserView() {
   if (api_web_contents_) {  // destroy() is called
     // Destroy WebContents asynchronously unless app is shutting down,
     // because destroy() might be called inside WebContents's event handler.
+    api_web_contents_->RemoveObserver(this);
     api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down());
   }
 }
@@ -100,6 +105,11 @@ void BrowserView::WebContentsDestroyed() {
   Unpin();
 }
 
+void BrowserView::OnDraggableRegionsUpdated(
+    const std::vector<mojom::DraggableRegionPtr>& regions) {
+  view_->UpdateDraggableRegions(regions);
+}
+
 // static
 gin::Handle<BrowserView> BrowserView::New(gin_helper::ErrorThrower thrower,
                                           gin::Arguments* args) {

shell/browser/api/electron_api_browser_view.h

@@ -7,11 +7,14 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "content/public/browser/web_contents_observer.h"
 #include "gin/handle.h"
 #include "gin/wrappable.h"
+#include "shell/browser/extended_web_contents_observer.h"
 #include "shell/browser/native_browser_view.h"
+#include "shell/common/api/api.mojom.h"
 #include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/pinnable.h"
@@ -35,7 +38,8 @@ class WebContents;
 class BrowserView : public gin::Wrappable<BrowserView>,
                     public gin_helper::Constructible<BrowserView>,
                     public gin_helper::Pinnable<BrowserView>,
-                    public content::WebContentsObserver {
+                    public content::WebContentsObserver,
+                    public ExtendedWebContentsObserver {
  public:
   // gin_helper::Constructible
   static gin::Handle<BrowserView> New(gin_helper::ErrorThrower thrower,
@@ -59,6 +63,10 @@ class BrowserView : public gin::Wrappable<BrowserView>,
   // content::WebContentsObserver:
   void WebContentsDestroyed() override;
 
+  // ExtendedWebContentsObserver:
+  void OnDraggableRegionsUpdated(
+      const std::vector<mojom::DraggableRegionPtr>& regions) override;
+
  private:
   void SetAutoResize(AutoResizeFlags flags);
   void SetBounds(const gfx::Rect& bounds);

shell/browser/api/electron_api_browser_window.cc

@@ -414,19 +414,6 @@ v8::Local<v8::Value> BrowserWindow::GetWebContents(v8::Isolate* isolate) {
   return v8::Local<v8::Value>::New(isolate, web_contents_);
 }
 
-// Convert draggable regions in raw format to SkRegion format.
-std::unique_ptr<SkRegion> BrowserWindow::DraggableRegionsToSkRegion(
-    const std::vector<mojom::DraggableRegionPtr>& regions) {
-  auto sk_region = std::make_unique<SkRegion>();
-  for (const auto& region : regions) {
-    sk_region->op(
-        {region->bounds.x(), region->bounds.y(), region->bounds.right(),
-         region->bounds.bottom()},
-        region->draggable ? SkRegion::kUnion_Op : SkRegion::kDifference_Op);
-  }
-  return sk_region;
-}
-
 void BrowserWindow::ScheduleUnresponsiveEvent(int ms) {
   if (!window_unresponsive_closure_.IsCancelled())
     return;

shell/browser/api/electron_api_browser_window.h

@@ -12,6 +12,7 @@
 #include "base/cancelable_callback.h"
 #include "shell/browser/api/electron_api_base_window.h"
 #include "shell/browser/api/electron_api_web_contents.h"
+#include "shell/browser/ui/drag_util.h"
 #include "shell/common/gin_helper/error_thrower.h"
 
 namespace electron {
@@ -102,10 +103,6 @@ class BrowserWindow : public BaseWindow,
   void UpdateDraggableRegions(
       const std::vector<mojom::DraggableRegionPtr>& regions);
 
-  // Convert draggable regions in raw format to SkRegion format.
-  std::unique_ptr<SkRegion> DraggableRegionsToSkRegion(
-      const std::vector<mojom::DraggableRegionPtr>& regions);
-
   // Schedule a notification unresponsive event.
   void ScheduleUnresponsiveEvent(int ms);
 

shell/browser/api/electron_api_browser_window_mac.mm

@@ -37,26 +37,6 @@ namespace electron {
 
 namespace api {
 
-namespace {
-
-// Return a vector of non-draggable regions that fill a window of size
-// |width| by |height|, but leave gaps where the window should be draggable.
-std::vector<gfx::Rect> CalculateNonDraggableRegions(
-    std::unique_ptr<SkRegion> draggable,
-    int width,
-    int height) {
-  std::vector<gfx::Rect> result;
-  SkRegion non_draggable;
-  non_draggable.op({0, 0, width, height}, SkRegion::kUnion_Op);
-  non_draggable.op(*draggable, SkRegion::kDifference_Op);
-  for (SkRegion::Iterator it(non_draggable); !it.done(); it.next()) {
-    result.push_back(gfx::SkIRectToRect(it.rect()));
-  }
-  return result;
-}
-
-}  // namespace
-
 void BrowserWindow::OverrideNSWindowContentView(InspectableWebContents* iwc) {
   // Make NativeWindow use a NSView as content view.
   static_cast<NativeWindowMac*>(window())->OverrideNSWindowContentView();
@@ -109,6 +89,12 @@ void BrowserWindow::UpdateDraggableRegions(
     for (const auto& r : regions)
       draggable_regions_.push_back(r.Clone());
   }
+
+  auto browser_views = window_->browser_views();
+  for (NativeBrowserView* view : browser_views) {
+    view->UpdateDraggableRegions(draggable_regions_);
+  }
+
   std::vector<gfx::Rect> drag_exclude_rects;
   if (regions.empty()) {
     drag_exclude_rects.push_back(gfx::Rect(0, 0, webViewWidth, webViewHeight));
@@ -117,11 +103,6 @@ void BrowserWindow::UpdateDraggableRegions(
         DraggableRegionsToSkRegion(regions), webViewWidth, webViewHeight);
   }
 
-  auto browser_views = window_->browser_views();
-  for (NativeBrowserView* view : browser_views) {
-    view->UpdateDraggableRegions(drag_exclude_rects);
-  }
-
   // Create and add a ControlRegionView for each region that needs to be
   // excluded from the dragging.
   for (const auto& rect : drag_exclude_rects) {

shell/browser/api/electron_api_browser_window_views.cc

@@ -15,7 +15,7 @@ void BrowserWindow::UpdateDraggableRegions(
   if (window_->has_frame())
     return;
   static_cast<NativeWindowViews*>(window_.get())
-      ->UpdateDraggableRegions(DraggableRegionsToSkRegion(regions));
+      ->UpdateDraggableRegions(regions);
 }
 
 }  // namespace api

shell/browser/api/electron_api_web_contents.h

@@ -31,6 +31,7 @@
 #include "shell/browser/api/save_page_handler.h"
 #include "shell/browser/common_web_contents_delegate.h"
 #include "shell/browser/event_emitter_mixin.h"
+#include "shell/browser/extended_web_contents_observer.h"
 #include "shell/common/gin_helper/cleaned_up_at_exit.h"
 #include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/error_thrower.h"
@@ -120,22 +121,6 @@ class OffScreenRenderWidgetHostView;
 
 namespace api {
 
-// Certain events are only in WebContentsDelegate, provide our own Observer to
-// dispatch those events.
-class ExtendedWebContentsObserver : public base::CheckedObserver {
- public:
-  virtual void OnCloseContents() {}
-  virtual void OnDraggableRegionsUpdated(
-      const std::vector<mojom::DraggableRegionPtr>& regions) {}
-  virtual void OnSetContentBounds(const gfx::Rect& rect) {}
-  virtual void OnActivateContents() {}
-  virtual void OnPageTitleUpdated(const base::string16& title,
-                                  bool explicit_set) {}
-
- protected:
-  ~ExtendedWebContentsObserver() override {}
-};
-
 // Wrapper around the content::WebContents.
 class WebContents : public gin::Wrappable<WebContents>,
                     public gin_helper::EventEmitterMixin<WebContents>,

shell/browser/extended_web_contents_observer.h

@@ -0,0 +1,35 @@
+// Copyright (c) 2020 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_BROWSER_EXTENDED_WEB_CONTENTS_OBSERVER_H_
+#define SHELL_BROWSER_EXTENDED_WEB_CONTENTS_OBSERVER_H_
+
+#include <vector>
+
+#include "base/observer_list.h"
+#include "base/strings/string16.h"
+#include "electron/shell/common/api/api.mojom.h"
+#include "ui/gfx/geometry/rect.h"
+
+namespace electron {
+
+// Certain events are only in WebContentsDelegate, so we provide our own
+// Observer to dispatch those events.
+class ExtendedWebContentsObserver : public base::CheckedObserver {
+ public:
+  virtual void OnCloseContents() {}
+  virtual void OnDraggableRegionsUpdated(
+      const std::vector<mojom::DraggableRegionPtr>& regions) {}
+  virtual void OnSetContentBounds(const gfx::Rect& rect) {}
+  virtual void OnActivateContents() {}
+  virtual void OnPageTitleUpdated(const base::string16& title,
+                                  bool explicit_set) {}
+
+ protected:
+  ~ExtendedWebContentsObserver() override {}
+};
+
+}  // namespace electron
+
+#endif  // SHELL_BROWSER_EXTENDED_WEB_CONTENTS_OBSERVER_H_

shell/browser/native_browser_view.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "shell/common/api/api.mojom.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 namespace gfx {
@@ -48,7 +49,7 @@ class NativeBrowserView : public content::WebContentsObserver {
 
   // Called when the window needs to update its draggable region.
   virtual void UpdateDraggableRegions(
-      const std::vector<gfx::Rect>& system_drag_exclude_areas) {}
+      const std::vector<mojom::DraggableRegionPtr>& regions) {}
 
  protected:
   explicit NativeBrowserView(InspectableWebContents* inspectable_web_contents);

shell/browser/native_browser_view_mac.h

@@ -25,7 +25,7 @@ class NativeBrowserViewMac : public NativeBrowserView {
   void SetBackgroundColor(SkColor color) override;
 
   void UpdateDraggableRegions(
-      const std::vector<gfx::Rect>& system_drag_exclude_areas) override;
+      const std::vector<mojom::DraggableRegionPtr>& regions) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NativeBrowserViewMac);

shell/browser/native_browser_view_mac.mm

@@ -6,6 +6,7 @@
 
 #include <vector>
 
+#include "shell/browser/ui/drag_util.h"
 #include "shell/browser/ui/inspectable_web_contents.h"
 #include "shell/browser/ui/inspectable_web_contents_view.h"
 #include "skia/ext/skia_utils_mac.h"
@@ -35,8 +36,8 @@ const NSAutoresizingMaskOptions kDefaultAutoResizingMask =
 
 - (NSView*)hitTest:(NSPoint)aPoint {
   // Pass-through events that don't hit one of the exclusion zones
-  for (NSView* exlusion_zones in [self subviews]) {
-    if ([exlusion_zones hitTest:aPoint])
+  for (NSView* exclusion_zones in [self subviews]) {
+    if ([exclusion_zones hitTest:aPoint])
       return nil;
   }
 
@@ -230,7 +231,7 @@ void NativeBrowserViewMac::SetBackgroundColor(SkColor color) {
 }
 
 void NativeBrowserViewMac::UpdateDraggableRegions(
-    const std::vector<gfx::Rect>& drag_exclude_rects) {
+    const std::vector<mojom::DraggableRegionPtr>& regions) {
   if (!inspectable_web_contents_)
     return;
   auto* web_contents = inspectable_web_contents_->GetWebContents();
@@ -240,6 +241,11 @@ void NativeBrowserViewMac::UpdateDraggableRegions(
   NSView* window_content_view = inspectable_view.superview;
   const auto window_content_view_height = NSHeight(window_content_view.bounds);
 
+  NSInteger webViewWidth = NSWidth([web_view bounds]);
+  NSInteger webViewHeight = NSHeight([web_view bounds]);
+  auto drag_exclude_rects = CalculateNonDraggableRegions(
+      DraggableRegionsToSkRegion(regions), webViewWidth, webViewHeight);
+
   // Remove all DragRegionViews that were added last time. Note that we need
   // to copy the `subviews` array to avoid mutation during iteration.
   base::scoped_nsobject<NSArray> subviews([[web_view subviews] copy]);

shell/browser/native_window_views.cc

@@ -20,6 +20,7 @@
 #include "content/public/browser/desktop_media_id.h"
 #include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/native_browser_view_views.h"
+#include "shell/browser/ui/drag_util.h"
 #include "shell/browser/ui/inspectable_web_contents.h"
 #include "shell/browser/ui/inspectable_web_contents_view.h"
 #include "shell/browser/ui/views/root_view.h"
@@ -1329,8 +1330,8 @@ gfx::Rect NativeWindowViews::WindowBoundsToContentBounds(
 }
 
 void NativeWindowViews::UpdateDraggableRegions(
-    std::unique_ptr<SkRegion> region) {
-  draggable_region_ = std::move(region);
+    const std::vector<mojom::DraggableRegionPtr>& regions) {
+  draggable_region_ = DraggableRegionsToSkRegion(regions);
 }
 
 #if defined(OS_WIN)

shell/browser/native_window_views.h

@@ -11,7 +11,9 @@
 #include <set>
 #include <string>
 #include <tuple>
+#include <vector>
 
+#include "shell/common/api/api.mojom.h"
 #include "ui/views/widget/widget_observer.h"
 
 #if defined(OS_WIN)
@@ -138,7 +140,8 @@ class NativeWindowViews : public NativeWindow,
   gfx::Rect ContentBoundsToWindowBounds(const gfx::Rect& bounds) const override;
   gfx::Rect WindowBoundsToContentBounds(const gfx::Rect& bounds) const override;
 
-  void UpdateDraggableRegions(std::unique_ptr<SkRegion> region);
+  void UpdateDraggableRegions(
+      const std::vector<mojom::DraggableRegionPtr>& regions);
 
   void IncrementChildModals();
   void DecrementChildModals();

shell/browser/ui/drag_util.cc

@@ -0,0 +1,42 @@
+// Copyright (c) 2020 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/browser/ui/drag_util.h"
+
+#include <memory>
+
+#include "ui/gfx/skia_util.h"
+
+namespace electron {
+
+// Return a vector of non-draggable regions that fill a window of size
+// |width| by |height|, but leave gaps where the window should be draggable.
+std::vector<gfx::Rect> CalculateNonDraggableRegions(
+    std::unique_ptr<SkRegion> draggable,
+    int width,
+    int height) {
+  std::vector<gfx::Rect> result;
+  SkRegion non_draggable;
+  non_draggable.op({0, 0, width, height}, SkRegion::kUnion_Op);
+  non_draggable.op(*draggable, SkRegion::kDifference_Op);
+  for (SkRegion::Iterator it(non_draggable); !it.done(); it.next()) {
+    result.push_back(gfx::SkIRectToRect(it.rect()));
+  }
+  return result;
+}
+
+// Convert draggable regions in raw format to SkRegion format.
+std::unique_ptr<SkRegion> DraggableRegionsToSkRegion(
+    const std::vector<mojom::DraggableRegionPtr>& regions) {
+  auto sk_region = std::make_unique<SkRegion>();
+  for (const auto& region : regions) {
+    sk_region->op(
+        {region->bounds.x(), region->bounds.y(), region->bounds.right(),
+         region->bounds.bottom()},
+        region->draggable ? SkRegion::kUnion_Op : SkRegion::kDifference_Op);
+  }
+  return sk_region;
+}
+
+}  // namespace electron

shell/browser/ui/drag_util.h

@@ -5,8 +5,12 @@
 #ifndef SHELL_BROWSER_UI_DRAG_UTIL_H_
 #define SHELL_BROWSER_UI_DRAG_UTIL_H_
 
+#include <memory>
 #include <vector>
 
+#include "shell/common/api/api.mojom.h"
+#include "third_party/skia/include/core/SkRegion.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image.h"
 
 namespace base {
@@ -19,6 +23,15 @@ void DragFileItems(const std::vector<base::FilePath>& files,
                    const gfx::Image& icon,
                    gfx::NativeView view);
 
+std::vector<gfx::Rect> CalculateNonDraggableRegions(
+    std::unique_ptr<SkRegion> draggable,
+    int width,
+    int height);
+
+// Convert draggable regions in raw format to SkRegion format.
+std::unique_ptr<SkRegion> DraggableRegionsToSkRegion(
+    const std::vector<mojom::DraggableRegionPtr>& regions);
+
 }  // namespace electron
 
 #endif  // SHELL_BROWSER_UI_DRAG_UTIL_H_