Enforce WARN_UNUSED_RESULT attribute on Windows

BUG=none
R=cpu,danakj,dpranke,hans,kelvinp,rickyz,sky,sorin,thestig,tuttle,yukawa
TBR=vitalybuka

Review URL: https://codereview.chromium.org/1390223002

Cr-Commit-Position: refs/heads/master@{#353450}

19 files changed, 79 insertions, 28 deletions

diff --git a/base/compiler_specific.h b/base/compiler_specific.h
index 02603df..d5d38b7 100644
--- a/base/compiler_specific.h
+++ b/base/compiler_specific.h
@@ -119,7 +119,9 @@
 // Use like:
 //   int foo() WARN_UNUSED_RESULT;
 // To explicitly ignore a result, see |ignore_result()| in base/macros.h.
-#if defined(COMPILER_GCC)
+// TODO(dcheng): Update //third_party/webrtc's macro definition to match.
+#undef WARN_UNUSED_RESULT
+#if defined(COMPILER_GCC) || defined(__clang__)
 #define WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #else
 #define WARN_UNUSED_RESULT
diff --git a/build/common.gypi b/build/common.gypi
index ea6da8e..944dcb6 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -5780,7 +5780,10 @@
                   '-Qunused-arguments',  # http://crbug.com/504658
                   '-Wno-microsoft-enum-value',  # http://crbug.com/505296
                   '-Wno-unknown-pragmas',  # http://crbug.com/505314
-                  '-Wno-unused-value',  # http://crbug.com/505318
+                  # Disable unused-value (crbug.com/505318) except
+                  # -Wunused-result.
+                  '-Wno-unused-value',
+                  '-Wunused-result',
                 ],
               },
             }],
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
index 6e5644c..de7167d 100644
--- a/build/config/compiler/BUILD.gn
+++ b/build/config/compiler/BUILD.gn
@@ -637,7 +637,10 @@ config("default_warnings") {
         "-Qunused-arguments",  # http://crbug.com/504658
         "-Wno-microsoft-enum-value",  # http://crbug.com/505296
         "-Wno-unknown-pragmas",  # http://crbug.com/505314
-        "-Wno-unused-value",  # http://crbug.com/505318
+
+        # Disable unused-value (crbug.com/505318) except -Wunused-result.
+        "-Wno-unused-value",
+        "-Wunused-result",
       ]
     }
   } else {
diff --git a/chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win_unittest.cc b/chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win_unittest.cc
index 8270b45..9ef043b 100644
--- a/chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win_unittest.cc
+++ b/chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win_unittest.cc
@@ -56,7 +56,7 @@ class BinaryIntegrityAnalyzerWinTest : public ::testing::Test {
 };
 
 BinaryIntegrityAnalyzerWinTest::BinaryIntegrityAnalyzerWinTest() {
-  temp_dir_.CreateUniqueTempDir();
+  EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
   base::CreateDirectory(temp_dir_.path().AppendASCII(CHROME_VERSION_STRING));
 
   // We retrieve DIR_TEST_DATA here because it is based on DIR_EXE and we are
diff --git a/chrome/installer/util/delete_tree_work_item.cc b/chrome/installer/util/delete_tree_work_item.cc
index 9cc47ed..dcb8d1e 100644
--- a/chrome/installer/util/delete_tree_work_item.cc
+++ b/chrome/installer/util/delete_tree_work_item.cc
@@ -67,7 +67,10 @@ bool DeleteTreeWorkItem::Do() {
         PLOG(ERROR) << "Could not back up " << key_file.value()
                     << " to directory " << backup.path().value();
         abort = true;
-        backup.Delete();
+        if (!backup.Delete()) {
+          PLOG(ERROR) << "Could not clean up temp dir in "
+                      << temp_path_.value();
+        }
       }
     }
     if (!abort) {
diff --git a/chrome/service/cloud_print/print_system_win.cc b/chrome/service/cloud_print/print_system_win.cc
index 26c56263..02016cf 100644
--- a/chrome/service/cloud_print/print_system_win.cc
+++ b/chrome/service/cloud_print/print_system_win.cc
@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/json/json_writer.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
@@ -435,7 +436,7 @@ class JobSpoolerWin : public PrintSystem::JobSpooler {
               pdf_path,
               printing::PdfRenderSettings(render_area, render_dpi, false))) {
         // The object will self-destruct when the child process dies.
-        utility_host.release();
+        ignore_result(utility_host.release());
       } else {
         client_task_runner->PostTask(
             FROM_HERE, base::Bind(&Core::PrintJobDone, this, false));
@@ -575,7 +576,7 @@ class PrinterCapsHandler : public ServiceUtilityProcessHost::Client {
         new ServiceUtilityProcessHost(this, client_task_runner.get()));
     if (utility_host->StartGetPrinterCapsAndDefaults(printer_name_)) {
       // The object will self-destruct when the child process dies.
-      utility_host.release();
+      ignore_result(utility_host.release());
     } else {
       client_task_runner->PostTask(
           FROM_HERE, base::Bind(&PrinterCapsHandler::OnChildDied, this));
@@ -589,7 +590,7 @@ class PrinterCapsHandler : public ServiceUtilityProcessHost::Client {
         new ServiceUtilityProcessHost(this, client_task_runner.get()));
     if (utility_host->StartGetPrinterSemanticCapsAndDefaults(printer_name_)) {
       // The object will self-destruct when the child process dies.
-      utility_host.release();
+      ignore_result(utility_host.release());
     } else {
       client_task_runner->PostTask(
           FROM_HERE, base::Bind(&PrinterCapsHandler::OnChildDied, this));
diff --git a/chrome/utility/importer/ie_importer_win.cc b/chrome/utility/importer/ie_importer_win.cc
index 6bff0e4..bc017ac 100644
--- a/chrome/utility/importer/ie_importer_win.cc
+++ b/chrome/utility/importer/ie_importer_win.cc
@@ -160,11 +160,20 @@ struct IEOrderBookmarkComparator {
 //   };
 // where each item_id should correspond to a favorites link file (*.url) in
 // the current folder.
-bool ParseFavoritesOrderBlob(
-    const Importer* importer,
-    const std::vector<uint8>& blob,
-    const base::FilePath& path,
-    std::map<base::FilePath, uint32>* sort_index) WARN_UNUSED_RESULT {
+// gcc, in its infinite wisdom, only allows WARN_UNUSED_RESULT for prototypes.
+// Clang, to be compatible with gcc, warns if WARN_UNUSED_RESULT is used in a
+// non-gcc compatible manner (-Wgcc-compat). So even though gcc isn't used to
+// build on Windows, declare some prototypes anyway to satisfy Clang's gcc
+// compatibility warnings.
+bool ParseFavoritesOrderBlob(const Importer* importer,
+                             const std::vector<uint8>& blob,
+                             const base::FilePath& path,
+                             std::map<base::FilePath, uint32>* sort_index)
+    WARN_UNUSED_RESULT;
+bool ParseFavoritesOrderBlob(const Importer* importer,
+                             const std::vector<uint8>& blob,
+                             const base::FilePath& path,
+                             std::map<base::FilePath, uint32>* sort_index) {
   static const int kItemCountOffset = 16;
   static const int kItemListStartOffset = 20;
 
@@ -212,7 +221,12 @@ bool ParseFavoritesOrderRegistryTree(
     const Importer* importer,
     const base::win::RegKey& key,
     const base::FilePath& path,
-    std::map<base::FilePath, uint32>* sort_index) WARN_UNUSED_RESULT {
+    std::map<base::FilePath, uint32>* sort_index) WARN_UNUSED_RESULT;
+bool ParseFavoritesOrderRegistryTree(
+    const Importer* importer,
+    const base::win::RegKey& key,
+    const base::FilePath& path,
+    std::map<base::FilePath, uint32>* sort_index) {
   // Parse the order information of the current folder.
   DWORD blob_length = 0;
   if (key.ReadValue(L"Order", NULL, &blob_length, NULL) == ERROR_SUCCESS) {
@@ -241,9 +255,11 @@ bool ParseFavoritesOrderRegistryTree(
   return true;
 }
 
-bool ParseFavoritesOrderInfo(
-    const Importer* importer,
-    std::map<base::FilePath, uint32>* sort_index) WARN_UNUSED_RESULT {
+bool ParseFavoritesOrderInfo(const Importer* importer,
+                             std::map<base::FilePath, uint32>* sort_index)
+    WARN_UNUSED_RESULT;
+bool ParseFavoritesOrderInfo(const Importer* importer,
+                             std::map<base::FilePath, uint32>* sort_index) {
   base::string16 key_path(importer::GetIEFavoritesOrderKey());
   base::win::RegKey key(HKEY_CURRENT_USER, key_path.c_str(), KEY_READ);
   if (!key.Valid())
diff --git a/cloud_print/service/win/service_listener.cc b/cloud_print/service/win/service_listener.cc
index 52d8ab8..31173c7 100644
--- a/cloud_print/service/win/service_listener.cc
+++ b/cloud_print/service/win/service_listener.cc
@@ -96,7 +96,8 @@ void ServiceListener::Connect() {
     // need for an attachment broker.
     channel_ =
         IPC::Channel::CreateClient(IPC::ChannelHandle(handle.Get()), this);
-    channel_->Connect();
+    bool connected = channel_->Connect();
+    DCHECK(connected);
   } else {
     ipc_thread_->message_loop()->PostDelayedTask(
         FROM_HERE,
diff --git a/cloud_print/service/win/setup_listener.cc b/cloud_print/service/win/setup_listener.cc
index 67cb85b..c4ffc79 100644
--- a/cloud_print/service/win/setup_listener.cc
+++ b/cloud_print/service/win/setup_listener.cc
@@ -119,7 +119,8 @@ void SetupListener::Connect(const base::string16& user) {
   if (pipe.IsValid()) {
     channel_ = IPC::Channel::CreateServer(IPC::ChannelHandle(pipe.Get()),
                                           this);
-    channel_->Connect();
+    if (!channel_->Connect())
+      done_event_->Signal();
   }
 }
 
diff --git a/components/storage_monitor.gypi b/components/storage_monitor.gypi
index a92fde4..d517672 100644
--- a/components/storage_monitor.gypi
+++ b/components/storage_monitor.gypi
@@ -121,6 +121,11 @@
             '../device/media_transfer_protocol/media_transfer_protocol.gyp:mtp_storage_info_proto',
           ],
         }],
+        ['OS=="win"', {
+          'dependencies': [
+            '../testing/gtest.gyp:gtest',
+          ],
+        }],
       ],
     },
   ],
diff --git a/components/storage_monitor/BUILD.gn b/components/storage_monitor/BUILD.gn
index ccb33ce..4a72f79 100644
--- a/components/storage_monitor/BUILD.gn
+++ b/components/storage_monitor/BUILD.gn
@@ -86,6 +86,7 @@ source_set("storage_monitor") {
 
 # GYP version: components/storage_monitor.gypi:storage_monitor_test_support
 source_set("test_support") {
+  testonly = true
   sources = [
     "mock_removable_storage_observer.cc",
     "mock_removable_storage_observer.h",
@@ -113,6 +114,12 @@ source_set("test_support") {
       "//device/media_transfer_protocol:mtp_storage_info_proto",
     ]
   }
+
+  if (is_win) {
+    deps = [
+      "//testing/gtest",
+    ]
+  }
 }
 
 source_set("unit_tests") {
diff --git a/components/storage_monitor/test_volume_mount_watcher_win.cc b/components/storage_monitor/test_volume_mount_watcher_win.cc
index 234b87f..7464545 100644
--- a/components/storage_monitor/test_volume_mount_watcher_win.cc
+++ b/components/storage_monitor/test_volume_mount_watcher_win.cc
@@ -12,6 +12,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/storage_monitor/storage_info.h"
 #include "content/public/browser/browser_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace storage_monitor {
 
@@ -19,7 +20,7 @@ namespace {
 
 base::FilePath GetTempRoot() {
   base::ScopedTempDir temp_dir;
-  temp_dir.CreateUniqueTempDir();
+  EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath temp_root = temp_dir.path();
   while (temp_root.DirName() != temp_root)
     temp_root = temp_root.DirName();
diff --git a/components/update_client/background_downloader_win.cc b/components/update_client/background_downloader_win.cc
index 262824a..1e449d3 100644
--- a/components/update_client/background_downloader_win.cc
+++ b/components/update_client/background_downloader_win.cc
@@ -16,6 +16,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_util.h"
+#include "base/macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
@@ -410,7 +411,7 @@ BackgroundDownloader::~BackgroundDownloader() {
   // is shutting down while a download is in progress, the timer is active and
   // the interface pointers are valid. Releasing the ownership means leaking
   // these objects and their associated resources.
-  timer_.release();
+  ignore_result(timer_.release());
   bits_manager_.Detach();
   job_.Detach();
 }
diff --git a/media/midi/midi_manager_win.cc b/media/midi/midi_manager_win.cc
index 40f2fb8..4adedbb 100644
--- a/media/midi/midi_manager_win.cc
+++ b/media/midi/midi_manager_win.cc
@@ -27,6 +27,7 @@
 
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
+#include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
@@ -168,7 +169,7 @@ void SendLongMidiMessageInternal(HMIDIOUT midi_out_handle,
   }
 
   // The ownership of |midi_header| is moved to MOM_DONE event handler.
-  midi_header.release();
+  ignore_result(midi_header.release());
 }
 
 template <size_t array_size>
diff --git a/net/dns/address_sorter_win.cc b/net/dns/address_sorter_win.cc
index 8d97a41..f469dae 100644
--- a/net/dns/address_sorter_win.cc
+++ b/net/dns/address_sorter_win.cc
@@ -109,8 +109,11 @@ class AddressSorterWin : public AddressSorter {
         list.reserve(output_buffer_->iAddressCount);
         for (int i = 0; i < output_buffer_->iAddressCount; ++i) {
           IPEndPoint ipe;
-          ipe.FromSockAddr(output_buffer_->Address[i].lpSockaddr,
-                           output_buffer_->Address[i].iSockaddrLength);
+          bool result =
+              ipe.FromSockAddr(output_buffer_->Address[i].lpSockaddr,
+                               output_buffer_->Address[i].iSockaddrLength);
+          DCHECK(result) << "Unable to roundtrip between IPEndPoint and "
+                         << "SOCKET_ADDRESS!";
           // Unmap V4MAPPED IPv6 addresses so that Happy Eyeballs works.
           if (IsIPv4Mapped(ipe.address())) {
             ipe = IPEndPoint(ConvertIPv4MappedToIPv4(ipe.address()),
diff --git a/remoting/host/setup/daemon_controller_delegate_win.cc b/remoting/host/setup/daemon_controller_delegate_win.cc
index 053d867..7026794 100644
--- a/remoting/host/setup/daemon_controller_delegate_win.cc
+++ b/remoting/host/setup/daemon_controller_delegate_win.cc
@@ -10,6 +10,7 @@
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "base/win/scoped_bstr.h"
@@ -81,7 +82,7 @@ bool ReadConfig(const base::FilePath& filename,
     return false;
   }
 
-  value.release();
+  ignore_result(value.release());
   config_out->reset(dictionary);
   return true;
 }
diff --git a/sandbox/win/src/broker_services.cc b/sandbox/win/src/broker_services.cc
index 92cd0e0..9579daa 100644
--- a/sandbox/win/src/broker_services.cc
+++ b/sandbox/win/src/broker_services.cc
@@ -7,6 +7,7 @@
 #include <AclAPI.h>
 
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
 #include "base/threading/platform_thread.h"
@@ -551,7 +552,7 @@ ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
   }
 
   // Release the pointer since it will be cleaned up by the callback.
-  peer.release();
+  ignore_result(peer.release());
   return SBOX_ALL_OK;
 }
 
diff --git a/sandbox/win/src/target_process.cc b/sandbox/win/src/target_process.cc
index 84d76b0..416713f 100644
--- a/sandbox/win/src/target_process.cc
+++ b/sandbox/win/src/target_process.cc
@@ -5,6 +5,7 @@
 #include "sandbox/win/src/target_process.h"
 
 #include "base/basictypes.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/win/pe_image.h"
 #include "base/win/startup_information.h"
@@ -99,7 +100,7 @@ TargetProcess::~TargetProcess() {
       // that.
       if (shared_section_.IsValid())
         shared_section_.Take();
-      ipc_server_.release();
+      ignore_result(ipc_server_.release());
       sandbox_process_info_.TakeProcessHandle();
       return;
     }
diff --git a/ui/gfx/icon_util_unittest.cc b/ui/gfx/icon_util_unittest.cc
index d077f3c..5138e70 100644
--- a/ui/gfx/icon_util_unittest.cc
+++ b/ui/gfx/icon_util_unittest.cc
@@ -29,7 +29,7 @@ class IconUtilTest : public testing::Test {
   void SetUp() override {
     gfx::RegisterPathProvider();
     ASSERT_TRUE(PathService::Get(gfx::DIR_TEST_DATA, &test_data_directory_));
-    temp_directory_.CreateUniqueTempDir();
+    ASSERT_TRUE(temp_directory_.CreateUniqueTempDir());
   }
 
   static const int kSmallIconWidth = 16;