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

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

Issue 2475693002: Do not reset NavigationHandle when navigating same-page (Closed)
Patch Set: Rebase + removed DCHECK Created 4 years, 1 month 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
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 763 matching lines...) Expand 10 before | Expand all | Expand 10 after
774 bool NavigationControllerImpl::PendingEntryMatchesHandle( 774 bool NavigationControllerImpl::PendingEntryMatchesHandle(
775 NavigationHandleImpl* handle) const { 775 NavigationHandleImpl* handle) const {
776 return pending_entry_ && 776 return pending_entry_ &&
777 pending_entry_->GetUniqueID() == handle->pending_nav_entry_id(); 777 pending_entry_->GetUniqueID() == handle->pending_nav_entry_id();
778 } 778 }
779 779
780 bool NavigationControllerImpl::RendererDidNavigate( 780 bool NavigationControllerImpl::RendererDidNavigate(
781 RenderFrameHostImpl* rfh, 781 RenderFrameHostImpl* rfh,
782 const FrameHostMsg_DidCommitProvisionalLoad_Params& params, 782 const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
783 LoadCommittedDetails* details, 783 LoadCommittedDetails* details,
784 bool is_navigation_within_page) { 784 bool is_navigation_within_page,
785 NavigationHandleImpl* navigation_handle) {
785 is_initial_navigation_ = false; 786 is_initial_navigation_ = false;
786 787
787 // Save the previous state before we clobber it. 788 // Save the previous state before we clobber it.
788 if (GetLastCommittedEntry()) { 789 if (GetLastCommittedEntry()) {
789 details->previous_url = GetLastCommittedEntry()->GetURL(); 790 details->previous_url = GetLastCommittedEntry()->GetURL();
790 details->previous_entry_index = GetLastCommittedEntryIndex(); 791 details->previous_entry_index = GetLastCommittedEntryIndex();
791 } else { 792 } else {
792 details->previous_url = GURL(); 793 details->previous_url = GURL();
793 details->previous_entry_index = -1; 794 details->previous_entry_index = -1;
794 } 795 }
(...skipping 10 matching lines...) Expand all
805 details->did_replace_entry = params.should_replace_current_entry; 806 details->did_replace_entry = params.should_replace_current_entry;
806 807
807 // Do navigation-type specific actions. These will make and commit an entry. 808 // Do navigation-type specific actions. These will make and commit an entry.
808 details->type = ClassifyNavigation(rfh, params); 809 details->type = ClassifyNavigation(rfh, params);
809 810
810 // is_in_page must be computed before the entry gets committed. 811 // is_in_page must be computed before the entry gets committed.
811 details->is_in_page = is_navigation_within_page; 812 details->is_in_page = is_navigation_within_page;
812 813
813 // Save reload type and timestamp for a reload navigation to detect 814 // Save reload type and timestamp for a reload navigation to detect
814 // consecutive reloads when the next reload is requested. 815 // consecutive reloads when the next reload is requested.
815 if (PendingEntryMatchesHandle(rfh->navigation_handle())) { 816 if (PendingEntryMatchesHandle(navigation_handle)) {
816 if (pending_entry_->reload_type() != ReloadType::NONE) { 817 if (pending_entry_->reload_type() != ReloadType::NONE) {
817 last_committed_reload_type_ = pending_entry_->reload_type(); 818 last_committed_reload_type_ = pending_entry_->reload_type();
818 last_committed_reload_time_ = 819 last_committed_reload_time_ =
819 time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run()); 820 time_smoother_.GetSmoothedTime(get_timestamp_callback_.Run());
820 } else if (!pending_entry_->is_renderer_initiated() || 821 } else if (!pending_entry_->is_renderer_initiated() ||
821 params.gesture == NavigationGestureUser) { 822 params.gesture == NavigationGestureUser) {
822 last_committed_reload_type_ = ReloadType::NONE; 823 last_committed_reload_type_ = ReloadType::NONE;
823 last_committed_reload_time_ = base::Time(); 824 last_committed_reload_time_ = base::Time();
824 } 825 }
825 } 826 }
826 827
827 switch (details->type) { 828 switch (details->type) {
828 case NAVIGATION_TYPE_NEW_PAGE: 829 case NAVIGATION_TYPE_NEW_PAGE:
829 RendererDidNavigateToNewPage(rfh, params, details->is_in_page, 830 RendererDidNavigateToNewPage(rfh, params, details->is_in_page,
830 details->did_replace_entry); 831 details->did_replace_entry,
832 navigation_handle);
831 break; 833 break;
832 case NAVIGATION_TYPE_EXISTING_PAGE: 834 case NAVIGATION_TYPE_EXISTING_PAGE:
833 details->did_replace_entry = details->is_in_page; 835 details->did_replace_entry = details->is_in_page;
834 RendererDidNavigateToExistingPage(rfh, params, details->is_in_page); 836 RendererDidNavigateToExistingPage(rfh, params, details->is_in_page,
837 navigation_handle);
835 break; 838 break;
836 case NAVIGATION_TYPE_SAME_PAGE: 839 case NAVIGATION_TYPE_SAME_PAGE:
837 RendererDidNavigateToSamePage(rfh, params); 840 RendererDidNavigateToSamePage(rfh, params, navigation_handle);
838 break; 841 break;
839 case NAVIGATION_TYPE_NEW_SUBFRAME: 842 case NAVIGATION_TYPE_NEW_SUBFRAME:
840 RendererDidNavigateNewSubframe(rfh, params, details->is_in_page, 843 RendererDidNavigateNewSubframe(rfh, params, details->is_in_page,
841 details->did_replace_entry); 844 details->did_replace_entry);
842 break; 845 break;
843 case NAVIGATION_TYPE_AUTO_SUBFRAME: 846 case NAVIGATION_TYPE_AUTO_SUBFRAME:
844 if (!RendererDidNavigateAutoSubframe(rfh, params)) { 847 if (!RendererDidNavigateAutoSubframe(rfh, params)) {
845 // In UseSubframeNavigationEntries mode, we won't send a notification 848 // In UseSubframeNavigationEntries mode, we won't send a notification
846 // about auto-subframe PageState during UpdateStateForFrame, since it 849 // about auto-subframe PageState during UpdateStateForFrame, since it
847 // looks like nothing has changed. Send it here at commit time instead. 850 // looks like nothing has changed. Send it here at commit time instead.
(...skipping 208 matching lines...) Expand 10 before | Expand all | Expand 10 after
1056 1059
1057 // Since we weeded out "new" navigations above, we know this is an existing 1060 // Since we weeded out "new" navigations above, we know this is an existing
1058 // (back/forward) navigation. 1061 // (back/forward) navigation.
1059 return NAVIGATION_TYPE_EXISTING_PAGE; 1062 return NAVIGATION_TYPE_EXISTING_PAGE;
1060 } 1063 }
1061 1064
1062 void NavigationControllerImpl::RendererDidNavigateToNewPage( 1065 void NavigationControllerImpl::RendererDidNavigateToNewPage(
1063 RenderFrameHostImpl* rfh, 1066 RenderFrameHostImpl* rfh,
1064 const FrameHostMsg_DidCommitProvisionalLoad_Params& params, 1067 const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
1065 bool is_in_page, 1068 bool is_in_page,
1066 bool replace_entry) { 1069 bool replace_entry,
1070 NavigationHandleImpl* handle) {
1067 std::unique_ptr<NavigationEntryImpl> new_entry; 1071 std::unique_ptr<NavigationEntryImpl> new_entry;
1068 bool update_virtual_url = false; 1072 bool update_virtual_url = false;
1069 1073
1070 // First check if this is an in-page navigation. If so, clone the current 1074 // First check if this is an in-page navigation. If so, clone the current
1071 // entry instead of looking at the pending entry, because the pending entry 1075 // entry instead of looking at the pending entry, because the pending entry
1072 // does not have any subframe history items. 1076 // does not have any subframe history items.
1073 if (is_in_page && GetLastCommittedEntry()) { 1077 if (is_in_page && GetLastCommittedEntry()) {
1074 FrameNavigationEntry* frame_entry = new FrameNavigationEntry( 1078 FrameNavigationEntry* frame_entry = new FrameNavigationEntry(
1075 params.frame_unique_name, params.item_sequence_number, 1079 params.frame_unique_name, params.item_sequence_number,
1076 params.document_sequence_number, rfh->GetSiteInstance(), nullptr, 1080 params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
1077 params.url, params.referrer, params.method, params.post_id); 1081 params.url, params.referrer, params.method, params.post_id);
1078 new_entry = GetLastCommittedEntry()->CloneAndReplace( 1082 new_entry = GetLastCommittedEntry()->CloneAndReplace(
1079 frame_entry, true, rfh->frame_tree_node(), 1083 frame_entry, true, rfh->frame_tree_node(),
1080 delegate_->GetFrameTree()->root()); 1084 delegate_->GetFrameTree()->root());
1081 1085
1082 // We expect |frame_entry| to be owned by |new_entry|. This should never 1086 // We expect |frame_entry| to be owned by |new_entry|. This should never
1083 // fail, because it's the main frame. 1087 // fail, because it's the main frame.
1084 CHECK(frame_entry->HasOneRef()); 1088 CHECK(frame_entry->HasOneRef());
1085 1089
1086 update_virtual_url = new_entry->update_virtual_url_with_url(); 1090 update_virtual_url = new_entry->update_virtual_url_with_url();
1087 } 1091 }
1088 1092
1089 // Only make a copy of the pending entry if it is appropriate for the new page 1093 // Only make a copy of the pending entry if it is appropriate for the new page
1090 // that was just loaded. Verify this by checking if the entry corresponds 1094 // that was just loaded. Verify this by checking if the entry corresponds
1091 // to the current navigation handle. Note that in some tests the render frame 1095 // to the given navigation handle. Additionally, coarsely check that:
1092 // host does not have a valid handle. Additionally, coarsely check that:
1093 // 1. The SiteInstance hasn't been assigned to something else. 1096 // 1. The SiteInstance hasn't been assigned to something else.
1094 // 2. The pending entry was intended as a new entry, rather than being a 1097 // 2. The pending entry was intended as a new entry, rather than being a
1095 // history navigation that was interrupted by an unrelated, 1098 // history navigation that was interrupted by an unrelated,
1096 // renderer-initiated navigation. 1099 // renderer-initiated navigation.
1097 // TODO(csharrison): Investigate whether we can remove some of the coarser 1100 // TODO(csharrison): Investigate whether we can remove some of the coarser
1098 // checks. 1101 // checks.
1099 NavigationHandleImpl* handle = rfh->navigation_handle();
1100 DCHECK(handle);
1101
1102 if (!new_entry && 1102 if (!new_entry &&
1103 PendingEntryMatchesHandle(handle) && pending_entry_index_ == -1 && 1103 PendingEntryMatchesHandle(handle) && pending_entry_index_ == -1 &&
1104 (!pending_entry_->site_instance() || 1104 (!pending_entry_->site_instance() ||
1105 pending_entry_->site_instance() == rfh->GetSiteInstance())) { 1105 pending_entry_->site_instance() == rfh->GetSiteInstance())) {
1106 new_entry = pending_entry_->Clone(); 1106 new_entry = pending_entry_->Clone();
1107 1107
1108 update_virtual_url = new_entry->update_virtual_url_with_url(); 1108 update_virtual_url = new_entry->update_virtual_url_with_url();
1109 new_entry->GetSSL() = handle->ssl_status(); 1109 new_entry->GetSSL() = handle->ssl_status();
1110 } 1110 }
1111 1111
(...skipping 59 matching lines...) Expand 10 before | Expand all | Expand 10 after
1171 entries_.clear(); 1171 entries_.clear();
1172 last_committed_entry_index_ = -1; 1172 last_committed_entry_index_ = -1;
1173 } 1173 }
1174 1174
1175 InsertOrReplaceEntry(std::move(new_entry), replace_entry); 1175 InsertOrReplaceEntry(std::move(new_entry), replace_entry);
1176 } 1176 }
1177 1177
1178 void NavigationControllerImpl::RendererDidNavigateToExistingPage( 1178 void NavigationControllerImpl::RendererDidNavigateToExistingPage(
1179 RenderFrameHostImpl* rfh, 1179 RenderFrameHostImpl* rfh,
1180 const FrameHostMsg_DidCommitProvisionalLoad_Params& params, 1180 const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
1181 bool is_in_page) { 1181 bool is_in_page,
1182 NavigationHandleImpl* handle) {
1182 // We should only get here for main frame navigations. 1183 // We should only get here for main frame navigations.
1183 DCHECK(!rfh->GetParent()); 1184 DCHECK(!rfh->GetParent());
1184 1185
1185 // TODO(creis): Classify location.replace as NEW_PAGE instead of EXISTING_PAGE 1186 // TODO(creis): Classify location.replace as NEW_PAGE instead of EXISTING_PAGE
1186 // in https://crbug.com/596707. 1187 // in https://crbug.com/596707.
1187 1188
1188 NavigationEntryImpl* entry; 1189 NavigationEntryImpl* entry;
1189 NavigationHandleImpl* handle = rfh->navigation_handle();
1190 if (params.intended_as_new_entry) { 1190 if (params.intended_as_new_entry) {
1191 // This was intended as a new entry but the pending entry was lost in the 1191 // This was intended as a new entry but the pending entry was lost in the
1192 // meanwhile and no new page was created. We are stuck at the last committed 1192 // meanwhile and no new page was created. We are stuck at the last committed
1193 // entry. 1193 // entry.
1194 entry = GetLastCommittedEntry(); 1194 entry = GetLastCommittedEntry();
1195 } else if (params.nav_entry_id) { 1195 } else if (params.nav_entry_id) {
1196 // This is a browser-initiated navigation (back/forward/reload). 1196 // This is a browser-initiated navigation (back/forward/reload).
1197 entry = GetEntryWithUniqueID(params.nav_entry_id); 1197 entry = GetEntryWithUniqueID(params.nav_entry_id);
1198 1198
1199 // Needed for the restore case, where the serialized NavigationEntry doesn't 1199 // Needed for the restore case, where the serialized NavigationEntry doesn't
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
1252 // actually change any other state, just kill the pointer. 1252 // actually change any other state, just kill the pointer.
1253 DiscardNonCommittedEntriesInternal(); 1253 DiscardNonCommittedEntriesInternal();
1254 1254
1255 // If a transient entry was removed, the indices might have changed, so we 1255 // If a transient entry was removed, the indices might have changed, so we
1256 // have to query the entry index again. 1256 // have to query the entry index again.
1257 last_committed_entry_index_ = GetIndexOfEntry(entry); 1257 last_committed_entry_index_ = GetIndexOfEntry(entry);
1258 } 1258 }
1259 1259
1260 void NavigationControllerImpl::RendererDidNavigateToSamePage( 1260 void NavigationControllerImpl::RendererDidNavigateToSamePage(
1261 RenderFrameHostImpl* rfh, 1261 RenderFrameHostImpl* rfh,
1262 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) { 1262 const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
1263 NavigationHandleImpl* handle) {
1263 // This classification says that we have a pending entry that's the same as 1264 // This classification says that we have a pending entry that's the same as
1264 // the last committed entry. This entry is guaranteed to exist by 1265 // the last committed entry. This entry is guaranteed to exist by
1265 // ClassifyNavigation. All we need to do is update the existing entry. 1266 // ClassifyNavigation. All we need to do is update the existing entry.
1266 NavigationEntryImpl* existing_entry = GetLastCommittedEntry(); 1267 NavigationEntryImpl* existing_entry = GetLastCommittedEntry();
1267 1268
1268 // If we classified this correctly, the SiteInstance should not have changed. 1269 // If we classified this correctly, the SiteInstance should not have changed.
1269 CHECK_EQ(existing_entry->site_instance(), rfh->GetSiteInstance()); 1270 CHECK_EQ(existing_entry->site_instance(), rfh->GetSiteInstance());
1270 1271
1271 // We assign the entry's unique ID to be that of the new one. Since this is 1272 // We assign the entry's unique ID to be that of the new one. Since this is
1272 // always the result of a user action, we want to dismiss infobars, etc. like 1273 // always the result of a user action, we want to dismiss infobars, etc. like
1273 // a regular user-initiated navigation. 1274 // a regular user-initiated navigation.
1274 DCHECK_EQ(pending_entry_->GetUniqueID(), params.nav_entry_id); 1275 DCHECK_EQ(pending_entry_->GetUniqueID(), params.nav_entry_id);
1275 existing_entry->set_unique_id(pending_entry_->GetUniqueID()); 1276 existing_entry->set_unique_id(pending_entry_->GetUniqueID());
1276 1277
1277 // The URL may have changed due to redirects. 1278 // The URL may have changed due to redirects.
1278 existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR 1279 existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR
1279 : PAGE_TYPE_NORMAL); 1280 : PAGE_TYPE_NORMAL);
1280 if (existing_entry->update_virtual_url_with_url()) 1281 if (existing_entry->update_virtual_url_with_url())
1281 UpdateVirtualURLToURL(existing_entry, params.url); 1282 UpdateVirtualURLToURL(existing_entry, params.url);
1282 existing_entry->SetURL(params.url); 1283 existing_entry->SetURL(params.url);
1283 1284
1284 // If a user presses enter in the omnibox and the server redirects, the URL 1285 // If a user presses enter in the omnibox and the server redirects, the URL
1285 // might change (but it's still considered a SAME_PAGE navigation). So we must 1286 // might change (but it's still considered a SAME_PAGE navigation). So we must
1286 // update the SSL status. 1287 // update the SSL status.
1287 existing_entry->GetSSL() = rfh->navigation_handle()->ssl_status(); 1288 existing_entry->GetSSL() = handle->ssl_status();
1288 1289
1289 // The extra headers may have changed due to reloading with different headers. 1290 // The extra headers may have changed due to reloading with different headers.
1290 existing_entry->set_extra_headers(pending_entry_->extra_headers()); 1291 existing_entry->set_extra_headers(pending_entry_->extra_headers());
1291 1292
1292 // Update the existing FrameNavigationEntry to ensure all of its members 1293 // Update the existing FrameNavigationEntry to ensure all of its members
1293 // reflect the parameters coming from the renderer process. 1294 // reflect the parameters coming from the renderer process.
1294 existing_entry->AddOrUpdateFrameEntry( 1295 existing_entry->AddOrUpdateFrameEntry(
1295 rfh->frame_tree_node(), params.item_sequence_number, 1296 rfh->frame_tree_node(), params.item_sequence_number,
1296 params.document_sequence_number, rfh->GetSiteInstance(), nullptr, 1297 params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
1297 params.url, params.referrer, params.redirects, params.page_state, 1298 params.url, params.referrer, params.redirects, params.page_state,
(...skipping 802 matching lines...) Expand 10 before | Expand all | Expand 10 after
2100 } 2101 }
2101 } 2102 }
2102 } 2103 }
2103 2104
2104 void NavigationControllerImpl::SetGetTimestampCallbackForTest( 2105 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
2105 const base::Callback<base::Time()>& get_timestamp_callback) { 2106 const base::Callback<base::Time()>& get_timestamp_callback) {
2106 get_timestamp_callback_ = get_timestamp_callback; 2107 get_timestamp_callback_ = get_timestamp_callback;
2107 } 2108 }
2108 2109
2109 } // namespace content 2110 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698