exo: Factor IconSurface class out of DragDropOperation

This CL extracts the drag icon surface class from DragDropOperation.
It can be null and there is no need to create a SurfaceTreeHost for it
in such case. This is not possible currently, because DragDropOperation
inherits from SurfaceTreeHost. This CL extracts it into an internal
class and uses composition (instead of inheritance) to create the
IconSurface only when needed.

This patch does not cause behavioral changes.

Bug: 1102946
Change-Id: If7497cbe436ef8bdb5fb3a80a184c5ac136732db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311127
Commit-Queue: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825332}

components/exo/drag_drop_operation.cc

@@ -12,6 +12,7 @@
 #include "components/exo/data_source.h"
 #include "components/exo/seat.h"
 #include "components/exo/surface.h"
+#include "components/exo/surface_tree_host.h"
 #include "components/viz/common/frame_sinks/copy_output_request.h"
 #include "components/viz/common/frame_sinks/copy_output_result.h"
 #include "ui/aura/client/drag_drop_client.h"
@@ -23,6 +24,7 @@
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/point_f.h"
+#include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/transform_util.h"
 #include "url/gurl.h"
 
@@ -83,6 +85,63 @@ DndAction DragOperationToDndAction(int op) {
 
 }  // namespace
 
+// Internal representation of a drag icon surface. Used when a non-null surface
+// is passed in wl_data_device::start_drag requests.
+// TODO(crbug.com/1119385): Rework icon implementation to avoid frame copies.
+class DragDropOperation::IconSurface final : public SurfaceTreeHost,
+                                             public ScopedSurface {
+ public:
+  IconSurface(Surface* icon, DragDropOperation* operation)
+      : SurfaceTreeHost("ExoDragIcon"),
+        ScopedSurface(icon, operation),
+        operation_(operation) {
+    DCHECK(operation_);
+    DCHECK(!icon->HasSurfaceDelegate());
+
+    Surface* origin_surface = operation_->origin_->get();
+    origin_surface->window()->AddChild(host_window());
+    SetRootSurface(icon);
+  }
+
+  IconSurface(const IconSurface&) = delete;
+  IconSurface& operator=(const IconSurface&) = delete;
+  ~IconSurface() override = default;
+
+ private:
+  // SurfaceTreeHost:
+  void OnSurfaceCommit() override {
+    SurfaceTreeHost::OnSurfaceCommit();
+    RequestCaptureIcon();
+  }
+
+  void RequestCaptureIcon() {
+    SubmitCompositorFrame();
+
+    std::unique_ptr<viz::CopyOutputRequest> request =
+        std::make_unique<viz::CopyOutputRequest>(
+            viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
+            base::BindOnce(&IconSurface::OnCaptured,
+                           weak_ptr_factory_.GetWeakPtr()));
+    request->set_result_task_runner(base::SequencedTaskRunnerHandle::Get());
+
+    host_window()->layer()->RequestCopyOfOutput(std::move(request));
+  }
+
+  void OnCaptured(std::unique_ptr<viz::CopyOutputResult> icon_result) {
+    // An empty response means the request was deleted before it was completed.
+    // If this happens, and no operation has yet finished, restart the capture.
+    if (icon_result->IsEmpty()) {
+      RequestCaptureIcon();
+      return;
+    }
+
+    operation_->OnDragIconCaptured(icon_result->AsSkBitmap());
+  }
+
+  DragDropOperation* const operation_;
+  base::WeakPtrFactory<IconSurface> weak_ptr_factory_{this};
+};
+
 base::WeakPtr<DragDropOperation> DragDropOperation::Create(
     DataSource* source,
     Surface* origin,
@@ -99,8 +158,7 @@ DragDropOperation::DragDropOperation(DataSource* source,
                                      Surface* icon,
                                      const gfx::PointF& drag_start_point,
                                      ui::mojom::DragEventSource event_source)
-    : SurfaceTreeHost("ExoDragDropOperation"),
-      source_(std::make_unique<ScopedDataSource>(source, this)),
+    : source_(std::make_unique<ScopedDataSource>(source, this)),
       origin_(std::make_unique<ScopedSurface>(origin, this)),
       drag_start_point_(drag_start_point),
       os_exchange_data_(std::make_unique<ui::OSExchangeData>()),
@@ -130,7 +188,7 @@ DragDropOperation::DragDropOperation(DataSource* source,
 #endif
 
   if (icon)
-    icon_ = std::make_unique<ScopedSurface>(icon, this);
+    icon_ = std::make_unique<IconSurface>(icon, this);
 
   auto start_op_callback =
       base::BindOnce(&DragDropOperation::ScheduleStartDragDropOperation,
@@ -151,11 +209,6 @@ DragDropOperation::DragDropOperation(DataSource* source,
       base::BindOnce(&DragDropOperation::OnFilenamesRead,
                      weak_ptr_factory_.GetWeakPtr()),
       counter_);
-
-  if (icon) {
-    origin_->get()->window()->AddChild(host_window());
-    SetRootSurface(icon);
-  }
 }
 
 DragDropOperation::~DragDropOperation() {
@@ -219,48 +272,18 @@ void DragDropOperation::OnFilenamesRead(const std::string& mime_type,
   counter_.Run();
 }
 
-void DragDropOperation::OnSurfaceCommit() {
-  SurfaceTreeHost::OnSurfaceCommit();
-
-  if (icon_)
-    CaptureDragIcon();
-}
-
-void DragDropOperation::CaptureDragIcon() {
-  SubmitCompositorFrame();
-
-  std::unique_ptr<viz::CopyOutputRequest> request =
-      std::make_unique<viz::CopyOutputRequest>(
-          viz::CopyOutputRequest::ResultFormat::RGBA_BITMAP,
-          base::BindOnce(&DragDropOperation::OnDragIconCaptured,
-                         weak_ptr_factory_.GetWeakPtr()));
-  request->set_result_task_runner(base::SequencedTaskRunnerHandle::Get());
-
-  host_window()->layer()->RequestCopyOfOutput(std::move(request));
-}
-
-void DragDropOperation::OnDragIconCaptured(
-    std::unique_ptr<viz::CopyOutputResult> icon_result) {
-  gfx::ImageSkia icon_skia;
-
-  // An empty response means the request was deleted before it was completed. If
-  // this happens, and no operation has yet finished, restart the capture.
-  if (icon_result->IsEmpty()) {
-    CaptureDragIcon();
-    return;
-  }
+void DragDropOperation::OnDragIconCaptured(const SkBitmap& icon_bitmap) {
+  DCHECK(icon_);
 
   float scale_factor = origin_->get()->window()->layer()->device_scale_factor();
-  icon_skia = gfx::ImageSkia(
-      gfx::ImageSkiaRep(icon_result->AsSkBitmap(), scale_factor));
+  gfx::ImageSkia icon_skia(gfx::ImageSkiaRep(icon_bitmap, scale_factor));
+  gfx::Vector2d icon_offset = -icon_->get()->GetBufferOffset();
 
   if (os_exchange_data_) {
-    os_exchange_data_->provider().SetDragImage(
-        icon_skia, -icon_->get()->GetBufferOffset());
+    os_exchange_data_->provider().SetDragImage(icon_skia, icon_offset);
   } else {
 #if defined(OS_CHROMEOS)
-    drag_drop_controller_->SetDragImage(icon_skia,
-                                        -icon_->get()->GetBufferOffset());
+    drag_drop_controller_->SetDragImage(icon_skia, icon_offset);
 #endif
   }
 
@@ -361,7 +384,7 @@ void DragDropOperation::ResetExtendedDragSource() {
 #endif
 
 void DragDropOperation::OnSurfaceDestroying(Surface* surface) {
-  DCHECK(surface == origin_->get() || surface == icon_->get());
+  DCHECK(surface == origin_->get() || (icon_ && surface == icon_->get()));
   delete this;
 }
 

components/exo/drag_drop_operation.h

@@ -5,11 +5,13 @@
 #ifndef COMPONENTS_EXO_DRAG_DROP_OPERATION_H_
 #define COMPONENTS_EXO_DRAG_DROP_OPERATION_H_
 
+#include <memory>
+#include <string>
+
 #include "components/exo/data_device.h"
 #include "components/exo/data_offer_observer.h"
 #include "components/exo/data_source_observer.h"
 #include "components/exo/surface_observer.h"
-#include "components/exo/surface_tree_host.h"
 #include "components/exo/wm_helper.h"
 #include "ui/aura/client/drag_drop_client_observer.h"
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom-forward.h"
@@ -19,6 +21,8 @@
 #include "components/exo/extended_drag_source.h"
 #endif
 
+class SkBitmap;
+
 namespace ash {
 class DragDropController;
 }  // namespace ash
@@ -33,12 +37,10 @@ namespace ui {
 class OSExchangeData;
 }
 
-namespace viz {
-class CopyOutputResult;
-}
-
 namespace exo {
 class ScopedDataSource;
+class Surface;
+class ScopedSurface;
 
 // This class represents an ongoing drag-drop operation started by an exo
 // client. It manages its own lifetime. It will delete itself when the drag
@@ -46,7 +48,6 @@ class ScopedDataSource;
 // (e.g. the client deletes the data source used to start the drag operation),
 // or if another drag operation races with this one to start and wins.
 class DragDropOperation : public DataSourceObserver,
-                          public SurfaceTreeHost,
                           public SurfaceObserver,
 #if defined(OS_CHROMEOS)
                           public ExtendedDragSource::Observer,
@@ -67,9 +68,6 @@ class DragDropOperation : public DataSourceObserver,
   // DataSourceObserver:
   void OnDataSourceDestroying(DataSource* source) override;
 
-  // SurfaceDelegate:
-  void OnSurfaceCommit() override;
-
   // SurfaceObserver:
   void OnSurfaceDestroying(Surface* surface) override;
 
@@ -84,6 +82,8 @@ class DragDropOperation : public DataSourceObserver,
 #endif
 
  private:
+  class IconSurface;
+
   // A private constructor and destructor are used to prevent anyone else from
   // attempting to manage the lifetime of a DragDropOperation.
   DragDropOperation(DataSource* source,
@@ -93,8 +93,7 @@ class DragDropOperation : public DataSourceObserver,
                     ui::mojom::DragEventSource event_source);
   ~DragDropOperation() override;
 
-  void CaptureDragIcon();
-  void OnDragIconCaptured(std::unique_ptr<viz::CopyOutputResult> icon_result);
+  void OnDragIconCaptured(const SkBitmap& icon_bitmap);
 
   void OnTextRead(const std::string& mime_type, base::string16 data);
   void OnHTMLRead(const std::string& mime_type, base::string16 data);

components/exo/surface.h

@@ -475,7 +475,7 @@ class Surface final : public ui::PropertyHandler {
 class ScopedSurface {
  public:
   ScopedSurface(Surface* surface, SurfaceObserver* observer);
-  ~ScopedSurface();
+  virtual ~ScopedSurface();
   Surface* get() { return surface_; }
 
  private: