Add DidSaveScrollOrScaleState flag to prevent restoreScrollPositionAndViewState restore from default

content/common/page_state_serialization.cc

 // 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.
 
 #include "content/common/page_state_serialization.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <limits>
@@ skipping 178 matching lines @@
 // 16: Switched from blob urls to blob uuids.
 // 17: Add a target frame id number.
 // 18: Add referrer policy.
 // 19: Remove target frame id, which was a bad idea, and original url string,
 //         which is no longer used.
 // 20: Add pinch viewport scroll offset, the offset of the pinched zoomed
 //     viewport within the unzoomed main frame.
 // 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
 // back to returning safe defaults when not.
 
 void WriteData(const void* data, int length, SerializeObject* obj) {
   obj->pickle.WriteData(static_cast<const char*>(data), length);
 }
 
 void ReadData(SerializeObject* obj, const void** data, int* length) {
@@ skipping 284 matching lines @@
     const ExplodedFrameState& state, SerializeObject* obj, bool is_top) {
   // WARNING: This data may be persisted for later use. As such, care must be
   // taken when changing the serialized format. If a new field needs to be
   // written, only adding at the end will make it easier to deal with loading
   // older versions. Similarly, this should NOT save fields with sensitive
   // data, such as password fields.
 
   WriteString(state.url_string, obj);
   WriteString(state.target, obj);
   WriteInteger(state.scroll_offset.x(), obj);
   WriteInteger(state.scroll_offset.y(), obj);

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

Is did_save_scroll_state is false, then why are we writing scroll_offset and
visual_viewport_scroll_offset? Presumably we can skip writing/reading.  (Maybe
even scale factor?)

   WriteString(state.referrer, obj);
 
   WriteStringVector(state.document_state, obj);
 
   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);
 
   WriteInteger(state.scroll_restoration_type, obj);
 
   bool has_state_object = !state.state_object.is_null();
   WriteBoolean(has_state_object, obj);
   if (has_state_object)
     WriteString(state.state_object, obj);
@@ skipping 44 matching lines @@
 
   state->page_scale_factor = ReadReal(obj);
   state->item_sequence_number = ReadInteger64(obj);
   state->document_sequence_number = ReadInteger64(obj);
   if (obj->version >= 21 && obj->version < 23)
     ReadInteger64(obj); // Skip obsolete frame sequence number.
 
   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));
   }
 
   if (obj->version >= 20) {
     double x = ReadReal(obj);
     double y = ReadReal(obj);
     state->visual_viewport_scroll_offset = gfx::PointF(x, y);
   } else {
@@ skipping 94 matching lines @@
 ExplodedHttpBody::ExplodedHttpBody() : contains_passwords(false) {}
 
 ExplodedHttpBody::~ExplodedHttpBody() {
 }
 
 ExplodedFrameState::ExplodedFrameState()
     : scroll_restoration_type(blink::WebHistoryScrollRestorationAuto),
       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);
 }
 
 ExplodedFrameState::~ExplodedFrameState() {
 }
 
 void ExplodedFrameState::operator=(const ExplodedFrameState& other) {
   if (&other != this)
     assign(other);
 }
 
 void ExplodedFrameState::assign(const ExplodedFrameState& other) {
   url_string = other.url_string;
   referrer = other.referrer;
   target = other.target;
   state_object = other.state_object;
   document_state = other.document_state;
   scroll_restoration_type = other.scroll_restoration_type;
   visual_viewport_scroll_offset = other.visual_viewport_scroll_offset;
   scroll_offset = other.scroll_offset;
   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;
 }
 
 ExplodedPageState::ExplodedPageState() {
 }
 
 ExplodedPageState::~ExplodedPageState() {
 }
@@ skipping 49 matching lines @@
   // because it covers additional data (e.g.|contains_sensitive_info|) which
   // is marshaled between native code and java. WriteResourceRequestBody()
   // serializes data which needs to be saved out to disk.
   WriteBoolean(resource_request_body.contains_sensitive_info(), &obj);
   return obj.GetAsString();
 }
 
 #endif
 
 }  // namespace content