SnapshotController implementation. It will be used in WebContentsObservers for Offline Pages - to d…

components/offline_pages/snapshot_controller.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 "components/offline_pages/snapshot_controller.h"
+
+#include "base/bind.h"
+#include "base/files/file_util.h"

[fgorski, 2016/04/25 04:28:45]

do you use it here?

[Dmitry Titov, 2016/04/26 01:45:29]

Done.

+#include "base/location.h"
+#include "base/logging.h"
+#include "base/time/time.h"
+
+namespace {
+// Delay, in milliseconds, between the main document parsed and snapshot.

[fgorski, 2016/04/25 04:28:45]

nit: main document parsed event and 

[Dmitry Titov, 2016/04/26 01:45:29]

Done.

+// Note if the "load" event fires before this delay is up, then the snapshot
+// is taken immediately.
+const size_t DELAY_AFTER_DOCUMENT_AVAILABLE = 7000;
+
+}  // namespace
+
+namespace offline_pages {
+
+SnapshotController::SnapshotController(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    SnapshotController::Client* client)
+    : task_runner_(task_runner),
+      client_(client),
+      state_(State::kStateReady),

[fgorski, 2016/04/25 04:28:45]

I see a peculiarity here, since this is an enum class, you will use it scoped
with State:: always, why not:
State::kReady
State::kSnapshotPending
...

Instead of repeating State?

[Dmitry Titov, 2016/04/26 01:45:29]

Done.

+      weak_ptr_factory_(this) {
+}
+
+SnapshotController::~SnapshotController() {}
+
+void SnapshotController::Reset() {
+  state_ = State::kStateReady;
+}
+
+void SnapshotController::Stop() {
+  state_ = State::kStateStopped;
+}
+
+void SnapshotController::PreviousSnapshotCompleted() {
+  if (state_ != State::kStateStopped)

[fgorski, 2016/04/25 04:28:45]

It feels a bit weird that you can have a snapshot in progress in both Pending
and Stopped states.
Was there no reason to have it be more precise?

[Dmitry Titov, 2016/04/26 01:45:29]

Stopped is the terminal state which prevents new snapshots from being started.
The snapshot started before may still be in progress. Added comment and used
early return in hope it'll be clearer.

+    state_ = State::kStateReady;
+}
+
+void SnapshotController::DocumentAvailableInMainFrame() {
+  // Post a delayed task. The snapshot will happen either when the delay
+  // is up, or if the "load" event is dispatched in the main frame.
+  task_runner_->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&SnapshotController::MaybeStartSnapshot,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(DELAY_AFTER_DOCUMENT_AVAILABLE));
+}
+
+void SnapshotController::DocumentOnLoadCompletedInMainFrame() {
+  MaybeStartSnapshot();
+  // No more snapshots after onLoad (there still can be other events
+  // or delayed tasks that can try to start another snapshot)
+  Stop();
+}
+
+void SnapshotController::MaybeStartSnapshot() {
+  if (state_ != State::kStateReady)
+    return;
+  if (client_->StartSnapshot())
+    state_ = State::kStateSnapshotPending;
+}
+
+size_t SnapshotController::GetDelayAfterDocumentAvailableForTest() {
+  return DELAY_AFTER_DOCUMENT_AVAILABLE;
+}
+
+
+}  // namespace offline_pages