[GCM] Fixing the initialization of last checkin/fetch time if they are

google_apis/gcm/engine/gcm_store_impl.cc

 // Copyright 2014 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 "google_apis/gcm/engine/gcm_store_impl.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ skipping 755 matching lines @@
     base::Time* last_checkin_time,
     std::set<std::string>* accounts) {
   leveldb::ReadOptions read_options;
   read_options.verify_checksums = true;
 
   std::string result;
   leveldb::Status s = db_->Get(read_options,
                                MakeSlice(kLastCheckinTimeKey),
                                &result);
   int64 time_internal = 0LL;
-  if (s.ok() && !base::StringToInt64(result, &time_internal))
+  if (s.ok() && !base::StringToInt64(result, &time_internal)) {
     LOG(ERROR) << "Failed to restore last checkin time. Using default = 0.";
+    time_internal = 0LL;

[miu, 2015/01/29 02:56:15]

Should this method return false here instead?

[fgorski, 2015/01/29 22:48:44]

This is not as severe as other problems we might be hitting in this file. I'll
work out the recovery strategy for this file being corrupted, but there is no
need to fail here.

+  }
 
   // In case we cannot read last checkin time, we default it to 0, as we don't
   // want that situation to cause the whole load to fail.
   *last_checkin_time = base::Time::FromInternalValue(time_internal);
 
   accounts->clear();
   s = db_->Get(read_options, MakeSlice(kLastCheckinAccountsKey), &result);
   if (!s.ok())
     DVLOG(1) << "No accounts where stored during last run.";
 
@@ skipping 57 matching lines @@
 
 bool GCMStoreImpl::Backend::LoadLastTokenFetchTime(
     base::Time* last_token_fetch_time) {
   leveldb::ReadOptions read_options;
   read_options.verify_checksums = true;
 
   std::string result;
   leveldb::Status s =
       db_->Get(read_options, MakeSlice(kLastTokenFetchTimeKey), &result);
   int64 time_internal = 0LL;
-  if (s.ok() && !base::StringToInt64(result, &time_internal))
-    LOG(ERROR) << "Failed to restore last checkin time. Using default = 0.";
+  if (s.ok() && !base::StringToInt64(result, &time_internal)) {
+    LOG(ERROR) <<
+        "Failed to restore last token fetching time. Using default = 0.";
+    time_internal = 0LL;

[miu, 2015/01/29 02:56:16]

ditto: Should this method return false here instead?

[fgorski, 2015/01/29 22:48:44]

Same here.

+  }
 
   // In case we cannot read last token fetching time, we default it to 0.
   *last_token_fetch_time = base::Time::FromInternalValue(time_internal);
 
   return true;
 }
 
 GCMStoreImpl::GCMStoreImpl(
     const base::FilePath& path,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner,
@@ skipping 270 matching lines @@
            removed_message_counts.begin();
        iter != removed_message_counts.end(); ++iter) {
     DCHECK_NE(app_message_counts_.count(iter->first), 0U);
     app_message_counts_[iter->first] -= iter->second;
     DCHECK_GE(app_message_counts_[iter->first], 0);
   }
   callback.Run(true);
 }
 
 }  // namespace gcm