Remove ScopedVector from NavigationController.

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 87abff0fa4d8799944dfd7443d97ea107efe915f..b6c6804daaf6a95fe4c1f26ac20c19f950308719 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -127,7 +127,7 @@ NavigationEntryImpl::RestoreType ControllerRestoreTypeToEntryType(
 // Configure all the NavigationEntries in entries for restore. This resets
 // the transition type to reload and makes sure the content state isn't empty.
 void ConfigureEntriesForRestore(
-    ScopedVector<NavigationEntryImpl>* entries,
+    std::vector<scoped_ptr<NavigationEntryImpl>>* entries,
     NavigationController::RestoreType type) {
   for (size_t i = 0; i < entries->size(); ++i) {
     // Use a transition type of reload so that we don't incorrectly increase
@@ -135,7 +135,7 @@ void ConfigureEntriesForRestore(
     (*entries)[i]->SetTransitionType(ui::PAGE_TRANSITION_RELOAD);
     (*entries)[i]->set_restore_type(ControllerRestoreTypeToEntryType(type));
     // NOTE(darin): This code is only needed for backwards compat.
-    SetPageStateIfEmpty((*entries)[i]);
+    SetPageStateIfEmpty((*entries)[i].get());
   }
 }
 
@@ -268,19 +268,22 @@ void NavigationControllerImpl::SetBrowserContext(
 void NavigationControllerImpl::Restore(
     int selected_navigation,
     RestoreType type,
-    ScopedVector<NavigationEntry>* entries) {
+    std::vector<scoped_ptr<NavigationEntry>>* entries) {
   // Verify that this controller is unused and that the input is valid.
   DCHECK(GetEntryCount() == 0 && !GetPendingEntry());
   DCHECK(selected_navigation >= 0 &&
          selected_navigation < static_cast<int>(entries->size()));
 
   needs_reload_ = true;

[ncarter (slow), 2015/11/12 18:10:40]

Could reserve the additional capacity here too (entries_.size() +
entries.size()).

[Avi (use Gerrit), 2015/11/12 19:13:04]

entries_ is supposed to be empty (and is DCHECKed) so I'll simplify.

-  for (size_t i = 0; i < entries->size(); ++i) {
-    NavigationEntryImpl* entry =
-        NavigationEntryImpl::FromNavigationEntry((*entries)[i]);
-    entries_.push_back(entry);
+  for (auto& entry : *entries) {
+    scoped_ptr<NavigationEntryImpl> entry_impl(
+        NavigationEntryImpl::FromNavigationEntry(entry.release()));

[ncarter (slow), 2015/11/12 18:10:40]

Could use the scoped_ptr variant of ::FromNavigationEntry here.

[Avi (use Gerrit), 2015/11/12 19:13:04]

WHY DID I FORGET THAT EXISTED

+    entries_.push_back(entry_impl.Pass());
   }
-  entries->weak_clear();
+
+  // At this point, the |entries| is full of empty scoped_ptrs, so it can be
+  // cleared out safely.
+  entries->clear();
 
   // And finish the restore.
   FinishRestore(selected_navigation, type);
@@ -422,13 +425,13 @@ bool NavigationControllerImpl::IsInitialBlankNavigation() const {
 NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID(
   SiteInstance* instance, int32 page_id) const {
   int index = GetEntryIndexWithPageID(instance, page_id);
-  return (index != -1) ? entries_[index] : nullptr;
+  return (index != -1) ? entries_[index].get() : nullptr;
 }
 
 NavigationEntryImpl*
 NavigationControllerImpl::GetEntryWithUniqueID(int nav_entry_id) const {
   int index = GetEntryIndexWithUniqueID(nav_entry_id);
-  return (index != -1) ? entries_[index] : nullptr;
+  return (index != -1) ? entries_[index].get() : nullptr;
 }
 
 void NavigationControllerImpl::LoadEntry(
@@ -452,7 +455,7 @@ void NavigationControllerImpl::SetPendingEntry(
 
 NavigationEntryImpl* NavigationControllerImpl::GetActiveEntry() const {
   if (transient_entry_index_ != -1)
-    return entries_[transient_entry_index_];
+    return entries_[transient_entry_index_].get();
   if (pending_entry_)
     return pending_entry_;
   return GetLastCommittedEntry();
@@ -460,7 +463,7 @@ NavigationEntryImpl* NavigationControllerImpl::GetActiveEntry() const {
 
 NavigationEntryImpl* NavigationControllerImpl::GetVisibleEntry() const {
   if (transient_entry_index_ != -1)
-    return entries_[transient_entry_index_];
+    return entries_[transient_entry_index_].get();
   // The pending entry is safe to return for new (non-history), browser-
   // initiated navigations.  Most renderer-initiated navigations should not
   // show the pending entry, to prevent URL spoof attacks.
@@ -502,7 +505,7 @@ int NavigationControllerImpl::GetCurrentEntryIndex() const {
 NavigationEntryImpl* NavigationControllerImpl::GetLastCommittedEntry() const {
   if (last_committed_entry_index_ == -1)
     return NULL;
-  return entries_[last_committed_entry_index_];
+  return entries_[last_committed_entry_index_].get();
 }
 
 bool NavigationControllerImpl::CanViewSource() const {
@@ -529,7 +532,7 @@ NavigationEntryImpl* NavigationControllerImpl::GetEntryAtIndex(
   if (index < 0 || index >= GetEntryCount())
     return nullptr;
 
-  return entries_[index];
+  return entries_[index].get();
 }
 
 NavigationEntryImpl* NavigationControllerImpl::GetEntryAtOffset(
@@ -1343,10 +1346,11 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe(
 
 int NavigationControllerImpl::GetIndexOfEntry(
     const NavigationEntryImpl* entry) const {
-  const NavigationEntries::const_iterator i(std::find(
-      entries_.begin(),
-      entries_.end(),
-      entry));
+  const auto i =
+      std::find_if(entries_.begin(), entries_.end(),
+                   [entry](const scoped_ptr<NavigationEntryImpl>& item) {
+                     return item.get() == entry;
+                   });

[ncarter (slow), 2015/11/12 18:10:40]

A dumb for loop here (using an int or size_t) would probably be simpler than a
lambda. From within the for loop, you can just return directly, eliminating the
need for the ternary operator below.

[Avi (use Gerrit), 2015/11/12 19:13:04]

Done.

   return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin());
 }
 
@@ -1666,11 +1670,7 @@ void NavigationControllerImpl::InsertOrReplaceEntry(
   if (replace && current_size > 0) {
     int32 page_id = entry->GetPageID();
 
-    // ScopedVectors don't automatically delete the replaced value, so make sure
-    // the previous value gets deleted.
-    scoped_ptr<NavigationEntryImpl> old_entry(
-        entries_[last_committed_entry_index_]);
-    entries_[last_committed_entry_index_] = entry.release();
+    entries_[last_committed_entry_index_] = entry.Pass();

[ncarter (slow), 2015/11/12 18:10:40]

Good riddance.

[Avi (use Gerrit), 2015/11/12 19:13:04]

Acknowledged.

 
     // This is a new page ID, so we need everybody to know about it.
     delegate_->UpdateMaxPageID(page_id);
@@ -1752,7 +1752,7 @@ void NavigationControllerImpl::NavigateToPendingEntry(ReloadType reload_type) {
   // For session history navigations only the pending_entry_index_ is set.
   if (!pending_entry_) {
     CHECK_NE(pending_entry_index_, -1);
-    pending_entry_ = entries_[pending_entry_index_];
+    pending_entry_ = entries_[pending_entry_index_].get();
   }
 
   // Any renderer-side debug URLs or javascript: URLs should be ignored if the
@@ -1999,7 +1999,7 @@ int NavigationControllerImpl::GetEntryIndexWithUniqueID(
 NavigationEntryImpl* NavigationControllerImpl::GetTransientEntry() const {
   if (transient_entry_index_ == -1)
     return NULL;
-  return entries_[transient_entry_index_];
+  return entries_[transient_entry_index_].get();
 }
 
 void NavigationControllerImpl::SetTransientEntry(
@@ -2009,8 +2009,9 @@ void NavigationControllerImpl::SetTransientEntry(
   if (last_committed_entry_index_ != -1)
     index = last_committed_entry_index_ + 1;
   DiscardTransientEntry();
-  entries_.insert(entries_.begin() + index,
-                  NavigationEntryImpl::FromNavigationEntry(entry.release()));
+  scoped_ptr<NavigationEntryImpl> entry_impl(
+      NavigationEntryImpl::FromNavigationEntry(entry.release()));

[ncarter (slow), 2015/11/12 18:10:40]

.Pass() would also work here, which I think would let you get rid of the
entry_impl variable.

[Avi (use Gerrit), 2015/11/12 19:13:04]

Done.

+  entries_.insert(entries_.begin() + index, entry_impl.Pass());
   transient_entry_index_ = index;
   delegate_->NotifyNavigationStateChanged(INVALIDATE_TYPE_ALL);
 }