commit b2d0677c761fdaa7a6d72b61e8a1833c9d1cb593
author Ben Chan <benchan@chromium.org> Tue Mar 14 22:03:02 2017
committer chrome-bot <chrome-bot@chromium.org> Thu Mar 16 21:23:51 2017
tree 5ddbe36673e1f851ea1bf259801ec1f635b281b1
parent bc9a6417c047ca110f4d5e5d6c354fe42b7091e3

shill: populate a missing error in Manager::ConfigureServiceForProfile

When Manager::ConfigureServiceForProfile() creates and configures a
temporary service, it decides not to return the unusable service but
forgets to populate an error, which will eventually trigger a assertion
in ChromeosManagerDBusAdaptor::ConfigureServiceForProfile(). This CL
ensures that Manager::ConfigureServiceForProfile populates an error
under such condition.

BUG=chromium:701315
TEST=Run unit tests.

Change-Id: Id990de604982cffcc68dff2899ac9859935625bb
Reviewed-on: https://chromium-review.googlesource.com/455256
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>

manager.cc

diff --git a/manager.cc b/manager.cc
index 6fe0d05..df342e9 100644
--- a/manager.cc
+++ b/manager.cc
@@ -2573,9 +2573,16 @@
 
   SetupServiceInProfile(service, profile, args, error);
 
-  // Although we have succeeded, this service will not exist, so its
-  // path is of no use to the caller.
+  // If we encountered an error when configuring the temporary service, we
+  // report the error as it is. Otherwise, we still need to report an error as
+  // the temporary service won't be usable by the caller.
   DCHECK(service->HasOneRef());
+  if (error->IsSuccess()) {
+    Error::PopulateAndLog(FROM_HERE,
+                          error,
+                          Error::kNotFound,
+                          "Temporary service configured but not usable");
+  }
   return nullptr;
 }
 

manager_unittest.cc

diff --git a/manager_unittest.cc b/manager_unittest.cc
index 255dd04..2ff18eb 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -2386,7 +2386,9 @@
   Error error;
   ServiceRefPtr service =
       manager()->ConfigureServiceForProfile(kProfileName0, args, &error);
-  EXPECT_TRUE(error.IsSuccess());
+  EXPECT_FALSE(error.IsSuccess());
+  EXPECT_EQ(Error::kNotFound, error.type());
+  EXPECT_EQ("Temporary service configured but not usable", error.message());
   EXPECT_EQ(nullptr, service.get());
   EXPECT_EQ(profile1.get(), matching_service->profile().get());
 }