Fix memory leak in RenderViewImpltest.OnNavigationHttpPost

PageStateToHistoryEntry was returning a raw pointer to an object that it
allocated, expecting all callers to manage its lifetime, and this test wasn't.
Change PageStateToHistoryEntry to return a scoped_ptr<HistoryEntry>, and use
scoped_ptrs in some other places to ensure future correct usage.

BUG=368798
TEST=No more leak

Review URL: https://codereview.chromium.org/267503006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267404 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 16 insertions, 13 deletions

diff --git a/content/renderer/history_controller.cc b/content/renderer/history_controller.cc
index d3efebe..b120198 100644
--- a/content/renderer/history_controller.cc
+++ b/content/renderer/history_controller.cc
@@ -53,12 +53,12 @@ HistoryController::HistoryController(RenderViewImpl* render_view)
 HistoryController::~HistoryController() {
 }
 
-void HistoryController::GoToEntry(HistoryEntry* target_entry,
+void HistoryController::GoToEntry(scoped_ptr<HistoryEntry> target_entry,
                                   WebURLRequest::CachePolicy cache_policy) {
   HistoryFrameLoadVector same_document_loads;
   HistoryFrameLoadVector different_document_loads;
 
-  provisional_entry_.reset(target_entry);
+  provisional_entry_ = target_entry.Pass();
 
   WebFrame* main_frame = render_view_->main_render_frame()->GetWebFrame();
   if (current_entry_) {
diff --git a/content/renderer/history_controller.h b/content/renderer/history_controller.h
index e57bed3..730ae43 100644
--- a/content/renderer/history_controller.h
+++ b/content/renderer/history_controller.h
@@ -109,7 +109,7 @@ class CONTENT_EXPORT HistoryController {
   explicit HistoryController(RenderViewImpl* render_view);
   ~HistoryController();
 
-  void GoToEntry(HistoryEntry* entry,
+  void GoToEntry(scoped_ptr<HistoryEntry> entry,
                  blink::WebURLRequest::CachePolicy cache_policy);
 
   void UpdateForCommit(RenderFrameImpl* frame,
diff --git a/content/renderer/history_serialization.cc b/content/renderer/history_serialization.cc
index c007907..dd78609 100644
--- a/content/renderer/history_serialization.cc
+++ b/content/renderer/history_serialization.cc
@@ -186,15 +186,15 @@ PageState SingleHistoryItemToPageState(const WebHistoryItem& item) {
   return PageState::CreateFromEncodedData(encoded_data);
 }
 
-HistoryEntry* PageStateToHistoryEntry(const PageState& page_state) {
+scoped_ptr<HistoryEntry> PageStateToHistoryEntry(const PageState& page_state) {
   ExplodedPageState state;
   if (!DecodePageState(page_state.ToEncodedData(), &state))
-    return NULL;
+    return scoped_ptr<HistoryEntry>();
 
-  HistoryEntry* entry = new HistoryEntry();
+  scoped_ptr<HistoryEntry> entry(new HistoryEntry());
   RecursivelyGenerateHistoryItem(state.top, entry->root_history_node());
 
-  return entry;
+  return entry.Pass();
 }
 
 }  // namespace content
diff --git a/content/renderer/history_serialization.h b/content/renderer/history_serialization.h
index 03758ba..e9752af 100644
--- a/content/renderer/history_serialization.h
+++ b/content/renderer/history_serialization.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/memory/scoped_ptr.h"
 #include "content/common/content_export.h"
 
 namespace blink {
@@ -20,7 +21,8 @@ class PageState;
 CONTENT_EXPORT PageState HistoryEntryToPageState(HistoryEntry* entry);
 CONTENT_EXPORT PageState SingleHistoryItemToPageState(
     const blink::WebHistoryItem& item);
-CONTENT_EXPORT HistoryEntry* PageStateToHistoryEntry(const PageState& state);
+CONTENT_EXPORT scoped_ptr<HistoryEntry> PageStateToHistoryEntry(
+    const PageState& state);
 
 }  // namespace content
 
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 449f5d2..ef12dda 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -729,12 +729,13 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) {
   } else if (params.page_state.IsValid()) {
     // We must know the page ID of the page we are navigating back to.
     DCHECK_NE(params.page_id, -1);
-    HistoryEntry* entry = PageStateToHistoryEntry(params.page_state);
+    scoped_ptr<HistoryEntry> entry =
+        PageStateToHistoryEntry(params.page_state);
     if (entry) {
       // Ensure we didn't save the swapped out URL in UpdateState, since the
       // browser should never be telling us to navigate to swappedout://.
       CHECK(entry->root().urlString() != WebString::fromUTF8(kSwappedOutURL));
-      render_view_->history_controller()->GoToEntry(entry, cache_policy);
+      render_view_->history_controller()->GoToEntry(entry.Pass(), cache_policy);
     }
   } else if (!params.base_url_for_data_url.is_empty()) {
     // A loadData request with a specified base URL.
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 0744475..f0a8802 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -333,9 +333,9 @@ TEST_F(RenderViewImplTest, OnNavigationHttpPost) {
 
   // Check post data sent to browser matches
   EXPECT_TRUE(host_nav_params.a.page_state.IsValid());
-  const blink::WebHistoryItem item =
-      PageStateToHistoryEntry(host_nav_params.a.page_state)->root();
-  blink::WebHTTPBody body = item.httpBody();
+  scoped_ptr<HistoryEntry> entry =
+      PageStateToHistoryEntry(host_nav_params.a.page_state);
+  blink::WebHTTPBody body = entry->root().httpBody();
   blink::WebHTTPBody::Element element;
   bool successful = body.elementAt(0, element);
   EXPECT_TRUE(successful);