Reduce boilerplate in DirectManipulation

fix some comments

Author: moffatman

lib/web_ui/lib/src/engine/pointer_converter.dart

@@ -616,7 +616,7 @@ class PointerDataConverter {
         case ui.PointerChange.panZoomStart:
         case ui.PointerChange.panZoomUpdate:
         case ui.PointerChange.panZoomEnd:
-          // Pointer pan/zoom events are not generated on web
+          // Pointer pan/zoom events are not generated on web.
           assert(false);
           break;
       }

shell/platform/windows/direct_manipulation.cc

@@ -8,6 +8,20 @@
 #include "flutter/shell/platform/windows/window_binding_handler_delegate.h"
 #include "flutter/shell/platform/windows/window_win32.h"
 
+#define VERIFY_HR(operation)                   \
+  if (FAILED(operation)) {                     \
+    FML_LOG(ERROR) << #operation << " failed"; \
+    manager_ = nullptr;                        \
+    updateManager_ = nullptr;                  \
+    viewport_ = nullptr;                       \
+    return -1;                                 \
+  }
+
+#define WARN_HR(operation)                     \
+  if (FAILED(operation)) {                     \
+    FML_LOG(ERROR) << #operation << " failed"; \
+  }
+
 namespace flutter {
 
 STDMETHODIMP DirectManipulationEventHandler::QueryInterface(REFIID iid,
@@ -31,20 +45,23 @@ HRESULT DirectManipulationEventHandler::OnViewportStatusChanged(
     DIRECTMANIPULATION_STATUS previous) {
   if (current == DIRECTMANIPULATION_RUNNING) {
     if (!resetting_) {
+      // Not a false event
       if (owner_->binding_handler_delegate) {
         owner_->binding_handler_delegate->OnPointerPanZoomStart(
             (int32_t) reinterpret_cast<int64_t>(this));
       }
     }
   } else if (previous == DIRECTMANIPULATION_RUNNING) {
     if (resetting_) {
+      // The resetting transition has concluded
       resetting_ = false;
     } else {
       if (owner_->binding_handler_delegate) {
         owner_->binding_handler_delegate->OnPointerPanZoomEnd(
             (int32_t) reinterpret_cast<int64_t>(this));
       }
-      // Need to reset the content transform
+      // Need to reset the content transform to its original position
+      // so that we are ready for the next gesture
       // Use resetting_ flag to prevent sending reset also to the framework
       resetting_ = true;
       RECT rect;
@@ -117,120 +134,38 @@ DirectManipulationOwner::DirectManipulationOwner(WindowWin32* window)
     : window_(window) {}
 
 int DirectManipulationOwner::Init(unsigned int width, unsigned int height) {
-  HRESULT hr = CoCreateInstance(CLSID_DirectManipulationManager, nullptr,
-                                CLSCTX_INPROC_SERVER,
-                                IID_IDirectManipulationManager, &manager_);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR)
-        << "CoCreateInstance(CLSID_DirectManipulationManager) failed";
-    manager_ = nullptr;
-    return -1;
-  }
-
-  hr = manager_->GetUpdateManager(IID_IDirectManipulationUpdateManager,
-                                  &updateManager_);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "GetUpdateManager failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    return -1;
-  }
-
-  hr = manager_->CreateViewport(nullptr, window_->GetWindowHandle(),
-                                IID_IDirectManipulationViewport, &viewport_);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "CreateViewport failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
+  VERIFY_HR(CoCreateInstance(CLSID_DirectManipulationManager, nullptr,
+                             CLSCTX_INPROC_SERVER,
+                             IID_IDirectManipulationManager, &manager_));
+  VERIFY_HR(manager_->GetUpdateManager(IID_IDirectManipulationUpdateManager,
+                                       &updateManager_));
+  VERIFY_HR(manager_->CreateViewport(nullptr, window_->GetWindowHandle(),
+                                     IID_IDirectManipulationViewport,
+                                     &viewport_));
   DIRECTMANIPULATION_CONFIGURATION configuration =
       DIRECTMANIPULATION_CONFIGURATION_INTERACTION |
       DIRECTMANIPULATION_CONFIGURATION_TRANSLATION_X |
       DIRECTMANIPULATION_CONFIGURATION_TRANSLATION_Y |
       DIRECTMANIPULATION_CONFIGURATION_SCALING;
-
-  hr = viewport_->ActivateConfiguration(configuration);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "ActivateConfiguration failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
-  hr = viewport_->SetViewportOptions(
-      DIRECTMANIPULATION_VIEWPORT_OPTIONS_MANUALUPDATE);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "SetViewportOptions failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
+  VERIFY_HR(viewport_->ActivateConfiguration(configuration));
+  VERIFY_HR(viewport_->SetViewportOptions(
+      DIRECTMANIPULATION_VIEWPORT_OPTIONS_MANUALUPDATE));
   handler_ = fml::MakeRefCounted<DirectManipulationEventHandler>(window_, this);
-
-  hr = viewport_->AddEventHandler(window_->GetWindowHandle(), handler_.get(),
-                                  &viewportHandlerCookie_);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "AddEventHandler failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
+  VERIFY_HR(viewport_->AddEventHandler(
+      window_->GetWindowHandle(), handler_.get(), &viewportHandlerCookie_));
   RECT rect = {0, 0, (LONG)width, (LONG)height};
-  hr = viewport_->SetViewportRect(&rect);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "SetViewportRect failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
-  hr = manager_->Activate(window_->GetWindowHandle());
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "manager_->Activate failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
-  hr = viewport_->Enable();
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "viewport_->Enable failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
-  hr = updateManager_->Update(nullptr);
-  if (FAILED(hr)) {
-    FML_LOG(ERROR) << "updateManager_->Update failed";
-    manager_ = nullptr;
-    updateManager_ = nullptr;
-    viewport_ = nullptr;
-    return -1;
-  }
-
+  VERIFY_HR(viewport_->SetViewportRect(&rect));
+  VERIFY_HR(manager_->Activate(window_->GetWindowHandle()));
+  VERIFY_HR(viewport_->Enable());
+  VERIFY_HR(updateManager_->Update(nullptr));
   return 0;
 }
 
 void DirectManipulationOwner::ResizeViewport(unsigned int width,
                                              unsigned int height) {
   if (viewport_) {
     RECT rect = {0, 0, (LONG)width, (LONG)height};
-    HRESULT hr = viewport_->SetViewportRect(&rect);
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "SetViewportRect failed";
-    }
+    WARN_HR(viewport_->SetViewportRect(&rect));
   }
 }
 
@@ -240,34 +175,15 @@ void DirectManipulationOwner::Destroy() {
     handler_->owner_ = nullptr;
   }
 
-  HRESULT hr;
   if (viewport_) {
-    hr = viewport_->Disable();
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "viewport_->Stop failed";
-    }
-
-    hr = viewport_->Disable();
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "viewport_->Disable failed";
-    }
-
-    hr = viewport_->RemoveEventHandler(viewportHandlerCookie_);
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "viewport_->RemoveEventHandler failed";
-    }
-
-    hr = viewport_->Abandon();
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "viewport_->Abandon failed";
-    }
+    WARN_HR(viewport_->Disable());
+    WARN_HR(viewport_->Disable());
+    WARN_HR(viewport_->RemoveEventHandler(viewportHandlerCookie_));
+    WARN_HR(viewport_->Abandon());
   }
 
   if (window_ && manager_) {
-    hr = manager_->Deactivate(window_->GetWindowHandle());
-    if (FAILED(hr)) {
-      FML_LOG(ERROR) << "manager_->Deactivate failed";
-    }
+    WARN_HR(manager_->Deactivate(window_->GetWindowHandle()));
   }
 
   handler_ = nullptr;

shell/platform/windows/direct_manipulation.h

@@ -75,6 +75,9 @@ class DirectManipulationEventHandler
  private:
   WindowWin32* window_;
   DirectManipulationOwner* owner_;
+  // We need to reset some parts of DirectManipulation after each gesture
+  // A flag is needed to ensure that false events created as the reset occurs
+  // are not sent to the flutter framework.
   bool resetting_ = false;
 };