Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(339)

Side by Side Diff: content/browser/frame_host/navigation_controller_impl.cc

Issue 1672373002: Don't rely on the pending NavigationEntry for location.replace. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix bug numbers Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | content/browser/frame_host/navigation_controller_impl_browsertest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 /* 5 /*
6 * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved. 6 * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
7 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies) 7 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
8 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. 8 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
9 * (http://www.torchmobile.com/) 9 * (http://www.torchmobile.com/)
10 * 10 *
(...skipping 829 matching lines...) Expand 10 before | Expand all | Expand 10 after
840 pending_entry_->set_restore_type(NavigationEntryImpl::RESTORE_NONE); 840 pending_entry_->set_restore_type(NavigationEntryImpl::RESTORE_NONE);
841 841
842 // If we are doing a cross-site reload, we need to replace the existing 842 // If we are doing a cross-site reload, we need to replace the existing
843 // navigation entry, not add another entry to the history. This has the side 843 // navigation entry, not add another entry to the history. This has the side
844 // effect of removing forward browsing history, if such existed. Or if we are 844 // effect of removing forward browsing history, if such existed. Or if we are
845 // doing a cross-site redirect navigation, we will do a similar thing. 845 // doing a cross-site redirect navigation, we will do a similar thing.
846 // 846 //
847 // If this is an error load, we may have already removed the pending entry 847 // If this is an error load, we may have already removed the pending entry
848 // when we got the notice of the load failure. If so, look at the copy of the 848 // when we got the notice of the load failure. If so, look at the copy of the
849 // pending parameters that were saved. 849 // pending parameters that were saved.
850 //
851 // TODO(creis): This block should be unnecessary now that we pass
852 // params.should_replace_current_entry. Remove it once we verify with the
853 // check below.
850 if (params.url_is_unreachable && failed_pending_entry_id_ != 0) { 854 if (params.url_is_unreachable && failed_pending_entry_id_ != 0) {
851 details->did_replace_entry = failed_pending_entry_should_replace_; 855 details->did_replace_entry = failed_pending_entry_should_replace_;
852 } else { 856 } else {
853 details->did_replace_entry = pending_entry_ && 857 details->did_replace_entry = pending_entry_ &&
854 pending_entry_->should_replace_entry(); 858 pending_entry_->should_replace_entry();
855 } 859 }
860 CHECK(!details->did_replace_entry || params.should_replace_current_entry);
861 if (params.should_replace_current_entry)
862 details->did_replace_entry = true;
856 863
857 // Do navigation-type specific actions. These will make and commit an entry. 864 // Do navigation-type specific actions. These will make and commit an entry.
858 details->type = ClassifyNavigation(rfh, params); 865 details->type = ClassifyNavigation(rfh, params);
859 866
860 // is_in_page must be computed before the entry gets committed. 867 // is_in_page must be computed before the entry gets committed.
861 details->is_in_page = IsURLInPageNavigation( 868 details->is_in_page = IsURLInPageNavigation(
862 params.url, params.was_within_same_page, rfh); 869 params.url, params.was_within_same_page, rfh);
863 870
864 switch (details->type) { 871 switch (details->type) {
865 case NAVIGATION_TYPE_NEW_PAGE: 872 case NAVIGATION_TYPE_NEW_PAGE:
(...skipping 124 matching lines...) Expand 10 before | Expand all | Expand 10 after
990 // navigated on a popup navigated to about:blank (the iframe would be 997 // navigated on a popup navigated to about:blank (the iframe would be
991 // written into the popup by script on the main page). For these cases, 998 // written into the popup by script on the main page). For these cases,
992 // there isn't any navigation stuff we can do, so just ignore it. 999 // there isn't any navigation stuff we can do, so just ignore it.
993 if (!GetLastCommittedEntry()) 1000 if (!GetLastCommittedEntry())
994 return NAVIGATION_TYPE_NAV_IGNORE; 1001 return NAVIGATION_TYPE_NAV_IGNORE;
995 1002
996 // Valid subframe navigation. 1003 // Valid subframe navigation.
997 return NAVIGATION_TYPE_NEW_SUBFRAME; 1004 return NAVIGATION_TYPE_NEW_SUBFRAME;
998 } 1005 }
999 1006
1007 // Cross-process location.replace navigations should be classified as New with
1008 // replacement rather than ExistingPage, since it is not safe to reuse the
1009 // NavigationEntry.
1010 // TODO(creis): Have the renderer classify location.replace as
1011 // did_create_new_entry for all cases and eliminate this special case. This
1012 // requires updating several test expectations. See https://crbug.com/317872.
1013 if (!rfh->GetParent() && GetLastCommittedEntry() &&
1014 GetLastCommittedEntry()->site_instance() != rfh->GetSiteInstance() &&
1015 params.should_replace_current_entry) {
1016 return NAVIGATION_TYPE_NEW_PAGE;
1017 }
1018
1000 // We only clear the session history when navigating to a new page. 1019 // We only clear the session history when navigating to a new page.
1001 DCHECK(!params.history_list_was_cleared); 1020 DCHECK(!params.history_list_was_cleared);
1002 1021
1003 if (rfh->GetParent()) { 1022 if (rfh->GetParent()) {
1004 // All manual subframes would be did_create_new_entry and handled above, so 1023 // All manual subframes would be did_create_new_entry and handled above, so
1005 // we know this is auto. 1024 // we know this is auto.
1006 if (GetLastCommittedEntry()) { 1025 if (GetLastCommittedEntry()) {
1007 return NAVIGATION_TYPE_AUTO_SUBFRAME; 1026 return NAVIGATION_TYPE_AUTO_SUBFRAME;
1008 } else { 1027 } else {
1009 // We ignore subframes created in non-committed pages; we'd appreciate if 1028 // We ignore subframes created in non-committed pages; we'd appreciate if
(...skipping 1034 matching lines...) Expand 10 before | Expand all | Expand 10 after
2044 } 2063 }
2045 } 2064 }
2046 } 2065 }
2047 2066
2048 void NavigationControllerImpl::SetGetTimestampCallbackForTest( 2067 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
2049 const base::Callback<base::Time()>& get_timestamp_callback) { 2068 const base::Callback<base::Time()>& get_timestamp_callback) {
2050 get_timestamp_callback_ = get_timestamp_callback; 2069 get_timestamp_callback_ = get_timestamp_callback;
2051 } 2070 }
2052 2071
2053 } // namespace content 2072 } // namespace content
OLDNEW
« no previous file with comments | « no previous file | content/browser/frame_host/navigation_controller_impl_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698