Remove ScopedVector from NavigationController.

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 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.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 109 matching lines @@
     case NavigationController::RESTORE_LAST_SESSION_CRASHED:
       return NavigationEntryImpl::RESTORE_LAST_SESSION_CRASHED;
   }
   NOTREACHED();
   return NavigationEntryImpl::RESTORE_CURRENT_SESSION;
 }
 
 // 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
     // the typed count.
     (*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());
   }
 }
 
 // Determines whether or not we should be carrying over a user agent override
 // between two NavigationEntries.
 bool ShouldKeepOverride(const NavigationEntry* last_entry) {
   return last_entry && last_entry->GetIsOverridingUserAgent();
 }
 
 // Helper method for FrameTree::ForEach to set the nav_entry_id on each current
@@ skipping 112 matching lines @@
 }
 
 void NavigationControllerImpl::SetBrowserContext(
     BrowserContext* browser_context) {
   browser_context_ = browser_context;
 }
 
 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);
 }
 
 void NavigationControllerImpl::Reload(bool check_for_repost) {
   ReloadInternal(check_for_repost, RELOAD);
 }
 void NavigationControllerImpl::ReloadIgnoringCache(bool check_for_repost) {
   ReloadInternal(check_for_repost, RELOAD_IGNORING_CACHE);
@@ skipping 121 matching lines @@
 bool NavigationControllerImpl::IsInitialBlankNavigation() const {
   // TODO(creis): Once we create a NavigationEntry for the initial blank page,
   // we'll need to check for entry count 1 and restore_type RESTORE_NONE (to
   // exclude the cloned tab case).
   return IsInitialNavigation() && GetEntryCount() == 0;
 }
 
 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(
     scoped_ptr<NavigationEntryImpl> entry) {
   // When navigating to a new page, we don't know for sure if we will actually
   // end up leaving the current page.  The new page load could for example
   // result in a download or a 'no content' response (e.g., a mailto: URL).
   SetPendingEntry(entry.Pass());
   NavigateToPendingEntry(NO_RELOAD);
 }
 
 void NavigationControllerImpl::SetPendingEntry(
     scoped_ptr<NavigationEntryImpl> entry) {
   DiscardNonCommittedEntriesInternal();
   pending_entry_ = entry.release();
   NotificationService::current()->Notify(
       NOTIFICATION_NAV_ENTRY_PENDING,
       Source<NavigationController>(this),
       Details<NavigationEntry>(pending_entry_));
 }
 
 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();
 }
 
 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.
   //
   // We make an exception for renderer-initiated navigations in new tabs, as
   // long as no other page has tried to access the initial empty document in
   // the new tab.  If another page modifies this blank page, a URL spoof is
   // possible, so we must stop showing the pending entry.
   bool safe_to_show_pending =
       pending_entry_ &&
@@ skipping 21 matching lines @@
   if (transient_entry_index_ != -1)
     return transient_entry_index_;
   if (pending_entry_index_ != -1)
     return pending_entry_index_;
   return last_committed_entry_index_;
 }
 
 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 {
   const std::string& mime_type = delegate_->GetContentsMimeType();
   bool is_viewable_mime_type =
       mime_util::IsSupportedNonImageMimeType(mime_type) &&
       !media::IsSupportedMediaMimeType(mime_type);
   NavigationEntry* visible_entry = GetVisibleEntry();
   return visible_entry && !visible_entry->IsViewSourceMode() &&
       is_viewable_mime_type && !delegate_->GetInterstitialPage();
 }
 
 int NavigationControllerImpl::GetLastCommittedEntryIndex() const {
   return last_committed_entry_index_;
 }
 
 int NavigationControllerImpl::GetEntryCount() const {
   DCHECK(entries_.size() <= max_entry_count());
   return static_cast<int>(entries_.size());
 }
 
 NavigationEntryImpl* NavigationControllerImpl::GetEntryAtIndex(
     int index) const {
   if (index < 0 || index >= GetEntryCount())
     return nullptr;
 
-  return entries_[index];
+  return entries_[index].get();
 }
 
 NavigationEntryImpl* NavigationControllerImpl::GetEntryAtOffset(
     int offset) const {
   return GetEntryAtIndex(GetIndexForOffset(offset));
 }
 
 int NavigationControllerImpl::GetIndexForOffset(int offset) const {
   return GetCurrentEntryIndex() + offset;
 }
@@ skipping 793 matching lines @@
     }
   }
 
   // We do not need to discard the pending entry in this case, since we will
   // not generate commit notifications for this auto-subframe navigation.
   return false;
 }
 
 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());
 }
 
 // There are two general cases where a navigation is "in page":
 // 1. A fragment navigation, in which the url is kept the same except for the
 //    reference fragment.
 // 2. A history API navigation (pushState and replaceState). This case is
 //    always in-page, but the urls are not guaranteed to match excluding the
 //    fragment. The relevant spec allows pushState/replaceState to any URL on
 //    the same origin.
@@ skipping 299 matching lines @@
     entry->set_unique_id(pending_entry_->GetUniqueID());
 
   DiscardNonCommittedEntriesInternal();
 
   int current_size = static_cast<int>(entries_.size());
 
   // When replacing, don't prune the forward history.
   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);
     return;
   }
 
   // We shouldn't see replace == true when there's no committed entries.
   DCHECK(!replace);
 
   if (current_size > 0) {
@@ skipping 61 matching lines @@
   // navigation to succeed.  The interstitial will stay visible until the
   // resulting DidNavigate.
   if (delegate_->GetInterstitialPage()) {
     static_cast<InterstitialPageImpl*>(delegate_->GetInterstitialPage())->
         CancelForNavigation();
   }
 
   // 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
   // renderer process is not live, unless it is the initial navigation of the
   // tab.
   if (IsRendererDebugURL(pending_entry_->GetURL())) {
     // TODO(creis): Find the RVH for the correct frame.
     if (!delegate_->GetRenderViewHost()->IsRenderViewLive() &&
         !IsInitialNavigation()) {
       DiscardNonCommittedEntries();
@@ skipping 226 matching lines @@
   for (int i = static_cast<int>(entries_.size()) - 1; i >= 0; --i) {
     if (entries_[i]->GetUniqueID() == nav_entry_id)
       return i;
   }
   return -1;
 }
 
 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(
     scoped_ptr<NavigationEntry> entry) {
   // Discard any current transient entry, we can only have one at a time.
   int index = 0;
   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);
 }
 
 void NavigationControllerImpl::InsertEntriesFrom(
     const NavigationControllerImpl& source,
     int max_index) {
   DCHECK_LE(max_index, source.GetEntryCount());
   size_t insert_index = 0;
   for (int i = 0; i < max_index; i++) {
     // When cloning a tab, copy all entries except interstitial pages.
     if (source.entries_[i]->GetPageType() != PAGE_TYPE_INTERSTITIAL) {
       // TODO(creis): Once we start sharing FrameNavigationEntries between
       // NavigationEntries, it will not be safe to share them with another tab.
       // Must have a version of Clone that recreates them.
       entries_.insert(entries_.begin() + insert_index++,
                       source.entries_[i]->Clone().Pass());
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content