Move Viewport reset into DirectManipulationEventHandler

The crash report shows DirectManipulation may call
DirectManipulationEventHandler OnViewportStatusChanged
to READY state after DirectManipulationHelper Destroyed
then crash because we called Reset to invalidate DMHelper.

In this CL, we pass the viewport size to DMEventHandler
and use it to reset viewport at gesture end. Also
DMEventHandler does not need to store pointer of DMHelper.

Bug: 951293
Change-Id: I39a3402659b9ae117ab45288b68dd5d595e58457
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1562153
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649712}

content/browser/renderer_host/direct_manipulation_event_handler_win.cc

@@ -28,19 +28,15 @@ bool FloatEquals(float f1, float f2) {
 }  // namespace
 
 DirectManipulationEventHandler::DirectManipulationEventHandler(
-    DirectManipulationHelper* helper)
-    : helper_(helper) {}
-
-void DirectManipulationEventHandler::SetWindowEventTarget(
-    ui::WindowEventTarget* event_target) {
-  if (!event_target && LoggingEnabled()) {
-    DebugLogging("Event target is null.", S_OK);
-    if (event_target_)
-      DebugLogging("Previous event target is not null", S_OK);
-    else
-      DebugLogging("Previous event target is null", S_OK);
-  }
-  event_target_ = event_target;
+    ui::WindowEventTarget* event_target)
+    : event_target_(event_target) {}
+
+bool DirectManipulationEventHandler::SetViewportSizeInPixels(
+    const gfx::Size& viewport_size_in_pixels) {
+  if (viewport_size_in_pixels_ == viewport_size_in_pixels)
+    return false;
+  viewport_size_in_pixels_ = viewport_size_in_pixels;
+  return true;
 }
 
 void DirectManipulationEventHandler::SetDeviceScaleFactor(
@@ -181,9 +177,14 @@ HRESULT DirectManipulationEventHandler::OnViewportStatusChanged(
   // RUNNING -> READY sequence.
   if (!FloatEquals(1.0f, last_scale_) || last_x_offset_ != 0 ||
       last_y_offset_ != 0) {
-    HRESULT hr = helper_->Reset();
-    if (!SUCCEEDED(hr))
+    HRESULT hr = viewport->ZoomToRect(
+        static_cast<float>(0), static_cast<float>(0),
+        static_cast<float>(viewport_size_in_pixels_.width()),
+        static_cast<float>(viewport_size_in_pixels_.height()), FALSE);
+    if (!SUCCEEDED(hr)) {
+      DebugLogging("Viewport zoom to rect failed.", hr);
       return hr;
+    }
   }
 
   last_scale_ = 1.0f;

content/browser/renderer_host/direct_manipulation_event_handler_win.h

@@ -11,6 +11,7 @@
 #include <wrl.h>
 
 #include "base/macros.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace ui {
 
@@ -20,7 +21,6 @@ class WindowEventTarget;
 
 namespace content {
 
-class DirectManipulationHelper;
 class DirectManipulationUnitTest;
 
 // DirectManipulationEventHandler receives status update and gesture events from
@@ -35,11 +35,10 @@ class DirectManipulationEventHandler
               Microsoft::WRL::FtmBase,
               IDirectManipulationViewportEventHandler>> {
  public:
-  explicit DirectManipulationEventHandler(DirectManipulationHelper* helper);
+  explicit DirectManipulationEventHandler(ui::WindowEventTarget* event_target);
 
-  // WindowEventTarget updates for every DM_POINTERHITTEST in case window
-  // hierarchy changed.
-  void SetWindowEventTarget(ui::WindowEventTarget* event_target);
+  // Return true if viewport_size_in_pixels_ changed.
+  bool SetViewportSizeInPixels(const gfx::Size& viewport_size_in_pixels);
 
   void SetDeviceScaleFactor(float device_scale_factor);
 
@@ -65,7 +64,6 @@ class DirectManipulationEventHandler
   OnContentUpdated(_In_ IDirectManipulationViewport* viewport,
                    _In_ IDirectManipulationContent* content) override;
 
-  DirectManipulationHelper* helper_ = nullptr;
   ui::WindowEventTarget* event_target_ = nullptr;
   float device_scale_factor_ = 1.0f;
   float last_scale_ = 1.0f;
@@ -76,6 +74,8 @@ class DirectManipulationEventHandler
   // Current recognized gesture from Direct Manipulation.
   GestureState gesture_state_ = GestureState::kNone;
 
+  gfx::Size viewport_size_in_pixels_;
+
   DISALLOW_COPY_AND_ASSIGN(DirectManipulationEventHandler);
 };
 

content/browser/renderer_host/direct_manipulation_helper_win.cc

@@ -77,8 +77,7 @@ DirectManipulationHelper::CreateInstanceForTesting(
       base::WrapUnique(new DirectManipulationHelper(0, nullptr));
 
   instance->event_handler_ =
-      Microsoft::WRL::Make<DirectManipulationEventHandler>(instance.get());
-  instance->event_handler_->SetWindowEventTarget(event_target);
+      Microsoft::WRL::Make<DirectManipulationEventHandler>(event_target);
 
   instance->viewport_ = viewport;
 
@@ -157,8 +156,8 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
     return false;
   }
 
-  event_handler_ = Microsoft::WRL::Make<DirectManipulationEventHandler>(this);
-  event_handler_->SetWindowEventTarget(event_target);
+  event_handler_ =
+      Microsoft::WRL::Make<DirectManipulationEventHandler>(event_target);
 
   // We got Direct Manipulation transform from
   // IDirectManipulationViewportEventHandler.
@@ -170,8 +169,9 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
   }
 
   // Set default rect for viewport before activate.
-  viewport_size_in_pixels_ = {1000, 1000};
-  RECT rect = gfx::Rect(viewport_size_in_pixels_).ToRECT();
+  gfx::Size viewport_size_in_pixels = {1000, 1000};
+  event_handler_->SetViewportSizeInPixels(viewport_size_in_pixels);
+  RECT rect = gfx::Rect(viewport_size_in_pixels).ToRECT();
   hr = viewport_->SetViewportRect(&rect);
   if (!SUCCEEDED(hr)) {
     DebugLogging("Viewport set rect failed.", hr);
@@ -205,7 +205,7 @@ bool DirectManipulationHelper::Initialize(ui::WindowEventTarget* event_target) {
 
 void DirectManipulationHelper::SetSizeInPixels(
     const gfx::Size& size_in_pixels) {
-  if (viewport_size_in_pixels_ == size_in_pixels)
+  if (!event_handler_->SetViewportSizeInPixels(size_in_pixels))
     return;
 
   HRESULT hr = viewport_->Stop();
@@ -214,8 +214,7 @@ void DirectManipulationHelper::SetSizeInPixels(
     return;
   }
 
-  viewport_size_in_pixels_ = size_in_pixels;
-  RECT rect = gfx::Rect(viewport_size_in_pixels_).ToRECT();
+  RECT rect = gfx::Rect(size_in_pixels).ToRECT();
   hr = viewport_->SetViewportRect(&rect);
   if (!SUCCEEDED(hr))
     DebugLogging("Viewport set rect failed.", hr);
@@ -245,21 +244,6 @@ void DirectManipulationHelper::OnPointerHitTest(WPARAM w_param) {
   }
 }
 
-HRESULT DirectManipulationHelper::Reset() {
-  // By zooming the primary content to a rect that match the viewport rect, we
-  // reset the content's transform to identity.
-  HRESULT hr = viewport_->ZoomToRect(
-      static_cast<float>(0), static_cast<float>(0),
-      static_cast<float>(viewport_size_in_pixels_.width()),
-      static_cast<float>(viewport_size_in_pixels_.height()), FALSE);
-  if (!SUCCEEDED(hr)) {
-    DebugLogging("Viewport zoom to rect failed.", hr);
-    return hr;
-  }
-
-  return S_OK;
-}
-
 void DirectManipulationHelper::SetDeviceScaleFactorForTesting(float factor) {
   event_handler_->SetDeviceScaleFactor(factor);
 }

content/browser/renderer_host/direct_manipulation_helper_win.h

@@ -73,9 +73,6 @@ class CONTENT_EXPORT DirectManipulationHelper
   // Updates viewport size. Call it when window bounds updated.
   void SetSizeInPixels(const gfx::Size& size_in_pixels);
 
-  // Reset for gesture end.
-  HRESULT Reset();
-
   // Pass the pointer hit test to Direct Manipulation.
   void OnPointerHitTest(WPARAM w_param);
 
@@ -100,7 +97,6 @@ class CONTENT_EXPORT DirectManipulationHelper
   HWND window_;
   ui::Compositor* compositor_ = nullptr;
   DWORD view_port_handler_cookie_;
-  gfx::Size viewport_size_in_pixels_;
 
   DISALLOW_COPY_AND_ASSIGN(DirectManipulationHelper);
 };