media: Remove LogDXVAError() from mf_helpers.h

LogDXVAError() reports UMAs that are specific to one single file
media/gpu/windows/dxva_video_decode_accelerator_win.cc. So it should not
be in media/base/win/mf_helpers.h which is a common helper for all
Windows media code.

This CL moves LogDXVAError() and related macros to
dxva_video_decode_accelerator_win.cc to solve this issue. In the future,
we should consolidate on fewer macros.

Change-Id: Icea7f0f8c5710bf034fce3dc32c36ccf2cc8e782
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066057Reviewed-by: Ted Meyer <tmathmeyer@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742959}

media/base/win/mf_helpers.cc

@@ -4,18 +4,10 @@
 
 #include "media/base/win/mf_helpers.h"
 
-#include "base/metrics/histogram_functions.h"
-
 namespace media {
 
 namespace mf {
 
-void LogDXVAError(int line) {
-  LOG(ERROR) << "Error in dxva_video_decode_accelerator_win.cc on line "
-             << line;
-  base::UmaHistogramSparse("Media.DXVAVDA.ErrorLine", line);
-}
-
 Microsoft::WRL::ComPtr<IMFSample> CreateEmptySampleWithBuffer(
     uint32_t buffer_length,
     int align) {

media/base/win/mf_helpers.h

@@ -17,36 +17,23 @@ namespace media {
 
 namespace mf {
 
-#define RETURN_ON_FAILURE(result, log, ret) \
-  do {                                      \
-    if (!(result)) {                        \
-      DLOG(ERROR) << log;                   \
-      mf::LogDXVAError(__LINE__);           \
-      return ret;                           \
-    }                                       \
+#define RETURN_ON_FAILURE(success, log, ret) \
+  do {                                       \
+    if (!(success)) {                        \
+      DLOG(ERROR) << log;                    \
+      return ret;                            \
+    }                                        \
   } while (0)
 
-#define RETURN_ON_HR_FAILURE(result, log, ret) \
-  RETURN_ON_FAILURE(SUCCEEDED(result),         \
-                    log << ", HRESULT: 0x" << std::hex << result, ret);
-
-#define RETURN_AND_NOTIFY_ON_FAILURE(result, log, error_code, ret) \
-  do {                                                             \
-    if (!(result)) {                                               \
-      DVLOG(1) << log;                                             \
-      mf::LogDXVAError(__LINE__);                                  \
-      StopOnError(error_code);                                     \
-      return ret;                                                  \
-    }                                                              \
-  } while (0)
-
-#define RETURN_AND_NOTIFY_ON_HR_FAILURE(result, log, error_code, ret)        \
-  RETURN_AND_NOTIFY_ON_FAILURE(SUCCEEDED(result),                            \
-                               log << ", HRESULT: 0x" << std::hex << result, \
-                               error_code, ret);
-
-MF_INITIALIZER_EXPORT void LogDXVAError(int line);
+#define RETURN_ON_HR_FAILURE(hresult, log, ret) \
+  RETURN_ON_FAILURE(SUCCEEDED(hresult),         \
+                    log << ", HRESULT: 0x" << std::hex << hresult, ret);
 
+// Macros that contain return statements can make code harder to read. Only use
+// it when necessary, e.g. in places where we deal with a lot of Windows API
+// calls, for each of which we have to check the returned HRESULT.
+// See discussion thread at:
+// https://groups.google.com/a/chromium.org/d/msg/cxx/zw5Xmcs--S4/r7Fwb-TsCAAJ
 #define RETURN_IF_FAILED(expr)                                \
   do {                                                        \
     HRESULT hresult = (expr);                                 \

media/gpu/windows/dxva_video_decode_accelerator_win.cc

@@ -32,6 +32,7 @@
 #include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/single_thread_task_runner.h"
@@ -134,8 +135,47 @@ uint64_t GetCurrentQPC() {
 uint64_t g_last_process_output_time;
 HRESULT g_last_device_removed_reason;
 
+void LogDXVAError(int line) {
+  LOG(ERROR) << "Error in dxva_video_decode_accelerator_win.cc on line "
+             << line;
+  base::UmaHistogramSparse("Media.DXVAVDA.ErrorLine", line);
+}
+
 }  // namespace
 
+// TODO(xhwang): Remove this after we remove or rename same macros in
+// mf_helpers.h.
+#undef RETURN_ON_FAILURE
+#undef RETURN_ON_HR_FAILURE
+
+#define RETURN_ON_FAILURE(result, log, ret) \
+  do {                                      \
+    if (!(result)) {                        \
+      DLOG(ERROR) << log;                   \
+      LogDXVAError(__LINE__);               \
+      return ret;                           \
+    }                                       \
+  } while (0)
+
+#define RETURN_ON_HR_FAILURE(result, log, ret) \
+  RETURN_ON_FAILURE(SUCCEEDED(result),         \
+                    log << ", HRESULT: 0x" << std::hex << result, ret);
+
+#define RETURN_AND_NOTIFY_ON_FAILURE(result, log, error_code, ret) \
+  do {                                                             \
+    if (!(result)) {                                               \
+      DVLOG(1) << log;                                             \
+      LogDXVAError(__LINE__);                                      \
+      StopOnError(error_code);                                     \
+      return ret;                                                  \
+    }                                                              \
+  } while (0)
+
+#define RETURN_AND_NOTIFY_ON_HR_FAILURE(result, log, error_code, ret)        \
+  RETURN_AND_NOTIFY_ON_FAILURE(SUCCEEDED(result),                            \
+                               log << ", HRESULT: 0x" << std::hex << result, \
+                               error_code, ret);
+
 namespace media {
 
 static const VideoCodecProfile kSupportedProfiles[] = {