[Offline pages] Adding ArchiveManager to abstract offline archives management

components/offline_pages/archive_manager.h

+// 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_ARCHIVE_MANAGER_H_
+#define COMPONENTS_OFFLINE_PAGES_ARCHIVE_MANAGER_H_
+
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
+
+namespace base {
+class SequencedTaskRunner;
+}  // namespace base
+
+namespace offline_pages {

[jianli, 2016/05/13 23:50:48]

nit: add empty line

[fgorski, 2016/05/16 17:46:51]

Done.

+// Class that helps managing archive files. It will fail any operations on

[jianli, 2016/05/13 23:50:48]

The comment "It will fail any operations on ..." is kind of confusing. How
about:
  // Class that manages all archive files for offline pages. They're all stored
in a specific archive directory.

[fgorski, 2016/05/16 17:46:52]

Done.

+// archives that are not in |archive_dir|.
+class ArchiveManager {
+ public:
+  ArchiveManager(const base::FilePath& archives_dir,
+                 const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+  virtual ~ArchiveManager();
+
+  // Creates archives directory of one does not exist yet;

[jianli, 2016/05/13 23:50:48]

Please also comment above this line that all operations are performed in the
task runner.

[fgorski, 2016/05/16 17:46:51]

Done.

+  virtual void EnsureArchivesDirCreated(const base::Closure& callback);
+
+  // Verifies whether an archive with specified |archive_path| exists.

[jianli, 2016/05/13 23:50:48]

Verifies => Checks

[fgorski, 2016/05/16 17:46:51]

Done.

+  virtual void ArchiveExists(const base::FilePath& archive_path,

[jianli, 2016/05/13 23:50:48]

ExistsArchive

[fgorski, 2016/05/16 17:46:51]

Done.

+                             const base::Callback<void(bool)>& callback);
+
+  // Deletes an archive with specified |archive_path|.
+  virtual void DeleteArchive(const base::FilePath& archive_path,
+                             const base::Callback<void(bool)>& callback);
+
+  // Deletes all archives in |archive_paths|.

[jianli, 2016/05/13 23:50:48]

The comment "all archives" is very confusing. How about updating it to:
  // Deletes multiple archives that are specified in |archive_paths| collection.

[fgorski, 2016/05/16 17:46:51]

Done.

+  virtual void DeleteArchives(const std::vector<base::FilePath>& archive_paths,
+                              const base::Callback<void(bool)>& callback);
+
+ private:
+  // Path under which all of the managed archives should be stored.
+  base::FilePath archives_dir_;
+  // Task runner for running file operations.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+};

[jianli, 2016/05/13 23:50:48]

nit: add empty line

[fgorski, 2016/05/16 17:46:52]

Done.

+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_ARCHIVE_MANAGER_H_