Add taskid for navigation, created in session sync

components/sync_sessions/task_tracker.cc

+// Copyright 2017 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/sync_sessions/task_tracker.h"
+
+#include <utility>
+
+#include "base/numerics/safe_conversions.h"
+
+namespace sync_sessions {
+
+TabTasks::TabTasks(base::Clock* clock) : clock_(clock) {}
+
+TabTasks::~TabTasks() {}
+
+std::vector<int64_t> TabTasks::GetTaskIdAtNavIndex(int nav_index) const {
+  CHECK(base::checked_cast<size_t>(nav_index) < ids_.size());

[Nicolas Zea, 2017/03/27 20:43:52]

CHECK_LT

[shenchao, 2017/04/06 00:54:55]

Done.

+  std::vector<int64_t> id;
+  for (int i = 0; i <= nav_index; i++) {
+    if (ids_[i] != -1)
+      id.push_back(ids_[i]);
+  }
+  return id;
+}
+
+void TabTasks::UpdateTask(int current_nav_index,
+                          ui::PageTransition current_nav_transition) {
+  // We expect current_nav_index == 0 when the function is first called.
+  // However it's possible that current_nav_index == 1 when navigation at 0 is
+  // chrome://newtab, which is ignored by session sync.
+  if (current_nav_index == 1 && current_task_index_ == -1) {

[Nicolas Zea, 2017/03/27 20:43:52]

This seems a bit brittle, as it assumes behavior of the caller. For example,
what will happen on restart, when a tab with a navigation stack is restored?

If seems like we should have the same behavior regardless of current_nav_index,
right?

[shenchao, 2017/04/06 00:54:55]

Done.

+    ids_.push_back(-1);
+    current_task_index_ = 0;
+  }
+
+  // Triggered by some notifications on the current page, do nothing.
+  if (current_nav_index < 0 || current_nav_index == current_task_index_) {
+    DVLOG(1) << "Doing nothing";

[Nicolas Zea, 2017/03/27 20:43:52]

nit: for debugability, might be good to also print out what the transition we're
ignoring is (here and below)

[shenchao, 2017/04/06 00:54:55]

Done.

+    return;
+  }
+
+  // Back to some previous navigation or forward to some future navigation after
+  // going back.
+  if (current_nav_index < current_task_index_ ||
+      (current_nav_index > current_task_index_ &&
+       current_nav_transition & ui::PAGE_TRANSITION_FORWARD_BACK &&
+       base::checked_cast<size_t>(current_nav_index) < ids_.size())) {
+    DVLOG(1) << "Just updating task depth.";
+    current_task_index_ = current_nav_index;
+    return;
+  }
+
+  // a new navigation

[Nicolas Zea, 2017/03/27 20:43:52]

grammar nit: "A new navigation."

[shenchao, 2017/04/06 00:54:55]

Done.

+  if (current_nav_index == current_task_index_ + 1) {

[Nicolas Zea, 2017/03/27 20:43:52]

This seems to create a new task regardless of transition type. Shouldn't this
check for link transition first?

[shenchao, 2017/04/06 00:54:55]

Done.

+    DVLOG(1) << "Creating a task.";
+    current_task_index_ = current_nav_index;
+    ids_.erase(ids_.begin() + current_nav_index, ids_.end());
+    ids_.push_back(clock_->Now().ToTimeT());

[Nicolas Zea, 2017/03/27 20:43:52]

I think the ids can't be based on their own clock. They need to match the global
id that are already being synced (so that we can use task id to identify
individual navigations)

[shenchao, 2017/04/06 00:54:55]

We can't use global id so have to create our own task id. 

+    return;
+  }
+}
+
+TaskTracker::TaskTracker(std::unique_ptr<base::Clock> clock)
+    : clock_(std::move(clock)) {}
+
+TaskTracker::~TaskTracker() {}
+
+TabTasks* TaskTracker::GetTabTasks(SessionID::id_type tab_id) {
+  if (local_tab_tasks_map_.find(tab_id) == local_tab_tasks_map_.end()) {
+    local_tab_tasks_map_[tab_id] = base::MakeUnique<TabTasks>(clock_.get());
+  }
+  return local_tab_tasks_map_[tab_id].get();
+}
+
+}  // namespace sync_sessions