Add DidSaveScrollOrScaleState flag to prevent restoreScrollPositionAndViewState restore from default

content/common/page_state_serialization.cc

Index: content/common/page_state_serialization.cc
diff --git a/content/common/page_state_serialization.cc b/content/common/page_state_serialization.cc
index 894ff2be722b9503cd63916528807cbe05eab224..31c4e3126d465770ed5acc2d314f480b2e294c22 100644
--- a/content/common/page_state_serialization.cc
+++ b/content/common/page_state_serialization.cc
@@ -196,12 +196,13 @@ struct SerializeObject {
 // 21: Add frame sequence number.
 // 22: Add scroll restoration type.
 // 23: Remove frame sequence number, there are easier ways.
+// 24: Add did save scroll state.
 //
 // NOTE: If the version is -1, then the pickle contains only a URL string.
 // See ReadPageState.
 //
 const int kMinVersion = 11;
-const int kCurrentVersion = 23;
+const int kCurrentVersion = 24;
 
 // A bunch of convenience functions to read/write to SerializeObjects.  The
 // de-serializers assume the input data will be in the correct format and fall
@@ -514,6 +515,7 @@ void WriteFrameState(
   WriteReal(state.page_scale_factor, obj);
   WriteInteger64(state.item_sequence_number, obj);
   WriteInteger64(state.document_sequence_number, obj);
+  WriteBoolean(state.did_save_scroll_state, obj);

[majidvp, 2017/02/14 20:42:16]

Why here? I feel if anything, it should be written just before the scroll
position.

   WriteInteger(static_cast<int>(state.referrer_policy), obj);
   WriteReal(state.visual_viewport_scroll_offset.x(), obj);
   WriteReal(state.visual_viewport_scroll_offset.y(), obj);
@@ -578,6 +580,9 @@ void ReadFrameState(SerializeObject* obj, bool is_top,
   if (obj->version >= 17 && obj->version < 19)
     ReadInteger64(obj); // Skip obsolete target frame id number.
 
+  if (obj->version >= 24)
+    state->did_save_scroll_state = ReadBoolean(obj);
+
   if (obj->version >= 18) {
     state->referrer_policy =
         static_cast<blink::WebReferrerPolicy>(ReadInteger(obj));
@@ -692,8 +697,8 @@ ExplodedFrameState::ExplodedFrameState()
       item_sequence_number(0),
       document_sequence_number(0),
       page_scale_factor(0.0),
-      referrer_policy(blink::WebReferrerPolicyDefault) {
-}
+      did_save_scroll_state(false),

[majidvp, 2017/02/14 21:00:57]

I think this is the incorrect initial value. In particular
this is used when we have not persisted the field (i.e. version < 24).
In such cases, I believe the legacy behaviour is to always assume that
there is a saved scroll state saved. Which suggests this should be 'true'. 

+      referrer_policy(blink::WebReferrerPolicyDefault) {}
 
 ExplodedFrameState::ExplodedFrameState(const ExplodedFrameState& other) {
   assign(other);
@@ -719,6 +724,7 @@ void ExplodedFrameState::assign(const ExplodedFrameState& other) {
   item_sequence_number = other.item_sequence_number;
   document_sequence_number = other.document_sequence_number;
   page_scale_factor = other.page_scale_factor;
+  did_save_scroll_state = other.did_save_scroll_state;
   referrer_policy = other.referrer_policy;
   http_body = other.http_body;
   children = other.children;