Switching to IDispatch::Invoke when calling Omaha interfaces.

remoting/base/dispatch_win.h.pump

Index: remoting/base/dispatch_win.h.pump
diff --git a/remoting/base/dispatch_win.h.pump b/remoting/base/dispatch_win.h.pump
new file mode 100644
index 0000000000000000000000000000000000000000..f6753bb8f5bc77586d776e14a88fba0b4be71e4c
--- /dev/null
+++ b/remoting/base/dispatch_win.h.pump
@@ -0,0 +1,193 @@
+$$ This is a pump file for generating file templates.  Pump is a python
+$$ script that is part of the Google Test suite of utilities.  Description
+$$ can be found here:
+$$
+$$ http://code.google.com/p/googletest/wiki/PumpManual
+
+$$ MAX_ARITY controls the number of arguments that dispatch::Invoke() supports.
+$$ It is choosend to match the number of arguments base::Bind() supports.

[Wez, 2012/06/13 00:34:36]

choosend -> chosen

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

+$var MAX_ARITY = 7
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_BASE_IDISPATCH_DRIVER_WIN_H_
+#define REMOTING_BASE_IDISPATCH_DRIVER_WIN_H_
+
+#include <oaidl.h>
+
+#include "base/basictypes.h"
+#include "base/template_util.h"
+#include "base/win/scoped_variant.h"
+
+namespace remoting {
+
+namespace dispatch {
+
+namespace internal {
+
+// |is_param| is used to test whether the passed parameters are either constant
+// references to |VARIANT| structures (input parameters) or pointers to
+// |VARIANT| structures (output parameters).

[Wez, 2012/06/13 00:34:36]

Why do you need that?  If Marshal() were simply an overloaded method then would
compilation not fail for unsupported types simply because of the missing
overload?

[alexeypa (please no reviews), 2012/06/13 16:39:47]

It wasn't really needed. Overloaded methods work much better in this case.

+template <typename T> struct is_param : base::false_type {};
+template <> struct is_param<VARIANT> : base::true_type {};
+template <> struct is_param<base::win::ScopedVariant> : base::true_type {};
+template <> struct is_param<VARIANT*> : base::true_type {};
+
+// |ScopedVariantArg| is used to cleanup local copies of |VARIANT| parameters
+// in case of an error. It should be possible to cast a pointer to a vector of
+// |ScopedVariantArg| to a pointer to a vector of |VARIANTARG| strcutures.

[Wez, 2012/06/13 00:34:36]

strcutures -> structures

[Wez, 2012/06/13 00:34:36]

For this to make sense the reader needs two bits of context that may not be
obvious; that IDispatch provides an un-typed method-invokation interface for COM
objects, and that the parameters are passed using VARIANTs and may be in or out
parameters - hence our need to manage resources allocated to hold the
parameters, and the fact that this helper class has slightly odd-looking
marshal/unmarshal interfaces.

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

+class ScopedVariantArg : public VARIANTARG {

[Wez, 2012/06/13 00:34:36]

Would it make sense to derived from base::win::ScopedVariant here instead?

[alexeypa (please no reviews), 2012/06/13 16:39:47]

I don't think so. base::win::ScopedVariant seems to be prone to adding new data
members/virtual methods in the future.

[Wez, 2012/06/13 17:55:15]

Actually I'd missed that this derives from VARIANTARG while ScopedVariant
contains a VARIANT, so that won't work anyway!

[alexeypa (please no reviews), 2012/06/13 18:45:11]

VARIANT and VARIANTARG is the same thing. 

+ public:
+  ScopedVariantArg() {
+    vt = VT_EMPTY;
+  }
+
+  ~ScopedVariantArg() {
+    VariantClear(this);
+  }
+
+  // Marshall() routines copy an input parameter to a VARIANTARG structure so
+  // that it can be passed to IDispatch::Invoke.
+  template <typename T>
+  HRESULT Marshall(const T& param) {

[Wez, 2012/06/13 00:34:36]

Marshall -> Marshal

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

+    CHECK(false);
+    return E_FAIL;
+  }
+
+  template <>
+  HRESULT Marshall<VARIANT>(const VARIANT& param) {
+    return VariantCopy(this, &param);
+  }
+
+  template <>
+  HRESULT Marshall<base::win::ScopedVariant>(
+      const base::win::ScopedVariant& param) {

[Wez, 2012/06/13 00:34:36]

Do you need this? ScopedVariant has an implicit conversion to const VARIANT*
defined.

[alexeypa (please no reviews), 2012/06/13 16:39:47]

The implicit conversion works if it is an overloaded method (it does not work
for template specialization). Done.

+    return VariantCopy(this, &param);
+  }
+
+  template <>
+  HRESULT Marshall<VARIANT*>(VARIANT* const & param) {

[Wez, 2012/06/13 00:34:36]

Add a comment to clarify that this is the out-parameter specialization, so we
don't copy the |param| value into the argument in this case.

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

+    return S_OK;
+  }
+
+  // Unmarshall() routines moves an output parameter from a VARIANTARG structure
+  // to the location specified by the caller.
+  template <typename T>
+  void Unmarshall(const T& param_out) {

[Wez, 2012/06/13 00:34:36]

Unmarshall -> Unmarshal

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Done.

+    CHECK(false);
+  }
+
+  template <>
+  void Unmarshall<VARIANT>(const VARIANT& param_out) {
+  }
+
+  template <>
+  void Unmarshall<base::win::ScopedVariant>(
+      const base::win::ScopedVariant& param_out) {
+  }
+
+  template <>
+  void Unmarshall<VARIANT*>(VARIANT* const & param_out) {
+    *param_out = *this;
+    vt = VT_EMPTY;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScopedVariantArg);
+};
+
+// Make sure the layout of |VARIANTARG| and |ScopedVariantArg| is identical.
+COMPILE_ASSERT(sizeof(ScopedVariantArg) == sizeof(VARIANTARG),
+               scoped_variant_arg_should_not_add_data_members);
+
+}  // namespace internal
+
+// Invoke() is a convenience wrapper for IDispatch::Invoke. It takes care of
+// calling the desired method by its ID and implements logic for passing
+// a variable number of in/out parameters to the called method.
+//
+// Current limitations:
+//   - in_out parameters are not supported.
+//   - more than $(MAX_ARITY) parameters are not supported.
+//   - the method ID cannot be cached and reused.
+//   - VARIANT is the only supported parameter type at the moment.
+$range ARITY 0..MAX_ARITY
+$for ARITY [[
+$range ARG 1..ARITY
+
+
+$if ARITY > 0 [[template <$for ARG , [[typename P$(ARG)]]>]]
+
+HRESULT Invoke(IDispatch* object,
+               LPOLESTR name,
+               WORD flags,
+$for ARG [[
+
+               const P$(ARG)& p$(ARG),
+]]
+
+               VARIANT* const & result_out) {
+$for ARG [[
+
+  COMPILE_ASSERT(internal::is_param<P$(ARG)>::value, parameters_must_be_variants);

[Wez, 2012/06/13 00:34:36]

nit: Line too long?

[alexeypa (please no reviews), 2012/06/13 16:39:47]

This line was removed.

+]]
+
+
+  // Retrieve the ID of the method to be called.
+  DISPID disp_id;
+  HRESULT hr = object->GetIDsOfNames(IID_NULL, &name, 1, LOCALE_USER_DEFAULT,
+                                     &disp_id);
+  if (FAILED(hr))
+    return hr;
+
+  // Request the return value if asked by the caller.
+  internal::ScopedVariantArg result;
+  VARIANT* disp_result = NULL;
+  if (result_out != NULL)
+    disp_result = &result;
+
+$if ARITY > 0 [[
+
+  // Marshal the parameters into an array of VARIANT structures.
+  internal::ScopedVariantArg disp_args[$(ARITY)];
+$for ARG [[
+
+  hr = disp_args[$(ARITY) - $(ARG)].Marshall(p$(ARG));
+  if (FAILED(hr))
+    return hr;
+]]
+]]
+
+
+  // Invoke the method.
+  DISPPARAMS disp_params = { $if ARITY > 0 [[disp_args]] $else [[NULL]], NULL, $(ARITY), 0 };

[Wez, 2012/06/13 00:34:36]

nit: Line too long?

[alexeypa (please no reviews), 2012/06/13 16:39:47]

Only the one in .pump. I changed it so that the line is short enough in both .h
and .pump.

+  hr = object->Invoke(disp_id, IID_NULL, LOCALE_USER_DEFAULT, flags,
+                      &disp_params, disp_result, NULL, NULL);
+  if (FAILED(hr))
+    return hr;
+
+$if ARITY > 0 [[
+
+  // Unmarshall the parameters.
+$for ARG [[
+
+  disp_args[$(ARITY) - $(ARG)].Unmarshall(p$(ARG));
+]]
+]]
+
+
+  // Unmarshall the return value.
+  if (result_out != NULL)
+    result.Unmarshall(result_out);
+
+  return S_OK;
+}
+
+]]
+
+} // namespace dispatch
+
+} // namespace remoting
+
+#endif // REMOTING_BASE_IDISPATCH_DRIVER_WIN_H_