Don't mark navigations as in-page when DSNs are mismatched.

content/renderer/render_frame_impl.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/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 5512 matching lines @@
         history_load_type = request_params.is_same_document_history_load
                                 ? blink::WebHistorySameDocumentLoad
                                 : blink::WebHistoryDifferentDocumentLoad;
         load_type = request_params.is_history_navigation_in_new_child
                         ? blink::WebFrameLoadType::InitialHistoryLoad
                         : blink::WebFrameLoadType::BackForward;
         should_load_request = true;
 
         // If this is marked as a same document load but we haven't committed
         // anything, treat it as a new load.  The browser shouldn't let this
-        // happen.
-        // TODO(creis): Add a similar check if the DSN doesn't match, and add a
-        // NOTREACHED when we're confident this won't happen.
-        if (history_load_type == blink::WebHistorySameDocumentLoad &&
-            current_history_item_.isNull()) {
-          history_load_type = blink::WebHistoryDifferentDocumentLoad;
+        // happen. Also treat it as a new load, if the DSN in
+        // |current_history_item_| and the item navigated to mismatch.
+        if (history_load_type == blink::WebHistorySameDocumentLoad) {
+           if (current_history_item_.isNull()) {
+             history_load_type = blink::WebHistoryDifferentDocumentLoad;
+           } else if (current_history_item_.documentSequenceNumber() !=
+                      item_for_history_navigation.documentSequenceNumber()) {
+             history_load_type = blink::WebHistoryDifferentDocumentLoad;
+             NOTREACHED();

[Charlie Reis, 2016/08/22 12:20:21]

I don't think we can have a NOTREACHED in this case, due to races where the
renderer process navigates to a different document (e.g., synchronous commit to
about:blank) just as the browser is telling it to go back in-page.  I bet we
could even write a test for that case.  :)

My TODO to add a NOTREACHED was for the "no current history item" case, when the
browser should know better than to treat it as in-page.  We should still do
that.

+           }
         }
 
         // If this navigation is to a history item for a new child frame, we may
         // want to ignore it in some cases.  If a Javascript navigation (i.e.,
         // client redirect) interrupted it and has either been scheduled,
         // started loading, or has committed, we should ignore the history item.
         // Similarly, if the history item just says to stay on about:blank,
         // don't load it again, which might clobber injected content.
         bool interrupted_by_client_redirect =
             frame_->isNavigationScheduledWithin(0) ||
@@ skipping 794 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content