Ntp: restore scroll position.

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 <stdint.h>
 
+#include <map>
+
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/frame_message_enums.h"
 #include "content/common/resource_request_body_impl.h"
@@ skipping 329 matching lines @@
   int frame_tree_node_id() const {
     return frame_tree_node_id_;
   }
   void set_frame_tree_node_id(int frame_tree_node_id) {
     frame_tree_node_id_ = frame_tree_node_id;
   }
 
   // Returns the history URL for a data URL to use in Blink.
   GURL GetHistoryURLForDataURL() const;
 
+  const std::map<std::string, base::string16>& get_extra_data() const {
+    return extra_data_;

[Ted C, 2016/09/19 16:00:43]

Sorry for my delay, but I have a couple general questions.

1.) What is extra data used for (not in the scope of this cl)?  Do we expect
things to be stored there normally?  Many things?  What is the lifetime of them?
 Is it entirely an in-memory map that we expect to get thrown away?

2.) Do we actually need to expose this?  Don't we only need to access one bit of
extra data?  Could we not just expose GetExtraDataForIndex(index, key) to avoid
the need for a new method, potentially unnecessary serialization across to JNI
that is only ever used once.

[Michael van Ouwerkerk, 2016/09/19 16:42:02]

SetExtraData is used from only a few places currently. These are
DataUseTabModel::OnNavigationEvent and SetPasswordStateInNavigation. Only the
password state is written into SerializedNavigationEntry.

The extra_data_ map has this doc:

// Used to store extra data to support browser features. This member is not
// persisted, unless specific data is taken out/put back in at save/restore
// time (see TabNavigation for an example of this).

From that (and the general concept of a NavigationEntry) it seems our use case
is suitable. Persistence is optional, and for this feature not desirable because
restoring scroll position after a long period would be confusing to the user.
Therefore the lifetime of the data is that of the NavigationEntryImpl. There is
currently very little of this data.
In Java we just have a copy of the NavigationEntry, it has no JNI of its own, or
a pointer to its C++ counterpart. I think this CL is in line with the existing
design. Creating a JNI method to go with NavigationController(Impl).java is
possible, but seems a bit counter to the existing class design.

[Ted C, 2016/09/19 19:59:08]

Cool...thanks for looking into this and describing it.
That is a fair point, but we seem to already be doing that for
setEntryExtraData, which is exposed on the Controller vs the Entry.  While I
don't have a strong preference whether that goes into the Controller or the
Entry, I think have symmetry of set and get is quite important.

+  }
+
 #if defined(OS_ANDROID)
   base::TimeTicks intent_received_timestamp() const {
     return intent_received_timestamp_;
   }
 
   void set_intent_received_timestamp(
       const base::TimeTicks intent_received_timestamp) {
     intent_received_timestamp_ = intent_received_timestamp;
   }
 
@@ skipping 140 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_