Various supervised user password fixes - 2

chrome/browser/chromeos/login/managed/supervised_user_authentication.cc

Index: chrome/browser/chromeos/login/managed/supervised_user_authentication.cc
diff --git a/chrome/browser/chromeos/login/managed/supervised_user_authentication.cc b/chrome/browser/chromeos/login/managed/supervised_user_authentication.cc
index a9b92cbd1a4e935f71ec850ad88c62432e12a55f..3bd08f00fa0422bc0469323953d627f7ff111e3a 100644
--- a/chrome/browser/chromeos/login/managed/supervised_user_authentication.cc
+++ b/chrome/browser/chromeos/login/managed/supervised_user_authentication.cc
@@ -102,17 +102,22 @@ std::string BuildPasswordSignature(const std::string& password,
 base::DictionaryValue* LoadPasswordData(base::FilePath profile_dir) {
   JSONFileValueSerializer serializer(profile_dir.Append(kPasswordUpdateFile));
   std::string error_message;
-  int error_code;
+  int error_code = JSONFileValueSerializer::JSON_NO_ERROR;
   scoped_ptr<base::Value> value(
       serializer.Deserialize(&error_code, &error_message));
   if (JSONFileValueSerializer::JSON_NO_ERROR != error_code) {
+    LOG(ERROR) << "Could not deserialize password data, error = " << error_code
+               << " / " << error_message;
     return NULL;
   }
   base::DictionaryValue* result;
   if (!value->GetAsDictionary(&result)) {
+    NOTREACHED();

[Bernhard Bauer, 2014/04/02 14:05:28]

In general, having a NOTREACHED() is an indication that you are handling a
DCHECK failure, which you're not supposed to do (see
http://dev.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-,
which I also mentioned in your last CL).

In this particular case, you log an error instead of DCHECKing if the file can't
be deserialized at all above, and you also handle a NULL return value in the
caller, so you could just remove the NOTREACHED. If you want to, you could add a
log statement instead, if not, you could simply initialize |result| to NULL and
return it unconditionally (if the value is not a dictionary, |result| will stay
NULL, which is exactly what you want to return).

[Denis Kuznetsov (DE-MUC), 2014/04/02 14:48:51]

Done.

     return NULL;
   }
-  value.Pass();
+  // Ignore unused result warning.

[Nikita (slow), 2014/04/02 14:18:20]

nit: Add empty line before, place comment into {}
or just remove comment and use ignore_result() as Bernhard is suggesting.

[Denis Kuznetsov (DE-MUC), 2014/04/02 14:48:51]

Done.

+  if (value.release()) {

[Bernhard Bauer, 2014/04/02 14:05:28]

base/macros.h has ignore_result().

[Denis Kuznetsov (DE-MUC), 2014/04/02 14:48:51]

Done.

+  }
   return result;
 }
 
@@ -213,6 +218,8 @@ bool SupervisedUserAuthentication::FillDataForNewUser(
         BuildPasswordSignature(salted_password, revision, base64_signature_key);
     password_data->SetStringWithoutPathExpansion(kEncryptedPassword,
                                                  salted_password);
+    password_data->SetStringWithoutPathExpansion(kPasswordSignature,
+                                                 base64_signature);
 
     extra_data->SetStringWithoutPathExpansion(kPasswordEncryptionKey,
                                               BuildRawHMACKey());
@@ -266,7 +273,6 @@ SupervisedUserAuthentication::GetPasswordSchema(
 bool SupervisedUserAuthentication::NeedPasswordChange(
     const std::string& user_id,
     const base::DictionaryValue* password_data) {
-
   base::DictionaryValue local;
   owner_->GetPasswordInformation(user_id, &local);
   int local_schema = SCHEMA_PLAIN;
@@ -279,6 +285,7 @@ bool SupervisedUserAuthentication::NeedPasswordChange(
                                                 &updated_schema);
   password_data->GetIntegerWithoutPathExpansion(kPasswordRevision,
                                                 &updated_revision);
+  LOG(ERROR) << "-----5";

[Bernhard Bauer, 2014/04/02 14:05:28]

Please remove.

[Denis Kuznetsov (DE-MUC), 2014/04/02 14:48:51]

Done.

   if (updated_schema > local_schema)
     return true;
   DCHECK_EQ(updated_schema, local_schema);
@@ -333,11 +340,11 @@ bool SupervisedUserAuthentication::HasIncompleteKey(
   return incomplete_key;
 }
 
-void SupervisedUserAuthentication::MarkKeyIncomplete(
-    const std::string& user_id) {
+void SupervisedUserAuthentication::MarkKeyIncomplete(const std::string& user_id,
+                                                     bool incomplete) {
   base::DictionaryValue holder;
   owner_->GetPasswordInformation(user_id, &holder);
-  holder.SetBoolean(kHasIncompleteKey, true);
+  holder.SetBoolean(kHasIncompleteKey, incomplete);
   owner_->SetPasswordInformation(user_id, &holder);
 }