Ntp: restore scroll position.

content/public/browser/navigation_entry.h

 // Copyright (c) 2012 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_PUBLIC_BROWSER_NAVIGATION_ENTRY_H_
 #define CONTENT_PUBLIC_BROWSER_NAVIGATION_ENTRY_H_
 
 #include <stdint.h>
 
+#include <map>
 #include <memory>
 #include <string>
 
 #include "base/memory/ref_counted_memory.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/common/content_export.h"
 #include "content/public/common/page_type.h"
 #include "content/public/common/referrer.h"
@@ skipping 178 matching lines @@
   virtual void SetCanLoadLocalResources(bool allow) = 0;
   virtual bool GetCanLoadLocalResources() const = 0;
 
   // Set extra data on this NavigationEntry according to the specified |key|.
   // This data is not persisted by default.
   virtual void SetExtraData(const std::string& key,
                             const base::string16& data) = 0;
   // If present, fills the |data| present at the specified |key|.
   virtual bool GetExtraData(const std::string& key,
                             base::string16* data) const = 0;
+  // Returns the extra data for this entry.
+  virtual const std::map<std::string, base::string16>& GetExtraData() const = 0;

[clamy, 2016/09/19 09:49:23]

I cannot see any usage outside of content/ of this function. The content/ public
API policy is not to expose any function that is not used outside of content/.
So this should be removed.

Additionally, why do you need to expose the map when you have getters and
setters to interact with it?

[Michael van Ouwerkerk, 2016/09/19 14:27:40]

Hi, are you suggesting that I should move this method to NavigationEntryImpl to
avoid defining it in the public content API? CreateJavaNavigationEntry in
navigation_controller_android.cc would then have to cast from NavigationEntry to
NavigationEntryImpl. That would feel like a strange design to me.

I need this method because CreateJavaNavigationEntry makes a Java copy of the
entry, and it does not know what the keys are.

[clamy, 2016/09/19 15:25:27]

Yes precisely. This is the design used all over content: anything not needed
outside of content/ is defined in the impl version of the class. Then classes
inside content/ freely use the impl version. This is done either by casting to
the FooImpl version or by having the implementation of functions of the public
API return the Imp version. Example:

NavigationEntry NavigationController::GetEntryAtIndex
is actually implemented as
NavigationEntryImpl NavigationControllerImpl::GetEntryAsIndex (in
navigation_controller_impl.h). For your use case, NavigationControllerAndroid
should just ensured it's keeping a pointer to a NavigationControllerImpl, and
then all NavigationEntries returned by this controller can be directly used a
NavigationEntryImpls.

Alternatively, you can safely cast the NavigationEntry to a NavigationEntryImpl.
Acknowledged.

[Michael van Ouwerkerk, 2016/09/19 15:57:39]

Done.

   // Removes the data at the specified |key|.
   virtual void ClearExtraData(const std::string& key) = 0;
 
   // The status code of the last known successful navigation.  If
   // GetHttpStatusCode() returns 0 that means that either:
   //
   //   - this navigation hasn't completed yet;
   //   - a response wasn't received;
   //   - or this navigation was restored and for some reason the
   //     status code wasn't available.
   virtual void SetHttpStatusCode(int http_status_code) = 0;
   virtual int GetHttpStatusCode() const = 0;
 
   // The redirect chain traversed during this navigation, from the initial
   // redirecting URL to the final non-redirecting current URL.
   virtual void SetRedirectChain(const std::vector<GURL>& redirects) = 0;
   virtual const std::vector<GURL>& GetRedirectChain() const = 0;
 
   // True if this entry is restored and hasn't been loaded.
   virtual bool IsRestored() const = 0;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_NAVIGATION_ENTRY_H_