[Sync] Refactor sync datatype error handling.

chrome/browser/sync/api/sync_error.cc

Index: chrome/browser/sync/api/sync_error.cc
diff --git a/chrome/browser/sync/api/sync_error.cc b/chrome/browser/sync/api/sync_error.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d4a12731741e37b6275bdeda3cdd098cf07797d2
--- /dev/null
+++ b/chrome/browser/sync/api/sync_error.cc
@@ -0,0 +1,84 @@
+// Copyright (c) 2011 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 "chrome/browser/sync/api/sync_error.h"
+
+#include "base/logging.h"
+#include "base/tracked.h"
+
+SyncError::SyncError() {

[akalin, 2011/07/25 22:43:13]

It would be good to initialize all variables, especially type_.  Suggest making
a private (or maybe public?) member function Clear() and calling that here.

[Nicolas Zea, 2011/07/25 23:08:19]

Done.

+}
+
+SyncError::SyncError(const tracked_objects::Location& location,
+                     const std::string& message,
+                     syncable::ModelType type) {
+  Init(location, message, type);
+  PrintLogError();
+}
+
+SyncError::SyncError(const SyncError& other) {
+  Copy(other);
+}
+
+SyncError& SyncError::operator=(const SyncError& other) {
+  if (this == &other) {

[akalin, 2011/07/25 22:43:13]

I don't think we should bother explicitly checking for self-assignment

[Nicolas Zea, 2011/07/25 23:08:19]

There's no point in copying if we're not changing anything, so why not shortcut
it and save the memory allocations? Otherwise we're doing location_.reset(new
Location(*location_.get())).

[akalin, 2011/07/25 23:49:32]

Yeah, but it's not a very common case, so I don't think it's worth the extra
code to handle it.

+    return *this;
+  }
+  Copy(other);
+  return *this;
+}
+
+void SyncError::Copy(const SyncError& other) {
+  if (other.IsSet()) {
+    Init(other.location(),
+         other.message(),
+         other.type());
+  } else {
+    location_.reset();

[akalin, 2011/07/25 22:43:13]

Move this code to Clear() and call that here.

[Nicolas Zea, 2011/07/25 23:08:19]

Done.

+    message_ = std::string();
+    type_ = syncable::UNSPECIFIED;
+  }
+}
+
+void SyncError::Reset(const tracked_objects::Location& location,
+                      const std::string& message,
+                      syncable::ModelType type) {
+  Init(location, message, type);
+  PrintLogError();
+}
+
+void SyncError::Init(const tracked_objects::Location& location,
+                     const std::string& message,
+                     syncable::ModelType type) {
+  location_.reset(new tracked_objects::Location(location));
+  message_ = message;
+  type_ = type;
+}
+
+bool SyncError::IsSet() const {
+  return location_.get() != NULL;
+}
+
+const tracked_objects::Location& SyncError::location() const {
+  CHECK(IsSet());
+  return *location_;
+}
+
+const std::string& SyncError::message() const {
+  CHECK(IsSet());
+  return message_;
+}
+
+const syncable::ModelType SyncError::type() const {
+  CHECK(IsSet());
+  return type_;
+}
+
+void SyncError::PrintLogError() const {
+  LAZY_STREAM(logging::LogMessage(location_->file_name(),
+                                  location_->line_number(),
+                                  logging::LOG_ERROR).stream(),
+              LOG_IS_ON(ERROR))
+      << syncable::ModelTypeToString(type_) << " Sync Error: " << message_;
+}