Show not uploaded crashes for Android.

chrome/browser/crash_upload_list/crash_upload_list_android.cc

+// Copyright 2016 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/crash_upload_list/crash_upload_list_android.h"
+
+#include <regex>
+
+#include "base/files/file.h"
+#include "base/files/file_enumerator.h"
+#include "base/files/file_util.h"
+#include "base/threading/sequenced_worker_pool.h"
+
+CrashUploadListAndroid::CrashUploadListAndroid(
+    Delegate* delegate,
+    const base::FilePath& upload_log_path,
+    const scoped_refptr<base::SequencedWorkerPool>& worker_pool)
+    : CrashUploadList(delegate, upload_log_path, worker_pool) {}
+
+CrashUploadListAndroid::~CrashUploadListAndroid() {}
+
+void CrashUploadListAndroid::LoadUploadList(
+    std::vector<UploadList::UploadInfo>* uploads) {
+  // This will load list of successfully uploaded logs.

[Ilya Sherman, 2016/09/01 20:49:31]

nit: s/will load list/will load the list

[gayane -on leave until 09-2017, 2016/09/01 22:17:59]

Done.

+  CrashUploadList::LoadUploadList(uploads);
+
+  LoadUnsuccessfulUploadList(uploads);
+}
+
+void CrashUploadListAndroid::LoadUnsuccessfulUploadList(
+    std::vector<UploadInfo>* uploads) {
+  std::string pattern = "\\.dmp|\\.skipped";

[Ilya Sherman, 2016/09/01 20:49:31]

Hmm, I think it should be appropriate to only scan for ".skipped" -- I *think*
".dmp" means the minidump hasn't been properly processed yet.  Are you thinking
of any cases that I might be overlooking?

[gayane -on leave until 09-2017, 2016/09/01 22:17:59]

I was thinking about the case when crash dump gets attempted but not uploaded
successfully. The extension then is something like .dmp0.try1 

[Ilya Sherman, 2016/09/01 22:43:04]

Ah, good call!  Okay =)

+  std::regex re(pattern);
+
+  base::FileEnumerator files(upload_log_path().DirName(), false,
+                             base::FileEnumerator::FILES);
+  base::File::Info info;

[Ilya Sherman, 2016/09/01 20:49:31]

nit: Could you please declare this closer to where it is used?

[gayane -on leave until 09-2017, 2016/09/01 22:17:58]

Done.

+  for (base::FilePath file = files.Next(); !file.empty(); file = files.Next()) {
+    if (std::regex_search(file.value(), re)) {

[Ilya Sherman, 2016/09/01 20:49:31]

I think it would be simpler to just have a string search.  Even if you keep both
suffixes, I think two calls to find() are pretty simple and at least as
efficient as a regex.

[Ilya Sherman, 2016/09/01 20:49:31]

nit: Mebbe reverse the condition and use a continue stmt to reduce the level of
indentation?

[gayane -on leave until 09-2017, 2016/09/01 22:17:59]

Done.

[gayane -on leave until 09-2017, 2016/09/01 22:17:59]

Done.

+
+      file = file.BaseName();
+      while (file != file.RemoveExtension())
+        file = file.RemoveExtension();
+
+      // Id is the last part of the file name.

[Ilya Sherman, 2016/09/01 20:49:31]

nit: It might help to include a sample file name in this comment.

[Ilya Sherman, 2016/09/01 20:49:31]

nit: s/Id/ID

[gayane -on leave until 09-2017, 2016/09/01 22:17:58]

Done.

[gayane -on leave until 09-2017, 2016/09/01 22:17:58]

Done.

+      std::string id = file.value();
+      id = id.substr(id.find_last_of("-") + 1);

[Ilya Sherman, 2016/09/01 20:49:31]

nit: Is it worth adding a DCHECK to verify that "-" exists in the filename?

[gayane -on leave until 09-2017, 2016/09/01 22:17:58]

I added a condition to just ignore files that will not have "-"

+
+      base::GetFileInfo(file, &info);
+      UploadList::UploadInfo upload("", base::Time(), id, info.creation_time,

[Ilya Sherman, 2016/09/01 20:49:31]

nit: Prefer std::string() to ""

[gayane -on leave until 09-2017, 2016/09/01 22:17:58]

Done.

+                                    UploadList::UploadInfo::State::NotUploaded);
+      uploads->push_back(upload);
+    }
+  }
+}