Update clang, re-enable checker, fix issues it flags The chromium roll cc0e4f9d4e73866f10c47b46f2a369df4fd24ce9 included an update to the clang plugin that had stricter checks for inline virtual destructors that were incompatible with parts of the SDK. Thus the roll disabled the checker completely with the idea it could be re-enabled when the checker's behavior was fixed. The checker itself was fixed a few days later but the plugin was left disabled. This rolls to a newer clang (the one Chromium is currently using) which includes the updated checker and fixes the things that have crept in since the checker was disabled. R=viettrungluu@chromium.org Review URL: https://codereview.chromium.org/1060683002

diff --git a/build/config/clang/BUILD.gn b/build/config/clang/BUILD.gn
index 721e87c..39fe251 100644
--- a/build/config/clang/BUILD.gn
+++ b/build/config/clang/BUILD.gn

@@ -21,6 +21,13 @@
               "//third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so",
               root_build_dir) ]
     }
+
+    cflags += [
+      "-Xclang",
+      "-add-plugin",
+      "-Xclang",
+      "find-bad-constructs",
+    ]
   }
 }
 

diff --git a/mojo/tools/roll/disable_find_bad_constructs.patch b/mojo/tools/roll/disable_find_bad_constructs.patch
deleted file mode 100644
index 4549e5b..0000000
--- a/mojo/tools/roll/disable_find_bad_constructs.patch
+++ /dev/null

@@ -1,18 +0,0 @@
-diff --git a/build/config/clang/BUILD.gn b/build/config/clang/BUILD.gn
-index 39fe251..721e87c 100644
---- a/build/config/clang/BUILD.gn
-+++ b/build/config/clang/BUILD.gn
-@@ -21,13 +21,6 @@ config("find_bad_constructs") {
-               "//third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so",
-               root_build_dir) ]
-     }
--
--    cflags += [
--      "-Xclang",
--      "-add-plugin",
--      "-Xclang",
--      "find-bad-constructs",
--    ]
-   }
- }
- 

diff --git a/services/files/BUILD.gn b/services/files/BUILD.gn
index bf85f23..a1afcf6 100644
--- a/services/files/BUILD.gn
+++ b/services/files/BUILD.gn

@@ -40,6 +40,7 @@
   sources = [
     "directory_impl_unittest.cc",
     "file_impl_unittest.cc",
+    "files_test_base.cc",
     "files_test_base.h",
   ]
 

diff --git a/services/files/c/BUILD.gn b/services/files/c/BUILD.gn
index b3768cd..11e446a 100644
--- a/services/files/c/BUILD.gn
+++ b/services/files/c/BUILD.gn

@@ -20,6 +20,7 @@
       "lib/mojio_fcntl.cc",
       "lib/mojio_sys_stat.cc",
       "lib/mojio_unistd.cc",
+      "lib/real_errno_impl.cc",
       "lib/real_errno_impl.h",
       "lib/singletons.cc",
       "lib/singletons.h",
@@ -53,9 +54,12 @@
       "tests/errno_impl_unittest.cc",
       "tests/fd_table_unittest.cc",
       "tests/file_fd_impl_unittest.cc",
+      "tests/mock_errno_impl.cc",
       "tests/mock_errno_impl.h",
+      "tests/mojio_impl_test_base.cc",
       "tests/mojio_impl_test_base.h",
       "tests/mojio_sys_stat_unittest.cc",
+      "tests/mojio_test_base.cc",
       "tests/mojio_test_base.h",
       "tests/mojio_unistd_unittest.cc",
       "tests/real_errno_impl_unittest.cc",

diff --git a/services/files/c/lib/real_errno_impl.cc b/services/files/c/lib/real_errno_impl.cc
new file mode 100644
index 0000000..f130d60
--- /dev/null
+++ b/services/files/c/lib/real_errno_impl.cc

@@ -0,0 +1,19 @@
+// Copyright 2015 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 "services/files/c/lib/real_errno_impl.h"
+
+#include <errno.h>
+
+namespace mojio {
+
+int RealErrnoImpl::Get() const {
+  return errno;
+}
+
+void RealErrnoImpl::Set(int error) {
+  errno = error;
+}
+
+}  // namespace mojio

