Better remove HistoryNodes

content/renderer/history_entry.h

 // Copyright 2014 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, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
  * Redistribution and use in source and binary forms, with or without
@@ skipping 20 matching lines @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef CONTENT_RENDERER_HISTORY_ENTRY_H_
 #define CONTENT_RENDERER_HISTORY_ENTRY_H_
 
 #include "base/containers/hash_tables.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
 #include "third_party/WebKit/public/web/WebHistoryItem.h"
 
 namespace blink {
 class WebFrame;
 }
 
 namespace content {
 class RenderFrameImpl;
 class RenderViewImpl;
 
 const int kInvalidFrameRoutingID = -1;
 
 class CONTENT_EXPORT HistoryEntry {
  public:
   class HistoryNode {
    public:
-    HistoryNode(HistoryEntry* entry,
-                const blink::WebHistoryItem& item,
-                int64_t frame_id);
+    HistoryNode(const base::WeakPtr<HistoryEntry>& entry,
+                const blink::WebHistoryItem& item);
     ~HistoryNode();
 
-    HistoryNode* AddChild(const blink::WebHistoryItem& item, int64_t frame_id);
+    HistoryNode* AddChild(const blink::WebHistoryItem& item);
     HistoryNode* AddChild();
-    HistoryNode* CloneAndReplace(HistoryEntry* new_entry,
+    HistoryNode* CloneAndReplace(const base::WeakPtr<HistoryEntry>& new_entry,
                                  const blink::WebHistoryItem& new_item,
                                  bool clone_children_of_target,
                                  RenderFrameImpl* target_frame,
                                  RenderFrameImpl* current_frame);
     blink::WebHistoryItem& item() { return item_; }
     void set_item(const blink::WebHistoryItem& item);
     std::vector<HistoryNode*>& children() const { return children_->get(); }
     void RemoveChildren();
 
    private:
-    HistoryEntry* entry_;
+    // When a HistoryEntry is destroyed, it takes all its HistoryNodes with it.
+    // Use a WeakPtr to ensure that HistoryNodes don't try to illegally access
+    // a dying HistoryEntry, or do unnecessary work when the whole entry is
+    // being destroyed.
+    base::WeakPtr<HistoryEntry> entry_;
     scoped_ptr<ScopedVector<HistoryNode> > children_;
     blink::WebHistoryItem item_;
+    std::vector<std::string> unique_names_;

[Charlie Reis, 2015/05/18 22:48:00]

Let's add a comment about why a single HistoryNode needs to track multiple
unique names.

[Nate Chapin, 2015/05/28 21:31:50]

Done.

   };
 
-  HistoryEntry(const blink::WebHistoryItem& root, int64_t frame_id);
+  HistoryEntry(const blink::WebHistoryItem& root);
   HistoryEntry();
   ~HistoryEntry();
 
   HistoryEntry* CloneAndReplace(const blink::WebHistoryItem& newItem,
                                 bool clone_children_of_target,
                                 RenderFrameImpl* target_frame,
                                 RenderViewImpl* render_view);
 
   HistoryNode* GetHistoryNodeForFrame(RenderFrameImpl* frame);
   blink::WebHistoryItem GetItemForFrame(RenderFrameImpl* frame);
   const blink::WebHistoryItem& root() const { return root_->item(); }
   HistoryNode* root_history_node() const { return root_.get(); }
 
  private:
-
   scoped_ptr<HistoryNode> root_;
 
-  typedef base::hash_map<uint64_t, HistoryNode*> FramesToItems;
-  FramesToItems frames_to_items_;
-
   typedef base::hash_map<std::string, HistoryNode*> UniqueNamesToItems;
   UniqueNamesToItems unique_names_to_items_;
+
+  base::WeakPtrFactory<HistoryEntry> weak_ptr_factory_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_HISTORY_ENTRY_H_