OOPIF: Support session restore by combining/splitting frame PageStates.

content/browser/frame_host/navigation_entry_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
@@ skipping 183 matching lines @@
 
   // Returns the FrameNavigationEntry corresponding to the frame with the given
   // |unique_name|, if any. This is useful when the FrameTreeNode cannot be used
   // to find the entry, such as for a newly created subframe in a history
   // navigation. Callers should update the FrameTreeNode ID of the entry so that
   // it can be found with |GetFrameEntry| above.
   // TODO(creis): Generate or verify the unique_name in the browser process.
   FrameNavigationEntry* GetFrameEntryByUniqueName(
       const std::string& unique_name) const;
 
+  // In OOPIF modes, this updates the cached copy of the full PageState whenever
+  // a frame's individual state changes. Keeping a cached copy is necessary to
+  // be able to return a const ref from GetPageState.

[Avi (use Gerrit), 2015/12/02 19:17:43]

I would much rather change the API. PageState isn't that big, and if we can
return a copy by value rather than const ref, then we can avoid the error-prone
scheme of having to call UpdatePageState after updating the state of any given
frame and simplify this a lot.

[Charlie Reis, 2015/12/02 19:48:18]

I can give it another try in a preliminary CL, though I remember it quickly got
unwieldy the last time I attempted.

Needing to call UpdatePageState alongside FNE::set_page_state is definitely
unfortunate, but I did kind of like having a separate NavEntry::page_state_ from
FNE::page_state_, since that frees us up to move FNE to a FrameState object
instead (while still supporting default Chrome).

[Charlie Reis, 2015/12/02 20:44:14]

Ah, I was mistaken!  The last time, I tried to remove the const first, hoping to
preserve the const ref (by keeping a cached copy).  Removing the const was the
viral change.

Dropping the const ref is much easier (and leads to a simpler CL).  We can do a
separate PageState for NavigationEntryImpl later when we start to move towards
FrameState.  Thanks for the suggestion!

[Avi (use Gerrit), 2015/12/02 21:15:45]

You're welcome.

Returning a const object by value is weird anyway.

+  void UpdatePageState();
+
   void set_unique_id(int unique_id) {
     unique_id_ = unique_id;
   }
 
   // The SiteInstance represents which pages must share processes. This is a
   // reference counted pointer to a shared SiteInstance.
   //
   // Note that the SiteInstance should usually not be changed after it is set,
   // but this may happen if the NavigationEntry was cloned and needs to use a
   // different SiteInstance.
@@ skipping 173 matching lines @@
   ui::PageTransition transition_type_;
   GURL user_typed_url_;
   bool has_post_data_;
   int64 post_id_;
   RestoreType restore_type_;
   GURL original_request_url_;
   bool is_overriding_user_agent_;
   base::Time timestamp_;
   int http_status_code_;
 
+  // In default Chrome, this is the PageState reported from the renderer. In
+  // OOPIF modes, this is a cached copy of the PageState generated from the
+  // various FrameNavigationEntry states. In either case, it is persisted to
+  // support session/tab restore.
+  PageState page_state_;
+
   // This member is not persisted with session restore because it is transient.
   // If the post request succeeds, this field is cleared since the same
   // information is stored in |content_state_| above. It is also only shallow
   // copied with compiler provided copy constructor.
   // Cleared in |ResetForCommit|.
   scoped_refptr<const base::RefCountedMemory> browser_initiated_post_data_;
 
   // This is also a transient member (i.e. is not persisted with session
   // restore). The screenshot of a page is taken when navigating away from the
   // page. This screenshot is displayed during an overscroll-navigation
@@ skipping 81 matching lines @@
   // persisted, unless specific data is taken out/put back in at save/restore
   // time (see TabNavigation for an example of this).
   std::map<std::string, base::string16> extra_data_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationEntryImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_ENTRY_IMPL_H_