diff --git a/services/files/c/lib/real_errno_impl.h b/services/files/c/lib/real_errno_impl.h
index 5f97efb..b32e143 100644
--- a/services/files/c/lib/real_errno_impl.h
+++ b/services/files/c/lib/real_errno_impl.h

@@ -5,8 +5,6 @@
 #ifndef SERVICES_FILES_C_LIB_REAL_ERRNO_IMPL_H_
 #define SERVICES_FILES_C_LIB_REAL_ERRNO_IMPL_H_
 
-#include <errno.h>
-
 #include "mojo/public/cpp/system/macros.h"
 #include "services/files/c/lib/errno_impl.h"
 
@@ -20,8 +18,8 @@
   RealErrnoImpl() {}
   ~RealErrnoImpl() override {}
 
-  int Get() const override { return errno; }
-  void Set(int error) override { errno = error; }
+  int Get() const override;
+  void Set(int error) override;
 
  private:
   MOJO_DISALLOW_COPY_AND_ASSIGN(RealErrnoImpl);

diff --git a/services/files/c/tests/fd_table_unittest.cc b/services/files/c/tests/fd_table_unittest.cc
index 72b2db9..1e4a090 100644
--- a/services/files/c/tests/fd_table_unittest.cc
+++ b/services/files/c/tests/fd_table_unittest.cc

