Do not use GetPendingEntryIndex in TabRestoreServiceHelper::PopulateTab.

components/sessions/core/tab_restore_service_helper.cc

Index: components/sessions/core/tab_restore_service_helper.cc
diff --git a/components/sessions/core/tab_restore_service_helper.cc b/components/sessions/core/tab_restore_service_helper.cc
index f4f111975bc8daabfb164e3953dcbd4cc415a35c..e792dd6b67f9fd726f6b013b37e7130b39c4343c 100644
--- a/components/sessions/core/tab_restore_service_helper.cc
+++ b/components/sessions/core/tab_restore_service_helper.cc
@@ -410,16 +410,11 @@ void TabRestoreServiceHelper::PopulateTab(Tab* tab,
                                           int index,
                                           LiveTabContext* context,
                                           LiveTab* live_tab) {
-  const int pending_index = live_tab->GetPendingEntryIndex();
   int entry_count =
       live_tab->IsInitialBlankNavigation() ? 0 : live_tab->GetEntryCount();
-  if (entry_count == 0 && pending_index == 0)

[Eugene But (OOO till 7-30), 2017/03/21 22:31:14]

When entry_count is 0, then pending_index is always -1, so this should never be
true:
https://cs.chromium.org/chromium/src/content/public/browser/navigation_contro...

[Charlie Reis, 2017/03/22 22:49:49]

Yes, I can't think of a case that both entry_count and pending_index would be 0.

I thought about the case of opening a slow URL in a new tab (when entry_count is
0 but there's a pending entry), but that indeed has pending_index -1.  (Note
that we *do* get here in that case, because we don't filter out tabs with zero
entries until AddEntry/FilterEntry/IsTabInteresting after this call.  But
entry_count is zero, so the loop below does nothing.)

-    entry_count++;
   tab->navigations.resize(static_cast<int>(entry_count));
   for (int i = 0; i < entry_count; ++i) {
-    SerializedNavigationEntry entry = (i == pending_index)
-                                          ? live_tab->GetPendingEntry()
-                                          : live_tab->GetEntryAtIndex(i);
+    SerializedNavigationEntry entry = live_tab->GetEntryAtIndex(i);

[Eugene But (OOO till 7-30), 2017/03/21 22:31:15]

Because |entry_count| does not include pending item, there is no need to restore
pending item.

[sky, 2017/03/22 03:20:16]

Are you sure about that? I thought reload and possibly other things making the
pending index point to an existing one.

[Eugene But (OOO till 7-30), 2017/03/22 14:38:40]

It is true that reload and back-forward navigation make pending index point to
existing one. However in that case |entry_count| will not be 0, so the previous
2 lines I deleted will not execute. Also |i == pending_index)| will be true iff
item is both bending and committed, so |live_tab->GetEntryAtIndex(i)| and
|live_tab->GetPendingEntry()| will return the same result.

[Charlie Reis, 2017/03/22 22:49:49]

Yeah, pointing to an existing one is fine-- your CL wouldn't affect that.  It
would only matter if GetPendingEntry() didn't return GetEntryAtIndex(i) when
pending_entry_index_ is i.  That seems like it can only happen if i is -1.

Since i can't be -1 here (given the loop bounds), I think your change is
probably safe.

     tab->navigations[i] = entry;
   }
   tab->timestamp = TimeNow();