Segregate UMA metrics for production scenarios from test scenarios.

Currently we separate the UMA metrics only by one category: whether the
device is in dev mode or not. In addition, we need to exclude the noise
from these two categories:
1. Most of our testing on MP-signed images which are performed
with autest.
2. All our hwlab tests run in non-dev mode but they use dev-signed images
with dev-firmware keys.

So this CL defines additional bit fields to represent these states and
if any of these three flags are set, the UMA metric is sent to a
DevModeErrorCodes bucket. Thus the NormalErrorCodes bucket will have only
the production errors and thus we can monitor more effectively.

BUG=chromium-os:37613
TEST=Updated unit tests, ran on ZGB for all scenarios.
Change-Id: Id9cce33f09d1cc50cb15e67c731f7548940cbc24
Reviewed-on: https://gerrit.chromium.org/gerrit/41103
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>

23 files changed, 413 insertions, 149 deletions

diff --git a/SConstruct b/SConstruct
index 47df390a..cbdf9deb 100644
--- a/SConstruct
+++ b/SConstruct
@@ -314,6 +314,7 @@ unittest_sources = Split("""action_unittest.cc
                             http_fetcher_unittest.cc
                             metadata_unittest.cc
                             mock_http_fetcher.cc
+                            mock_system_state.cc
                             omaha_hash_calculator_unittest.cc
                             omaha_request_action_unittest.cc
                             omaha_request_params_unittest.cc
diff --git a/action_processor.h b/action_processor.h
index 7c425dea..58616b5c 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -92,13 +92,26 @@ enum ActionExitCode {
   // TODO(jaysri): Move out all the bit masks into separate constants
   // outside the enum as part of fixing bug 34369.
   // Bit flags. Remember to update the mask below for new bits.
-  kActionCodeResumedFlag = 1 << 30,  // Set if resuming an interruped update.
-  kActionCodeBootModeFlag = 1 << 31,  // Set if boot mode not normal.
 
-  // Mask to be used to extract the actual code without the higher-order
-  // bit flags (for reporting to UMA which requires small contiguous
-  // enum values)
-  kActualCodeMask = ~(kActionCodeResumedFlag | kActionCodeBootModeFlag)
+  // Set if boot mode not normal.
+  kActionCodeDevModeFlag        = 1 << 31,
+
+  // Set if resuming an interruped update.
+  kActionCodeResumedFlag         = 1 << 30,
+
+  // Set if using a dev/test image as opposed to an MP-signed image.
+  kActionCodeTestImageFlag       = 1 << 29,
+
+  // Set if using devserver or Omaha sandbox (using crosh autest).
+  kActionCodeTestOmahaUrlFlag    = 1 << 28,
+
+  // Mask that indicates bit positions that are used to indicate special flags
+  // that are embedded in the error code to provide additional context about
+  // the system in which the error was encountered.
+  kSpecialFlags = (kActionCodeDevModeFlag |
+                   kActionCodeResumedFlag |
+                   kActionCodeTestImageFlag |
+                   kActionCodeTestOmahaUrlFlag)
 };
 
 class AbstractAction;
diff --git a/delta_performer.cc b/delta_performer.cc
index 3dbfbc08..3f549cdd 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -23,6 +23,7 @@
 #include "update_engine/extent_writer.h"
 #include "update_engine/graph_types.h"
 #include "update_engine/payload_signer.h"
+#include "update_engine/payload_state_interface.h"
 #include "update_engine/prefs_interface.h"
 #include "update_engine/subprocess.h"
 #include "update_engine/terminator.h"
@@ -1118,9 +1119,7 @@ bool DeltaPerformer::PrimeUpdateState() {
 }
 
 void DeltaPerformer::SendUmaStat(ActionExitCode code) {
-  if (system_state_) {
-    utils::SendErrorCodeToUma(system_state_->metrics_lib(), code);
-  }
+  utils::SendErrorCodeToUma(system_state_, code);
 }
 
 }  // namespace chromeos_update_engine
diff --git a/main.cc b/main.cc
index c9562788..cd519959 100644
--- a/main.cc
+++ b/main.cc
@@ -21,11 +21,11 @@
 #include "update_engine/dbus_constants.h"
 #include "update_engine/dbus_interface.h"
 #include "update_engine/dbus_service.h"
+#include "update_engine/real_system_state.h"
 #include "update_engine/subprocess.h"
 #include "update_engine/terminator.h"
 #include "update_engine/update_attempter.h"
 #include "update_engine/update_check_scheduler.h"
-#include "update_engine/system_state.h"
 #include "update_engine/utils.h"
 
 extern "C" {
@@ -171,6 +171,9 @@ int main(int argc, char** argv) {
   // protocol for testing of MP-signed images (chromium-os:25400).
   LOG_IF(ERROR, !real_system_state.Initialize(false))
       << "Failed to initialize system state.";
+  chromeos_update_engine::UpdateAttempter *update_attempter =
+      real_system_state.update_attempter();
+  CHECK(update_attempter);
 
   // Sets static members for the certificate checker.
   chromeos_update_engine::CertificateChecker::set_system_state(
@@ -179,40 +182,35 @@ int main(int argc, char** argv) {
   chromeos_update_engine::CertificateChecker::set_openssl_wrapper(
       &openssl_wrapper);
 
-  // Create the update attempter:
-  chromeos_update_engine::ConcreteDbusGlib dbus;
-  chromeos_update_engine::UpdateAttempter update_attempter(&real_system_state,
-                                                           &dbus);
-
   // Create the dbus service object:
   dbus_g_object_type_install_info(UPDATE_ENGINE_TYPE_SERVICE,
                                   &dbus_glib_update_engine_service_object_info);
   UpdateEngineService* service =
       UPDATE_ENGINE_SERVICE(g_object_new(UPDATE_ENGINE_TYPE_SERVICE, NULL));
-  service->update_attempter_ = &update_attempter;
-  update_attempter.set_dbus_service(service);
+  service->update_attempter_ = update_attempter;
+  update_attempter->set_dbus_service(service);
   chromeos_update_engine::SetupDbusService(service);
 
   // Schedule periodic update checks.
-  chromeos_update_engine::UpdateCheckScheduler scheduler(&update_attempter,
+  chromeos_update_engine::UpdateCheckScheduler scheduler(update_attempter,
                                                          &real_system_state);
   scheduler.Run();
 
   // Update boot flags after 45 seconds.
   g_timeout_add_seconds(45,
                         &chromeos_update_engine::UpdateBootFlags,
-                        &update_attempter);
+                        update_attempter);
 
   // Broadcast the update engine status on startup to ensure consistent system
   // state on crashes.
-  g_idle_add(&chromeos_update_engine::BroadcastStatus, &update_attempter);
+  g_idle_add(&chromeos_update_engine::BroadcastStatus, update_attempter);
 
   // Run the main loop until exit time:
   g_main_loop_run(loop);
 
   // Cleanup:
   g_main_loop_unref(loop);
-  update_attempter.set_dbus_service(NULL);
+  update_attempter->set_dbus_service(NULL);
   g_object_unref(G_OBJECT(service));
 
   LOG(INFO) << "Chrome OS Update Engine terminating";
diff --git a/mock_system_state.cc b/mock_system_state.cc
new file mode 100644
index 00000000..13017b92
--- /dev/null
+++ b/mock_system_state.cc
@@ -0,0 +1,28 @@
+// Copyright (c) 2012 The Chromium OS 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 "update_engine/mock_system_state.h"
+#include "update_engine/update_attempter_mock.h"
+
+namespace chromeos_update_engine {
+
+// Mock the SystemStateInterface so that we could lie that
+// OOBE is completed even when there's no such marker file, etc.
+MockSystemState::MockSystemState() : prefs_(&mock_prefs_) {
+  mock_payload_state_.Initialize(&mock_prefs_);
+  mock_gpio_handler_ = new testing::NiceMock<MockGpioHandler>();
+  mock_update_attempter_ = new testing::NiceMock<UpdateAttempterMock>(
+      this, &dbus_);
+}
+
+MockSystemState::~MockSystemState() {
+  delete mock_gpio_handler_;
+}
+
+UpdateAttempter* MockSystemState::update_attempter() {
+  return mock_update_attempter_;
+}
+
+} // namespeace chromeos_update_engine
+
diff --git a/mock_system_state.h b/mock_system_state.h
index 2b17ce45..956088b7 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -10,6 +10,7 @@
 #include <metrics/metrics_library_mock.h>
 #include <policy/mock_device_policy.h>
 
+#include "update_engine/mock_dbus_interface.h"
 #include "update_engine/mock_gpio_handler.h"
 #include "update_engine/mock_payload_state.h"
 #include "update_engine/prefs_mock.h"
@@ -17,17 +18,15 @@
 
 namespace chromeos_update_engine {
 
+class UpdateAttempterMock;
+
 // Mock the SystemStateInterface so that we could lie that
 // OOBE is completed even when there's no such marker file, etc.
 class MockSystemState : public SystemState {
  public:
-  inline MockSystemState() : prefs_(&mock_prefs_) {
-    mock_payload_state_.Initialize(&mock_prefs_);
-    mock_gpio_handler_ = new testing::NiceMock<MockGpioHandler>();
-  }
-  inline virtual ~MockSystemState() {
-    delete mock_gpio_handler_;
-  }
+  MockSystemState();
+
+  virtual ~MockSystemState();
 
   MOCK_METHOD0(IsOOBEComplete, bool());
   MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
@@ -53,6 +52,8 @@ class MockSystemState : public SystemState {
     return mock_gpio_handler_;
   }
 
+  virtual UpdateAttempter* update_attempter();
+
   // MockSystemState-specific public method.
   inline void set_connection_manager(ConnectionManager* connection_manager) {
     connection_manager_ = connection_manager;
@@ -76,10 +77,13 @@ class MockSystemState : public SystemState {
 
  private:
   // These are Mock objects or objects we own.
-  MetricsLibraryMock mock_metrics_lib_;
+  testing::NiceMock<MetricsLibraryMock> mock_metrics_lib_;
   testing::NiceMock<PrefsMock> mock_prefs_;
   testing::NiceMock<MockPayloadState> mock_payload_state_;
   testing::NiceMock<MockGpioHandler>* mock_gpio_handler_;
+  testing::NiceMock<UpdateAttempterMock>* mock_update_attempter_;
+
+  MockDbusGlib dbus_;
 
   // These are pointers to objects which caller can override.
   PrefsInterface* prefs_;
diff --git a/omaha_hash_calculator.cc b/omaha_hash_calculator.cc
index 33d8a9ec..245706dd 100644
--- a/omaha_hash_calculator.cc
+++ b/omaha_hash_calculator.cc
@@ -176,7 +176,7 @@ bool OmahaHashCalculator::Finalize() {
                    &ctx_) == 1);
 
   // Convert raw_hash_ to base64 encoding and store it in hash_.
-  return Base64Encode(&raw_hash_[0], raw_hash_.size(), &hash_);;
+  return Base64Encode(&raw_hash_[0], raw_hash_.size(), &hash_);
 }
 
 bool OmahaHashCalculator::RawHashOfBytes(const char* data,
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 679d3265..edfea539 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -20,6 +20,7 @@
 
 #include "update_engine/action_pipe.h"
 #include "update_engine/omaha_request_params.h"
+#include "update_engine/payload_state_interface.h"
 #include "update_engine/prefs_interface.h"
 #include "update_engine/utils.h"
 
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index e240c861..d8572976 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -30,7 +30,7 @@ const char* const OmahaRequestParams::kAppId(
     "{87efface-864d-49a5-9bb3-4b050a7c227a}");
 const char* const OmahaRequestParams::kOsPlatform("Chrome OS");
 const char* const OmahaRequestParams::kOsVersion("Indy");
-const char* const OmahaRequestParams::kUpdateUrl(
+const char* const kProductionOmahaUrl(
     "https://tools.google.com/service/update2");
 
 const char OmahaRequestParams::kUpdateTrackKey[] = "CHROMEOS_RELEASE_TRACK";
@@ -95,7 +95,7 @@ bool OmahaRequestDeviceParams::Init(const std::string& in_app_version,
 
   update_url = in_update_url.empty() ?
       GetLsbValue("CHROMEOS_AUSERVER",
-                  OmahaRequestParams::kUpdateUrl,
+                  kProductionOmahaUrl,
                   NULL,
                   stateful_override) :
       in_update_url;
diff --git a/omaha_request_params.h b/omaha_request_params.h
index 74354688..ea8bab09 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -16,6 +16,9 @@
 
 namespace chromeos_update_engine {
 
+// The default "official" Omaha update URL.
+extern const char* const kProductionOmahaUrl;
+
 // This struct encapsulates the data Omaha gets for the request, along with
 // essential state needed for the processing of the request/response.
 // The strings in this struct should not be XML escaped.
diff --git a/omaha_request_params_unittest.cc b/omaha_request_params_unittest.cc
index ecaa682c..89083113 100644
--- a/omaha_request_params_unittest.cc
+++ b/omaha_request_params_unittest.cc
@@ -216,7 +216,7 @@ TEST_F(OmahaRequestDeviceParamsTest, MissingURLTest) {
   EXPECT_EQ("en-US", out.app_lang);
   EXPECT_TRUE(out.delta_okay);
   EXPECT_EQ("footrack", out.app_track);
-  EXPECT_EQ(OmahaRequestParams::kUpdateUrl, out.update_url);
+  EXPECT_EQ(kProductionOmahaUrl, out.update_url);
 }
 
 TEST_F(OmahaRequestDeviceParamsTest, NoDeltasTest) {
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index ab083aad..24b8dcb9 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -10,6 +10,7 @@
 #include "base/string_util.h"
 
 #include "update_engine/delta_performer.h"
+#include "update_engine/payload_state_interface.h"
 #include "update_engine/prefs_interface.h"
 #include "update_engine/utils.h"
 
diff --git a/payload_state.cc b/payload_state.cc
index 26a3a3fa..9c4a7e45 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -94,7 +94,8 @@ void PayloadState::DownloadProgress(size_t count) {
 
 void PayloadState::UpdateFailed(ActionExitCode error) {
   ActionExitCode base_error = utils::GetBaseErrorCode(error);
-  LOG(INFO) << "Updating payload state for error code: " << base_error;
+  LOG(INFO) << "Updating payload state for error code: " << base_error
+            << " (" << utils::CodeToString(base_error) << ")";
 
   if (GetNumUrls() == 0) {
     // This means we got this error even before we got a valid Omaha response.
@@ -177,9 +178,11 @@ void PayloadState::UpdateFailed(ActionExitCode error) {
     case kActionCodeSetBootableFlagError:               // unused
     case kActionCodeUmaReportedMax:                     // not an error code
     case kActionCodeOmahaRequestHTTPResponseBase:       // aggregated already
+    case kActionCodeDevModeFlag:                       // not an error code
     case kActionCodeResumedFlag:                        // not an error code
-    case kActionCodeBootModeFlag:                       // not an error code
-    case kActualCodeMask:                               // not an error code
+    case kActionCodeTestImageFlag:                      // not an error code
+    case kActionCodeTestOmahaUrlFlag:                   // not an error code
+    case kSpecialFlags:                                 // not an error code
       // These shouldn't happen. Enumerating these  explicitly here so that we
       // can let the compiler warn about new error codes that are added to
       // action_processor.h but not added here.
diff --git a/real_system_state.h b/real_system_state.h
new file mode 100644
index 00000000..871a942d
--- /dev/null
+++ b/real_system_state.h
@@ -0,0 +1,99 @@
+// Copyright (c) 2013 The Chromium OS 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 CHROMEOS_PLATFORM_UPDATE_ENGINE_REAL_SYSTEM_STATE_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_REAL_SYSTEM_STATE_H_
+
+#include <update_engine/system_state.h>
+
+#include <update_engine/connection_manager.h>
+#include <update_engine/gpio_handler.h>
+#include <update_engine/payload_state.h>
+#include <update_engine/prefs.h>
+#include <update_engine/update_attempter.h>
+
+namespace chromeos_update_engine {
+
+// A real implementation of the SystemStateInterface which is
+// used by the actual product code.
+class RealSystemState : public SystemState {
+public:
+  // Constructors and destructors.
+  RealSystemState();
+  virtual ~RealSystemState() {}
+
+  virtual bool IsOOBEComplete();
+
+  virtual inline void set_device_policy(
+      const policy::DevicePolicy* device_policy) {
+    device_policy_ = device_policy;
+  }
+
+  virtual inline const policy::DevicePolicy* device_policy() const {
+    return device_policy_;
+  }
+
+  virtual inline ConnectionManager* connection_manager() {
+    return &connection_manager_;
+  }
+
+  virtual inline MetricsLibraryInterface* metrics_lib() {
+    return &metrics_lib_;
+  }
+
+  virtual inline PrefsInterface* prefs() {
+    return &prefs_;
+  }
+
+  virtual inline PayloadStateInterface* payload_state() {
+    return &payload_state_;
+  }
+
+  virtual inline GpioHandler* gpio_handler() const {
+    return gpio_handler_.get();
+  }
+
+  virtual inline UpdateAttempter* update_attempter() {
+    return update_attempter_.get();
+  }
+
+  // Initializes this concrete object. Other methods should be invoked only
+  // if the object has been initialized successfully.
+  bool Initialize(bool enable_gpio);
+
+private:
+  // The latest device policy object from the policy provider.
+  const policy::DevicePolicy* device_policy_;
+
+  // The connection manager object that makes download
+  // decisions depending on the current type of connection.
+  ConnectionManager connection_manager_;
+
+  // The Metrics Library interface for reporting UMA stats.
+  MetricsLibrary metrics_lib_;
+
+  // Interface for persisted store.
+  Prefs prefs_;
+
+  // All state pertaining to payload state such as
+  // response, URL, backoff states.
+  PayloadState payload_state_;
+
+  // Pointer to a GPIO handler and other needed modules (note that the order of
+  // declaration significant for destruction, as the latter depends on the
+  // former).
+  scoped_ptr<UdevInterface> udev_iface_;
+  scoped_ptr<FileDescriptor> file_descriptor_;
+  scoped_ptr<GpioHandler> gpio_handler_;
+
+  // The dbus object used to initialize the update attempter.
+  ConcreteDbusGlib dbus_;
+
+  // Pointer to the update attempter object.
+  scoped_ptr<UpdateAttempter> update_attempter_;
+};
+
+}  // namespace chromeos_update_engine
+
+#endif  // CHROMEOS_PLATFORM_UPDATE_ENGINE_REAL_SYSTEM_STATE_H_
diff --git a/system_state.cc b/system_state.cc
index fc3ba57b..42737bbb 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -4,7 +4,7 @@
 
 #include <base/file_util.h>
 
-#include "update_engine/system_state.h"
+#include "update_engine/real_system_state.h"
 
 namespace chromeos_update_engine {
 
@@ -42,6 +42,9 @@ bool RealSystemState::Initialize(bool enable_gpio) {
     gpio_handler_.reset(new NoopGpioHandler(false));
   }
 
+  // Create the update attempter.
+  update_attempter_.reset(new UpdateAttempter(this, &dbus_));
+
   // All is well. Initialization successful.
   return true;
 }
diff --git a/system_state.h b/system_state.h
index 772205c5..20e11a7f 100644
--- a/system_state.h
+++ b/system_state.h
@@ -9,13 +9,17 @@
 #include <policy/device_policy.h>
 #include <policy/libpolicy.h>
 
-#include <update_engine/connection_manager.h>
-#include <update_engine/gpio_handler.h>
-#include <update_engine/payload_state.h>
-#include <update_engine/prefs.h>
-
 namespace chromeos_update_engine {
 
+// SystemState is the root class within the update engine. So we should avoid
+// any circular references in header file inclusion. Hence forward-declaring
+// the required classes.
+class ConnectionManager;
+class PrefsInterface;
+class PayloadStateInterface;
+class GpioHandler;
+class UpdateAttempter;
+
 // An interface to global system context, including platform resources,
 // the current state of the system, high-level objects whose lifetime is same
 // as main, system interfaces, etc.
@@ -50,78 +54,11 @@ class SystemState {
 
   // Returns a pointer to the GPIO handler.
   virtual GpioHandler* gpio_handler() const = 0;
-};
-
-// A real implementation of the SystemStateInterface which is
-// used by the actual product code.
-class RealSystemState : public SystemState {
-public:
-  // Constructors and destructors.
-  RealSystemState();
-  virtual ~RealSystemState() {}
-
-  virtual bool IsOOBEComplete();
-
-  virtual inline void set_device_policy(
-      const policy::DevicePolicy* device_policy) {
-    device_policy_ = device_policy;
-  }
-
-  virtual inline const policy::DevicePolicy* device_policy() const {
-    return device_policy_;
-  }
-
-  virtual inline ConnectionManager* connection_manager() {
-    return &connection_manager_;
-  }
-
-  virtual inline MetricsLibraryInterface* metrics_lib() {
-    return &metrics_lib_;
-  }
-
-  virtual inline PrefsInterface* prefs() {
-    return &prefs_;
-  }
-
-  virtual inline PayloadStateInterface* payload_state() {
-    return &payload_state_;
-  }
-
-  // Returns a pointer to the GPIO handler.
-  virtual inline GpioHandler* gpio_handler() const {
-    return gpio_handler_.get();
-  }
-
-  // Initializs this concrete object. Other methods should be invoked only
-  // if the object has been initialized successfully.
-  bool Initialize(bool enable_gpio);
-
-private:
-  // The latest device policy object from the policy provider.
-  const policy::DevicePolicy* device_policy_;
-
-  // The connection manager object that makes download
-  // decisions depending on the current type of connection.
-  ConnectionManager connection_manager_;
-
-  // The Metrics Library interface for reporting UMA stats.
-  MetricsLibrary metrics_lib_;
-
-  // Interface for persisted store.
-  Prefs prefs_;
-
-  // All state pertaining to payload state such as
-  // response, URL, backoff states.
-  PayloadState payload_state_;
 
-  // Pointer to a GPIO handler and other needed modules (note that the order of
-  // declaration significant for destruction, as the latter depends on the
-  // former).
-  scoped_ptr<UdevInterface> udev_iface_;
-  scoped_ptr<FileDescriptor> file_descriptor_;
-  scoped_ptr<GpioHandler> gpio_handler_;
+  // Returns a pointer to the update attempter object.
+  virtual UpdateAttempter* update_attempter() = 0;
 };
 
 }  // namespace chromeos_update_engine
 
-#endif  // CHROMEOS_PLATFORM_UPDATE_ENGINE_UTILS_H_
+#endif  // CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
diff --git a/update_attempter.cc b/update_attempter.cc
index 3aa97499..569b6263 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -25,11 +25,13 @@
 #include "update_engine/dbus_service.h"
 #include "update_engine/download_action.h"
 #include "update_engine/filesystem_copier_action.h"
+#include "update_engine/gpio_handler.h"
 #include "update_engine/libcurl_http_fetcher.h"
 #include "update_engine/multi_range_http_fetcher.h"
 #include "update_engine/omaha_request_action.h"
 #include "update_engine/omaha_request_params.h"
 #include "update_engine/omaha_response_handler_action.h"
+#include "update_engine/payload_state_interface.h"
 #include "update_engine/postinstall_runner_action.h"
 #include "update_engine/prefs_interface.h"
 #include "update_engine/subprocess.h"
@@ -602,7 +604,7 @@ void UpdateAttempter::ProcessingDone(const ActionProcessor* processor,
 
     // Also report the success code so that the percentiles can be
     // interpreted properly for the remaining error codes in UMA.
-    utils::SendErrorCodeToUma(system_state_->metrics_lib(), code);
+    utils::SendErrorCodeToUma(system_state_, code);
     return;
   }
 
@@ -811,6 +813,25 @@ void UpdateAttempter::BroadcastStatus() {
       new_payload_size_);
 }
 
+uint32_t UpdateAttempter::GetErrorCodeFlags()  {
+  uint32_t flags = 0;
+
+  if (!utils::IsNormalBootMode())
+    flags |= kActionCodeDevModeFlag;
+
+  if (response_handler_action_.get() &&
+      response_handler_action_->install_plan().is_resume)
+    flags |= kActionCodeResumedFlag;
+
+  if (!utils::IsOfficialBuild())
+    flags |= kActionCodeTestImageFlag;
+
+  if (omaha_request_params_.update_url != kProductionOmahaUrl)
+    flags |= kActionCodeTestOmahaUrlFlag;
+
+  return flags;
+}
+
 void UpdateAttempter::SetStatusAndNotify(UpdateStatus status,
                                          UpdateNotice notice) {
   status_ = status;
@@ -847,6 +868,7 @@ void UpdateAttempter::CreatePendingErrorEvent(AbstractAction* action,
   switch (code) {
     case kActionCodeOmahaUpdateIgnoredPerPolicy:
     case kActionCodeOmahaUpdateDeferredPerPolicy:
+    case kActionCodeOmahaUpdateDeferredForBackoff:
       event_result = OmahaEvent::kResultUpdateDeferred;
       break;
     default:
@@ -857,15 +879,8 @@ void UpdateAttempter::CreatePendingErrorEvent(AbstractAction* action,
   code = GetErrorCodeForAction(action, code);
   fake_update_success_ = code == kActionCodePostinstallBootedFromFirmwareB;
 
-  // Apply the bit modifiers to the error code.
-  if (!utils::IsNormalBootMode()) {
-    code = static_cast<ActionExitCode>(code | kActionCodeBootModeFlag);
-  }
-  if (response_handler_action_.get() &&
-      response_handler_action_->install_plan().is_resume) {
-    code = static_cast<ActionExitCode>(code | kActionCodeResumedFlag);
-  }
-
+  // Compute the final error code with all the bit flags to be sent to Omaha.
+  code = static_cast<ActionExitCode>(code | GetErrorCodeFlags());
   error_event_.reset(new OmahaEvent(OmahaEvent::kTypeUpdateComplete,
                                     event_result,
                                     code));
@@ -878,9 +893,11 @@ bool UpdateAttempter::ScheduleErrorEventAction() {
   LOG(ERROR) << "Update failed.";
   system_state_->payload_state()->UpdateFailed(error_event_->error_code);
 
+  // Send it to Uma.
   LOG(INFO) << "Reporting the error event";
-  utils::SendErrorCodeToUma(system_state_->metrics_lib(),
-                            error_event_->error_code);
+  utils::SendErrorCodeToUma(system_state_, error_event_->error_code);
+
+  // Send it to Omaha.
   shared_ptr<OmahaRequestAction> error_event_action(
       new OmahaRequestAction(system_state_,
                              &omaha_request_params_,
diff --git a/update_attempter.h b/update_attempter.h
index ec0567b7..c42715f2 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -154,6 +154,10 @@ class UpdateAttempter : public ActionProcessorDelegate,
   // Broadcasts the current status over D-Bus.
   void BroadcastStatus();
 
+  // Returns the special flags to be added to ActionExitCode values based on the
+  // parameters used in the current update attempt.
+  uint32_t GetErrorCodeFlags();
+
  private:
   // Update server URL for automated lab test.
   static const char* const kTestUpdateUrl;
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 8b47c35a..782277ac 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -13,6 +13,8 @@
 
 namespace chromeos_update_engine {
 
+class MockSystemState;
+
 class UpdateAttempterMock : public UpdateAttempter {
  public:
   explicit UpdateAttempterMock(MockSystemState* mock_system_state,
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 61d9010c..18bcb677 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -64,7 +64,7 @@ class UpdateAttempterTest : public ::testing::Test {
     EXPECT_EQ(0, attempter_.last_checked_time_);
     EXPECT_EQ("0.0.0.0", attempter_.new_version_);
     EXPECT_EQ(0, attempter_.new_payload_size_);
-    processor_ = new ActionProcessorMock();
+    processor_ = new NiceMock<ActionProcessorMock>();
     attempter_.processor_.reset(processor_);  // Transfers ownership.
     prefs_ = mock_system_state_.mock_prefs();
   }
@@ -102,12 +102,12 @@ class UpdateAttempterTest : public ::testing::Test {
   static gboolean StaticNoScatteringDoneDuringManualUpdateTestStart(
       gpointer data);
 
-  MockSystemState mock_system_state_;
-  MockDbusGlib dbus_;
+  NiceMock<MockSystemState> mock_system_state_;
+  NiceMock<MockDbusGlib> dbus_;
   UpdateAttempterUnderTest attempter_;
-  ActionProcessorMock* processor_;
+  NiceMock<ActionProcessorMock>* processor_;
   NiceMock<PrefsMock>* prefs_; // shortcut to mock_system_state_->mock_prefs()
-  MockConnectionManager mock_connection_manager;
+  NiceMock<MockConnectionManager> mock_connection_manager;
   GMainLoop* loop_;
 };
 
@@ -433,7 +433,8 @@ TEST_F(UpdateAttempterTest, CreatePendingErrorEventTest) {
   ASSERT_TRUE(attempter_.error_event_.get() != NULL);
   EXPECT_EQ(OmahaEvent::kTypeUpdateComplete, attempter_.error_event_->type);
   EXPECT_EQ(OmahaEvent::kResultError, attempter_.error_event_->result);
-  EXPECT_EQ(kCode, attempter_.error_event_->error_code);
+  EXPECT_EQ(kCode | kActionCodeTestOmahaUrlFlag,
+            attempter_.error_event_->error_code);
 }
 
 TEST_F(UpdateAttempterTest, CreatePendingErrorEventResumedTest) {
@@ -447,7 +448,7 @@ TEST_F(UpdateAttempterTest, CreatePendingErrorEventResumedTest) {
   ASSERT_TRUE(attempter_.error_event_.get() != NULL);
   EXPECT_EQ(OmahaEvent::kTypeUpdateComplete, attempter_.error_event_->type);
   EXPECT_EQ(OmahaEvent::kResultError, attempter_.error_event_->result);
-  EXPECT_EQ(kCode | kActionCodeResumedFlag,
+  EXPECT_EQ(kCode | kActionCodeResumedFlag | kActionCodeTestOmahaUrlFlag,
             attempter_.error_event_->error_code);
 }
 
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 85e734f7..227816df 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -6,6 +6,7 @@
 
 #include "update_engine/certificate_checker.h"
 #include "update_engine/http_common.h"
+#include "update_engine/gpio_handler.h"
 #include "update_engine/system_state.h"
 #include "update_engine/utils.h"
 
diff --git a/utils.cc b/utils.cc
index e5d7290f..fee34393 100644
--- a/utils.cc
+++ b/utils.cc
@@ -34,6 +34,8 @@
 #include "update_engine/file_writer.h"
 #include "update_engine/omaha_request_params.h"
 #include "update_engine/subprocess.h"
+#include "update_engine/system_state.h"
+#include "update_engine/update_attempter.h"
 
 using base::Time;
 using base::TimeDelta;
@@ -715,8 +717,7 @@ ActionExitCode GetBaseErrorCode(ActionExitCode code) {
   // Ignore the higher order bits in the code by applying the mask as
   // we want the enumerations to be in the small contiguous range
   // with values less than kActionCodeUmaReportedMax.
-  ActionExitCode base_code = static_cast<ActionExitCode>(
-      code & kActualCodeMask);
+  ActionExitCode base_code = static_cast<ActionExitCode>(code & ~kSpecialFlags);
 
   // Make additional adjustments required for UMA and error classification.
   // TODO(jaysri): Move this logic to UeErrorCode.cc when we fix
@@ -724,27 +725,169 @@ ActionExitCode GetBaseErrorCode(ActionExitCode code) {
   if (base_code >= kActionCodeOmahaRequestHTTPResponseBase) {
     // Since we want to keep the enums to a small value, aggregate all HTTP
     // errors into this one bucket for UMA and error classification purposes.
+    LOG(INFO) << "Converting error code " << base_code
+              << " to kActionCodeOmahaErrorInHTTPResponse";
     base_code = kActionCodeOmahaErrorInHTTPResponse;
   }
 
   return base_code;
 }
 
+// Returns a printable version of the various flags denoted in the higher order
+// bits of the given code. Returns an empty string if none of those bits are
+// set.
+string GetFlagNames(uint32_t code) {
+  uint32_t flags = code & kSpecialFlags;
+  string flag_names;
+  string separator = "";
+  for(size_t i = 0; i < sizeof(flags) * 8; i++) {
+    uint32_t flag = flags & (1 << i);
+    if (flag) {
+      flag_names += separator + CodeToString(static_cast<ActionExitCode>(flag));
+      separator = ", ";
+    }
+  }
 
+  return flag_names;
+}
+
+void SendErrorCodeToUma(SystemState* system_state, ActionExitCode code) {
+  if (!system_state)
+    return;
+
+  ActionExitCode uma_error_code = GetBaseErrorCode(code);
+
+  // If the code doesn't have flags computed already, compute them now based on
+  // the state of the current update attempt.
+  uint32_t flags = code & kSpecialFlags;
+  if (!flags)
+    flags = system_state->update_attempter()->GetErrorCodeFlags();
+
+  // Determine the UMA bucket depending on the flags. But, ignore the resumed
+  // flag, as it's perfectly normal for production devices to resume their
+  // downloads and so we want to record those cases also in NormalErrorCodes
+  // bucket.
+  string metric = (flags & ~kActionCodeResumedFlag) ?
+      "Installer.DevModeErrorCodes" : "Installer.NormalErrorCodes";
+
+  LOG(INFO) << "Sending error code " << uma_error_code
+            << " (" << CodeToString(uma_error_code) << ")"
+            << " to UMA metric: " << metric
+            << ". Flags = " << (flags ? GetFlagNames(flags) : "None");
+
+  system_state->metrics_lib()->SendEnumToUMA(metric,
+                                             uma_error_code,
+                                             kActionCodeUmaReportedMax);
+}
+
+string CodeToString(ActionExitCode code) {
+  // If the given code has both parts (i.e. the error code part and the flags
+  // part) then strip off the flags part since the switch statement below
+  // has case statements only for the base error code or a single flag but
+  // doesn't support any combinations of those.
+  if ((code & kSpecialFlags) && (code & ~kSpecialFlags))
+    code = static_cast<ActionExitCode>(code & ~kSpecialFlags);
+  switch (code) {
+    case kActionCodeSuccess: return "kActionCodeSuccess";
+    case kActionCodeError: return "kActionCodeError";
+    case kActionCodeOmahaRequestError: return "kActionCodeOmahaRequestError";
+    case kActionCodeOmahaResponseHandlerError:
+      return "kActionCodeOmahaResponseHandlerError";
+    case kActionCodeFilesystemCopierError:
+      return "kActionCodeFilesystemCopierError";
+    case kActionCodePostinstallRunnerError:
+      return "kActionCodePostinstallRunnerError";
+    case kActionCodeSetBootableFlagError:
+      return "kActionCodeSetBootableFlagError";
+    case kActionCodeInstallDeviceOpenError:
+      return "kActionCodeInstallDeviceOpenError";
+    case kActionCodeKernelDeviceOpenError:
+      return "kActionCodeKernelDeviceOpenError";
+    case kActionCodeDownloadTransferError:
+      return "kActionCodeDownloadTransferError";
+    case kActionCodePayloadHashMismatchError:
+      return "kActionCodePayloadHashMismatchError";
+    case kActionCodePayloadSizeMismatchError:
+      return "kActionCodePayloadSizeMismatchError";
+    case kActionCodeDownloadPayloadVerificationError:
+      return "kActionCodeDownloadPayloadVerificationError";
+    case kActionCodeDownloadNewPartitionInfoError:
+      return "kActionCodeDownloadNewPartitionInfoError";
+    case kActionCodeDownloadWriteError:
+      return "kActionCodeDownloadWriteError";
+    case kActionCodeNewRootfsVerificationError:
+      return "kActionCodeNewRootfsVerificationError";
+    case kActionCodeNewKernelVerificationError:
+      return "kActionCodeNewKernelVerificationError";
+    case kActionCodeSignedDeltaPayloadExpectedError:
+      return "kActionCodeSignedDeltaPayloadExpectedError";
+    case kActionCodeDownloadPayloadPubKeyVerificationError:
+      return "kActionCodeDownloadPayloadPubKeyVerificationError";
+    case kActionCodePostinstallBootedFromFirmwareB:
+      return "kActionCodePostinstallBootedFromFirmwareB";
+    case kActionCodeDownloadStateInitializationError:
+      return "kActionCodeDownloadStateInitializationError";
+    case kActionCodeDownloadInvalidMetadataMagicString:
+      return "kActionCodeDownloadInvalidMetadataMagicString";
+    case kActionCodeDownloadSignatureMissingInManifest:
+      return "kActionCodeDownloadSignatureMissingInManifest";
+    case kActionCodeDownloadManifestParseError:
+      return "kActionCodeDownloadManifestParseError";
+    case kActionCodeDownloadMetadataSignatureError:
+      return "kActionCodeDownloadMetadataSignatureError";
+    case kActionCodeDownloadMetadataSignatureVerificationError:
+      return "kActionCodeDownloadMetadataSignatureVerificationError";
+    case kActionCodeDownloadMetadataSignatureMismatch:
+      return "kActionCodeDownloadMetadataSignatureMismatch";
+    case kActionCodeDownloadOperationHashVerificationError:
+      return "kActionCodeDownloadOperationHashVerificationError";
+    case kActionCodeDownloadOperationExecutionError:
+      return "kActionCodeDownloadOperationExecutionError";
+    case kActionCodeDownloadOperationHashMismatch:
+      return "kActionCodeDownloadOperationHashMismatch";
+    case kActionCodeOmahaRequestEmptyResponseError:
+      return "kActionCodeOmahaRequestEmptyResponseError";
+    case kActionCodeOmahaRequestXMLParseError:
+      return "kActionCodeOmahaRequestXMLParseError";
+    case kActionCodeDownloadInvalidMetadataSize:
+      return "kActionCodeDownloadInvalidMetadataSize";
+    case kActionCodeDownloadInvalidMetadataSignature:
+      return "kActionCodeDownloadInvalidMetadataSignature";
+    case kActionCodeOmahaResponseInvalid:
+      return "kActionCodeOmahaResponseInvalid";
+    case kActionCodeOmahaUpdateIgnoredPerPolicy:
+      return "kActionCodeOmahaUpdateIgnoredPerPolicy";
+    case kActionCodeOmahaUpdateDeferredPerPolicy:
+      return "kActionCodeOmahaUpdateDeferredPerPolicy";
+    case kActionCodeOmahaErrorInHTTPResponse:
+      return "kActionCodeOmahaErrorInHTTPResponse";
+    case kActionCodeDownloadOperationHashMissingError:
+      return "kActionCodeDownloadOperationHashMissingError";
+    case kActionCodeDownloadMetadataSignatureMissingError:
+      return "kActionCodeDownloadMetadataSignatureMissingError";
+    case kActionCodeOmahaUpdateDeferredForBackoff:
+      return "kActionCodeOmahaUpdateDeferredForBackoff";
+    case kActionCodeUmaReportedMax:
+      return "kActionCodeUmaReportedMax";
+    case kActionCodeOmahaRequestHTTPResponseBase:
+      return "kActionCodeOmahaRequestHTTPResponseBase";
+    case kActionCodeResumedFlag:
+      return "Resumed";
+    case kActionCodeDevModeFlag:
+      return "DevMode";
+    case kActionCodeTestImageFlag:
+      return "TestImage";
+    case kActionCodeTestOmahaUrlFlag:
+      return "TestOmahaUrl";
+    case kSpecialFlags:
+      return "kSpecialFlags";
+    // Don't add a default case to let the compiler warn about newly added
+    // error codes which should be added here.
+  }
 
-void SendErrorCodeToUma(MetricsLibraryInterface* metrics_lib,
-                        ActionExitCode code) {
-  string metric = utils::IsNormalBootMode() ? "Installer.NormalErrorCodes" :
-                                              "Installer.DevModeErrorCodes";
-
-  ActionExitCode reported_code = GetBaseErrorCode(code);
-
-  LOG(INFO) << "Sending error code " << reported_code
-            << " to UMA metric: " << metric;
-  metrics_lib->SendEnumToUMA(metric, reported_code, kActionCodeUmaReportedMax);
+  return "Unknown error: " + base::UintToString(static_cast<unsigned>(code));
 }
 
-
 }  // namespace utils
 
 }  // namespace chromeos_update_engine
diff --git a/utils.h b/utils.h
index c2cd21db..6c178537 100644
--- a/utils.h
+++ b/utils.h
@@ -23,6 +23,8 @@
 
 namespace chromeos_update_engine {
 
+class SystemState;
+
 namespace utils {
 
 // Returns true if this is an official Chrome OS build, false otherwise.
@@ -286,11 +288,15 @@ std::string FormatTimeDelta(base::TimeDelta delta);
 // it'll return the same value again.
 ActionExitCode GetBaseErrorCode(ActionExitCode code);
 
-// Sends the error code to the appropriate bucket in UMA using the metrics_lib
-// interface. This method uses GetBaseErrorCode to process the given code and
-// report only the base error code.
-void SendErrorCodeToUma(MetricsLibraryInterface* metrics_lib,
-                        ActionExitCode code);
+// Sends the error code to UMA using the metrics interface object in the given
+// system state. It also uses the system state to determine the right UMA
+// bucket for the error code.
+void SendErrorCodeToUma(SystemState* system_state, ActionExitCode code);
+
+// Returns a string representation of the ActionExitCodes (either the base
+// error codes or the bit flags) for logging purposes.
+std::string CodeToString(ActionExitCode code);
+
 }  // namespace utils