[TaskScheduler]: Save last error in ScopedBlockingCall

Tls affects result of GetLastError on windows. To avoid interaction with
base::File, last error is saved and restored in the destructor of
ScopedBlockingCall.

Bug: 874080
Change-Id: Id17ac4e7ee4ffaf33eda30784b86ad41e21b8b96
Reviewed-on: https://chromium-review.googlesource.com/1220172
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591525}

base/BUILD.gn

@@ -647,7 +647,8 @@ jumbo_component("base") {
     "sampling_heap_profiler/poisson_allocation_sampler.h",
     "sampling_heap_profiler/sampling_heap_profiler.cc",
     "sampling_heap_profiler/sampling_heap_profiler.h",
-    "scoped_clear_errno.h",
+    "scoped_clear_last_error.h",
+    "scoped_clear_last_error_win.cc",
     "scoped_generic.h",
     "scoped_native_library.cc",
     "scoped_native_library.h",
@@ -2394,7 +2395,7 @@ test("base_unittests") {
     "safe_numerics_unittest.cc",
     "sampling_heap_profiler/lock_free_address_hash_set_unittest.cc",
     "sampling_heap_profiler/module_cache_unittest.cc",
-    "scoped_clear_errno_unittest.cc",
+    "scoped_clear_last_error_unittest.cc",
     "scoped_generic_unittest.cc",
     "scoped_native_library_unittest.cc",
     "security_unittest.cc",

base/allocator/allocator_interception_mac.mm

@@ -34,7 +34,6 @@
 #include "base/mac/mac_util.h"
 #include "base/mac/mach_logging.h"
 #include "base/process/memory.h"
-#include "base/scoped_clear_errno.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 #include "third_party/apple_apsl/CFBase.h"

base/logging.cc

@@ -555,15 +555,6 @@ void DisplayDebugMessageInDialog(const std::string& str) {
 }
 #endif  // !defined(NDEBUG)
 
-#if defined(OS_WIN)
-LogMessage::SaveLastError::SaveLastError() : last_error_(::GetLastError()) {
-}
-
-LogMessage::SaveLastError::~SaveLastError() {
-  ::SetLastError(last_error_);
-}
-#endif  // defined(OS_WIN)
-
 LogMessage::LogMessage(const char* file, int line, LogSeverity severity)
     : severity_(severity), file_(file), line_(line) {
   Init(file, line);

base/logging.h

@@ -19,6 +19,7 @@
 #include "base/compiler_specific.h"
 #include "base/debug/debugger.h"
 #include "base/macros.h"
+#include "base/scoped_clear_last_error.h"
 #include "base/strings/string_piece_forward.h"
 #include "base/template_util.h"
 #include "build/build_config.h"
@@ -1019,25 +1020,10 @@ class BASE_EXPORT LogMessage {
   const char* file_;
   const int line_;
 
-#if defined(OS_WIN)
-  // Stores the current value of GetLastError in the constructor and restores
-  // it in the destructor by calling SetLastError.
   // This is useful since the LogMessage class uses a lot of Win32 calls
   // that will lose the value of GLE and the code that called the log function
   // will have lost the thread error value when the log call returns.
-  class SaveLastError {
-   public:
-    SaveLastError();
-    ~SaveLastError();
-
-    unsigned long get_error() const { return last_error_; }
-
-   protected:
-    unsigned long last_error_;
-  };
-
-  SaveLastError last_error_;
-#endif
+  base::internal::ScopedClearLastError last_error_;
 
   DISALLOW_COPY_AND_ASSIGN(LogMessage);
 };

base/scoped_clear_errno.h

-// Copyright (c) 2013 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 BASE_SCOPED_CLEAR_ERRNO_H_
-#define BASE_SCOPED_CLEAR_ERRNO_H_
-
-#include <errno.h>
-
-#include "base/macros.h"
-
-namespace base {
-
-// Simple scoper that saves the current value of errno, resets it to 0, and on
-// destruction puts the old value back.
-class ScopedClearErrno {
- public:
-  ScopedClearErrno() : old_errno_(errno) {
-    errno = 0;
-  }
-  ~ScopedClearErrno() {
-    if (errno == 0)
-      errno = old_errno_;
-  }
-
- private:
-  const int old_errno_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedClearErrno);
-};
-
-}  // namespace base
-
-#endif  // BASE_SCOPED_CLEAR_ERRNO_H_

base/scoped_clear_errno_unittest.cc

-// Copyright (c) 2013 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.
-
-#include <errno.h>
-
-#include "base/scoped_clear_errno.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace base {
-
-TEST(ScopedClearErrno, TestNoError) {
-  errno = 1;
-  {
-    ScopedClearErrno clear_error;
-    EXPECT_EQ(0, errno);
-  }
-  EXPECT_EQ(1, errno);
-}
-
-TEST(ScopedClearErrno, TestError) {
-  errno = 1;
-  {
-    ScopedClearErrno clear_error;
-    errno = 2;
-  }
-  EXPECT_EQ(2, errno);
-}
-
-}  // namespace base

base/scoped_clear_last_error.h

+// Copyright (c) 2018 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 BASE_SCOPED_CLEAR_LAST_ERROR_H_
+#define BASE_SCOPED_CLEAR_LAST_ERROR_H_
+
+#include <errno.h>
+
+#include "base/base_export.h"
+#include "base/macros.h"
+#include "build/build_config.h"
+
+namespace base {
+namespace internal {
+
+// ScopedClearLastError stores and resets the value of thread local error codes
+// (errno, GetLastError()), and restores them in the destructor. This is useful
+// to avoid side effects on these values in instrumentation functions that
+// interact with the OS.
+
+// Common implementation of ScopedClearLastError for all platforms. Use
+// ScopedClearLastError instead.
+class BASE_EXPORT ScopedClearLastErrorBase {
+ public:
+  ScopedClearLastErrorBase() : last_errno_(errno) { errno = 0; }
+  ~ScopedClearLastErrorBase() { errno = last_errno_; }
+
+ private:
+  const int last_errno_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedClearLastErrorBase);
+};
+
+#if defined(OS_WIN)
+
+// Windows specific implementation of ScopedClearLastError.
+class BASE_EXPORT ScopedClearLastError : public ScopedClearLastErrorBase {
+ public:
+  ScopedClearLastError();
+  ~ScopedClearLastError();
+
+ private:
+  unsigned int last_system_error_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedClearLastError);
+};
+
+#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
+
+using ScopedClearLastError = ScopedClearLastErrorBase;
+
+#endif  // defined(OS_WIN)
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_SCOPED_CLEAR_LAST_ERROR_H_

base/scoped_clear_last_error_unittest.cc

+// Copyright (c) 2018 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.
+
+#include "base/scoped_clear_last_error.h"
+
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+#endif  // defined(OS_WIN)
+
+namespace base {
+namespace internal {
+
+TEST(ScopedClearLastError, TestNoError) {
+  errno = 1;
+  {
+    ScopedClearLastError clear_error;
+    EXPECT_EQ(0, errno);
+  }
+  EXPECT_EQ(1, errno);
+}
+
+TEST(ScopedClearLastError, TestError) {
+  errno = 1;
+  {
+    ScopedClearLastError clear_error;
+    errno = 2;
+  }
+  EXPECT_EQ(1, errno);
+}
+
+#if defined(OS_WIN)
+
+TEST(ScopedClearLastError, TestNoErrorWin) {
+  ::SetLastError(1);
+  {
+    ScopedClearLastError clear_error;
+    EXPECT_EQ(logging::SystemErrorCode(0), ::GetLastError());
+  }
+  EXPECT_EQ(logging::SystemErrorCode(1), ::GetLastError());
+}
+
+TEST(ScopedClearLastError, TestErrorWin) {
+  ::SetLastError(1);
+  {
+    ScopedClearLastError clear_error;
+    ::SetLastError(2);
+  }
+  EXPECT_EQ(logging::SystemErrorCode(1), ::GetLastError());
+}
+
+#endif  // defined(OS_WIN)
+
+}  // namespace internal
+}  // namespace base

base/scoped_clear_last_error_win.cc

+// Copyright (c) 2018 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.
+
+#include "base/scoped_clear_last_error.h"
+
+#include <windows.h>
+
+namespace base {
+namespace internal {
+
+ScopedClearLastError::ScopedClearLastError()
+    : last_system_error_(::GetLastError()) {
+  ::SetLastError(0);
+}
+
+ScopedClearLastError::~ScopedClearLastError() {
+  ::SetLastError(last_system_error_);
+}
+
+}  // namespace internal
+}  // namespace base

base/strings/string_number_conversions.cc

@@ -14,7 +14,7 @@
 
 #include "base/logging.h"
 #include "base/numerics/safe_math.h"
-#include "base/scoped_clear_errno.h"
+#include "base/scoped_clear_last_error.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/third_party/dmg_fp/dmg_fp.h"
 
@@ -419,7 +419,7 @@ bool StringToSizeT(StringPiece16 input, size_t* output) {
 
 bool StringToDouble(const std::string& input, double* output) {
   // Thread-safe?  It is on at least Mac, Linux, and Windows.
-  ScopedClearErrno clear_errno;
+  internal::ScopedClearLastError clear_errno;
 
   char* endptr = nullptr;
   *output = dmg_fp::strtod(input.c_str(), &endptr);

base/strings/stringprintf.cc

@@ -10,7 +10,7 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/scoped_clear_errno.h"
+#include "base/scoped_clear_last_error.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -55,9 +55,7 @@ static void StringAppendVT(StringType* dst,
   va_list ap_copy;
   va_copy(ap_copy, ap);
 
-#if !defined(OS_WIN)
-  ScopedClearErrno clear_errno;
-#endif
+  base::internal::ScopedClearLastError last_error;
   int result = vsnprintfT(stack_buf, arraysize(stack_buf), format, ap_copy);
   va_end(ap_copy);
 

base/threading/scoped_blocking_call.cc

@@ -5,6 +5,7 @@
 #include "base/threading/scoped_blocking_call.h"
 
 #include "base/lazy_instance.h"
+#include "base/scoped_clear_last_error.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 
@@ -43,6 +44,9 @@ UncheckedScopedBlockingCall::UncheckedScopedBlockingCall(
 }
 
 UncheckedScopedBlockingCall::~UncheckedScopedBlockingCall() {
+  // TLS affects result of GetLastError() on Windows. ScopedClearLastError
+  // prevents side effect.
+  base::internal::ScopedClearLastError save_last_error;
   DCHECK_EQ(this, tls_last_scoped_blocking_call.Get().Get());
   tls_last_scoped_blocking_call.Get().Set(previous_scoped_blocking_call_);
   if (blocking_observer_ && !previous_scoped_blocking_call_)