@@ -18,7 +18,7 @@
 class TestFDImpl : public FDImpl {
  public:
   TestFDImpl() : FDImpl(nullptr) {}
-  virtual ~TestFDImpl() {}
+  ~TestFDImpl() override {}
 
   // |FDImpl| implementation:
   bool Close() override { return false; }

diff --git a/services/files/c/tests/mock_errno_impl.cc b/services/files/c/tests/mock_errno_impl.cc
new file mode 100644
index 0000000..09bd4dc
--- /dev/null
+++ b/services/files/c/tests/mock_errno_impl.cc

@@ -0,0 +1,20 @@
+// Copyright 2015 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 "services/files/c/tests/mock_errno_impl.h"
+
+namespace mojio {
+namespace test {
+
+int MockErrnoImpl::Get() const {
+  return last_error_;
+}
+
+void MockErrnoImpl::Set(int error) {
+  last_error_ = error;
+  was_set_ = true;
+}
+
+}  // namespace test
+}  // namespace mojio

diff --git a/services/files/c/tests/mock_errno_impl.h b/services/files/c/tests/mock_errno_impl.h
index f9675d0..89a08f8 100644
--- a/services/files/c/tests/mock_errno_impl.h
+++ b/services/files/c/tests/mock_errno_impl.h

@@ -17,11 +17,8 @@
   ~MockErrnoImpl() override {}
 
   // |ErrnoImpl| implementation:
-  int Get() const override { return last_error_; }
-  void Set(int error) override {
-    last_error_ = error;
-    was_set_ = true;
-  }
+  int Get() const override;
+  void Set(int error) override;
 
   // Reset to initial state (with specified value for |last_error_|).
   void Reset(int error) {

diff --git a/services/files/c/tests/mojio_impl_test_base.cc b/services/files/c/tests/mojio_impl_test_base.cc
new file mode 100644
index 0000000..e71c778
--- /dev/null
+++ b/services/files/c/tests/mojio_impl_test_base.cc

@@ -0,0 +1,36 @@
+// Copyright 2015 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 "services/files/c/tests/mojio_impl_test_base.h"
+
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "mojo/public/cpp/environment/logging.h"
+#include "mojo/services/files/public/interfaces/files.mojom.h"
+#include "services/files/c/lib/template_util.h"
+
+namespace mojio {
+namespace test {
+
+MojioImplTestBase::MojioImplTestBase() {
+}
+
+MojioImplTestBase::~MojioImplTestBase() {
+}
+
+void MojioImplTestBase::SetUp() {
+  mojo::test::ApplicationTestBase::SetUp();
+
+  mojo::files::FilesPtr files;
+  application_impl()->ConnectToService("mojo:files", &files);
+
+  mojo::files::Error error = mojo::files::ERROR_INTERNAL;
+  files->OpenFileSystem(mojo::files::FILE_SYSTEM_TEMPORARY,
+                        mojo::GetProxy(&directory_), Capture(&error));
+  MOJO_CHECK(files.WaitForIncomingMethodCall());
+  MOJO_CHECK(error == mojo::files::ERROR_OK);
+}
+
+}  // namespace test
+}  // namespace mojio

diff --git a/services/files/c/tests/mojio_impl_test_base.h b/services/files/c/tests/mojio_impl_test_base.h
index bd2d6c8..46d283f 100644
--- a/services/files/c/tests/mojio_impl_test_base.h
+++ b/services/files/c/tests/mojio_impl_test_base.h

@@ -5,14 +5,9 @@
 #ifndef SERVICES_FILES_C_TESTS_MOJIO_IMPL_TEST_BASE_H_
 #define SERVICES_FILES_C_TESTS_MOJIO_IMPL_TEST_BASE_H_
 
-#include "mojo/public/cpp/application/application_impl.h"
 #include "mojo/public/cpp/application/application_test_base.h"
-#include "mojo/public/cpp/bindings/interface_request.h"
-#include "mojo/public/cpp/environment/logging.h"
 #include "mojo/public/cpp/system/macros.h"
 #include "mojo/services/files/public/interfaces/directory.mojom.h"
-#include "mojo/services/files/public/interfaces/files.mojom.h"
-#include "services/files/c/lib/template_util.h"
 
 namespace mojio {
 namespace test {
@@ -21,21 +16,10 @@
 // doesn't use the singletons).
 class MojioImplTestBase : public mojo::test::ApplicationTestBase {
  public:
-  MojioImplTestBase() {}
-  ~MojioImplTestBase() override {}
+  MojioImplTestBase();
+  ~MojioImplTestBase() override;
 
-  void SetUp() override {
-    mojo::test::ApplicationTestBase::SetUp();
-
-    mojo::files::FilesPtr files;
-    application_impl()->ConnectToService("mojo:files", &files);
-
-    mojo::files::Error error = mojo::files::ERROR_INTERNAL;
-    files->OpenFileSystem(mojo::files::FILE_SYSTEM_TEMPORARY,
-                          mojo::GetProxy(&directory_), Capture(&error));
-    MOJO_CHECK(files.WaitForIncomingMethodCall());
-    MOJO_CHECK(error == mojo::files::ERROR_OK);
-  }
+  void SetUp() override;
 
  protected:
   mojo::files::DirectoryPtr& directory() { return directory_; }

diff --git a/services/files/c/tests/mojio_test_base.cc b/services/files/c/tests/mojio_test_base.cc
new file mode 100644
index 0000000..6451607
--- /dev/null
+++ b/services/files/c/tests/mojio_test_base.cc

@@ -0,0 +1,33 @@
+// Copyright 2015 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 "services/files/c/tests/mojio_test_base.h"
+
+#include "mojo/public/cpp/environment/logging.h"
+#include "services/files/c/lib/singletons.h"
+
+namespace mojio {
+namespace test {
+
+void MojioTestBase::SetUp() {
+  MojioImplTestBase::SetUp();
+
+  // Explicitly set up the singletons.
+  MOJO_CHECK(singletons::GetErrnoImpl());
+  MOJO_CHECK(singletons::GetFDTable());
+  singletons::SetCurrentWorkingDirectory(directory().Pass());
+  MOJO_CHECK(singletons::GetCurrentWorkingDirectory());
+}
+
+void MojioTestBase::TearDown() {
+  // Explicitly tear down the singletons.
+  singletons::ResetCurrentWorkingDirectory();
+  singletons::ResetFDTable();
+  singletons::ResetErrnoImpl();
+
+  MojioImplTestBase::TearDown();
+}
+
+}  // namespace test
+}  // namespace mojio

diff --git a/services/files/c/tests/mojio_test_base.h b/services/files/c/tests/mojio_test_base.h
index 61404ab..a63e9e3 100644
--- a/services/files/c/tests/mojio_test_base.h
+++ b/services/files/c/tests/mojio_test_base.h

@@ -5,8 +5,6 @@
 #ifndef SERVICES_FILES_C_TESTS_MOJIO_TEST_BASE_H_
 #define SERVICES_FILES_C_TESTS_MOJIO_TEST_BASE_H_
 
-#include "mojo/public/cpp/environment/logging.h"
-#include "services/files/c/lib/singletons.h"
 #include "services/files/c/tests/mojio_impl_test_base.h"
 
 namespace mojio {
@@ -19,24 +17,8 @@
   MojioTestBase() {}
   ~MojioTestBase() override {}
 
-  void SetUp() override {
-    MojioImplTestBase::SetUp();
-
-    // Explicitly set up the singletons.
-    MOJO_CHECK(singletons::GetErrnoImpl());
-    MOJO_CHECK(singletons::GetFDTable());
-    singletons::SetCurrentWorkingDirectory(directory().Pass());
-    MOJO_CHECK(singletons::GetCurrentWorkingDirectory());
-  }
-
-  void TearDown() override {
-    // Explicitly tear down the singletons.
-    singletons::ResetCurrentWorkingDirectory();
-    singletons::ResetFDTable();
-    singletons::ResetErrnoImpl();
-
-    MojioImplTestBase::TearDown();
-  }
+  void SetUp() override;
+  void TearDown() override;
 
  private:
   MOJO_DISALLOW_COPY_AND_ASSIGN(MojioTestBase);

diff --git a/services/files/file_impl.h b/services/files/file_impl.h
index 58fa94c..355b117 100644
--- a/services/files/file_impl.h
+++ b/services/files/file_impl.h

@@ -55,7 +55,7 @@
   void AsBuffer(const AsBufferCallback& callback) override;
   void Ioctl(uint32_t request,
              Array<uint32_t> in_values,
-             const IoctlCallback& callback);
+             const IoctlCallback& callback) override;
 
  private:
   StrongBinding<File> binding_;

diff --git a/services/files/files_test_base.cc b/services/files/files_test_base.cc
new file mode 100644
index 0000000..132cb00
--- /dev/null
+++ b/services/files/files_test_base.cc

@@ -0,0 +1,34 @@
+// Copyright 2015 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 "services/files/files_test_base.h"
+
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/services/files/public/interfaces/directory.mojom.h"
+#include "mojo/services/files/public/interfaces/types.mojom.h"
+
+namespace mojo {
+namespace files {
+
+FilesTestBase::FilesTestBase() {
+}
+
+FilesTestBase::~FilesTestBase() {
+}
+
+void FilesTestBase::SetUp() {
+  test::ApplicationTestBase::SetUp();
+  application_impl()->ConnectToService("mojo:files", &files_);
+}
+
+void FilesTestBase::GetTemporaryRoot(DirectoryPtr* directory) {
+  Error error = ERROR_INTERNAL;
+  files()->OpenFileSystem(FILE_SYSTEM_TEMPORARY, GetProxy(directory),
+                          Capture(&error));
+  ASSERT_TRUE(files().WaitForIncomingMethodCall());
+  ASSERT_EQ(ERROR_OK, error);
+}
+
+}  // namespace files
+}  // namespace mojo

diff --git a/services/files/files_test_base.h b/services/files/files_test_base.h
index 5ca38f2..d44d7df 100644
--- a/services/files/files_test_base.h
+++ b/services/files/files_test_base.h

@@ -6,11 +6,8 @@
 #define SERVICES_FILES_FILES_TEST_BASE_H_
 
 #include "base/macros.h"
-#include "mojo/public/cpp/application/application_impl.h"
 #include "mojo/public/cpp/application/application_test_base.h"
-#include "mojo/services/files/public/interfaces/directory.mojom.h"
 #include "mojo/services/files/public/interfaces/files.mojom.h"
-#include "mojo/services/files/public/interfaces/types.mojom.h"
 
 namespace mojo {
 namespace files {
@@ -84,24 +81,15 @@
 
 class FilesTestBase : public test::ApplicationTestBase {
  public:
-  FilesTestBase() {}
-  ~FilesTestBase() override {}
+  FilesTestBase();
+  ~FilesTestBase() override;
 
-  void SetUp() override {
-    test::ApplicationTestBase::SetUp();
-    application_impl()->ConnectToService("mojo:files", &files_);
-  }
+  void SetUp() override;
 
  protected:
   // Note: This has an out parameter rather than returning the |DirectoryPtr|,
   // since |ASSERT_...()| doesn't work with return values.
-  void GetTemporaryRoot(DirectoryPtr* directory) {
-    Error error = ERROR_INTERNAL;
-    files()->OpenFileSystem(FILE_SYSTEM_TEMPORARY, GetProxy(directory),
-                            Capture(&error));
-    ASSERT_TRUE(files().WaitForIncomingMethodCall());
-    ASSERT_EQ(ERROR_OK, error);
-  }
+  void GetTemporaryRoot(DirectoryPtr* directory);
 
   FilesPtr& files() { return files_; }
 

diff --git a/services/native_viewport/native_viewport_impl.h b/services/native_viewport/native_viewport_impl.h
index 741b95c..4ac9bbd 100644
--- a/services/native_viewport/native_viewport_impl.h
+++ b/services/native_viewport/native_viewport_impl.h

@@ -76,11 +76,12 @@
   RequestMetricsCallback metrics_callback_;
   mojo::NativeViewportEventDispatcherPtr event_dispatcher_;
   mojo::Binding<mojo::NativeViewport> binding_;
-  base::WeakPtrFactory<NativeViewportImpl> weak_factory_;
 
   // Set of pointer_ids we've sent a move to and are waiting on an ack.
   std::set<int32> pointers_waiting_on_ack_;
 
+  base::WeakPtrFactory<NativeViewportImpl> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeViewportImpl);
 };
 

diff --git a/services/reaper/reaper_binding.cc b/services/reaper/reaper_binding.cc
index d9e908d..169dc4a 100644
--- a/services/reaper/reaper_binding.cc
+++ b/services/reaper/reaper_binding.cc

@@ -14,6 +14,8 @@
     : caller_url_(caller_url), impl_(impl), binding_(this, request.Pass()) {
 }
 
+ReaperBinding::~ReaperBinding() = default;
+
 void ReaperBinding::GetApplicationSecret(
     const mojo::Callback<void(uint64)>& callback) {
   impl_->GetApplicationSecret(caller_url_, callback);

diff --git a/services/reaper/reaper_binding.h b/services/reaper/reaper_binding.h
index fabd7f7..cf13aa7 100644
--- a/services/reaper/reaper_binding.h
+++ b/services/reaper/reaper_binding.h

@@ -24,7 +24,7 @@
                 mojo::InterfaceRequest<Reaper> request);
 
  protected:
-  ~ReaperBinding() override = default;
+  ~ReaperBinding() override;
 
  private:
   // Reaper

diff --git a/services/reaper/transfer_binding.cc b/services/reaper/transfer_binding.cc
index 2c82f62..2c5fa6a 100644
--- a/services/reaper/transfer_binding.cc
+++ b/services/reaper/transfer_binding.cc

@@ -14,6 +14,8 @@
     : node_id_(node_id), impl_(impl), binding_(this, request.Pass()) {
 }
 
+TransferBinding::~TransferBinding() = default;
+
 void TransferBinding::Complete(uint64 dest_app_secret, uint32 dest_node_id) {
   impl_->CompleteTransfer(node_id_, dest_app_secret, dest_node_id);
 }

diff --git a/services/reaper/transfer_binding.h b/services/reaper/transfer_binding.h
index fc0268d..7ff7c8b 100644
--- a/services/reaper/transfer_binding.h
+++ b/services/reaper/transfer_binding.h

@@ -22,7 +22,7 @@
                   mojo::InterfaceRequest<Transfer> request);
 
  protected:
-  ~TransferBinding() override = default;
+  ~TransferBinding() override;
 
  private:
   // Transfer

diff --git a/shell/child_process_host.h b/shell/child_process_host.h
index ab29982..c537615 100644
--- a/shell/child_process_host.h
+++ b/shell/child_process_host.h

@@ -29,7 +29,7 @@
 class ChildProcessHost : public ErrorHandler {
  public:
   explicit ChildProcessHost(Context* context);
-  virtual ~ChildProcessHost();
+  ~ChildProcessHost() override;
 
   // |Start()|s the child process; calls |DidStart()| (on the thread on which
   // |Start()| was called) when the child has been started (or failed to start).

diff --git a/tools/clang/scripts/update.sh b/tools/clang/scripts/update.sh
index c842a58..10a4645 100755
--- a/tools/clang/scripts/update.sh
+++ b/tools/clang/scripts/update.sh

@@ -8,7 +8,7 @@
 # Do NOT CHANGE this if you don't know what you're doing -- see
 # https://code.google.com/p/chromium/wiki/UpdatingClang
 # Reverting problematic clang rolls is safe, though.
-CLANG_REVISION=231690
+CLANG_REVISION=233105
 
 # This is incremented when pushing a new build of Clang at the same revision.
 CLANG_SUB_REVISION=1