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

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

Issue 2368183004: Move redirect_chain from NavigationEntry to FrameNavigationEntry. (Closed)
Patch Set: Add tests + bugfix. Created 4 years, 2 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
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 709 matching lines...) Expand 10 before | Expand all | Expand 10 after
720 frame_tree_node_id = node->frame_tree_node_id(); 720 frame_tree_node_id = node->frame_tree_node_id();
721 721
722 // In --site-per-process, create an identical NavigationEntry with a 722 // In --site-per-process, create an identical NavigationEntry with a
723 // new FrameNavigationEntry for the target subframe. 723 // new FrameNavigationEntry for the target subframe.
724 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { 724 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
725 entry = GetLastCommittedEntry()->Clone(); 725 entry = GetLastCommittedEntry()->Clone();
726 entry->SetPageID(-1); 726 entry->SetPageID(-1);
727 entry->AddOrUpdateFrameEntry( 727 entry->AddOrUpdateFrameEntry(
728 node, -1, -1, nullptr, 728 node, -1, -1, nullptr,
729 static_cast<SiteInstanceImpl*>(params.source_site_instance.get()), 729 static_cast<SiteInstanceImpl*>(params.source_site_instance.get()),
730 params.url, params.referrer, PageState(), "GET", -1); 730 params.url, params.referrer, params.redirect_chain, PageState(),
731 "GET", -1);
731 } 732 }
732 } 733 }
733 } 734 }
734 735
735 // Otherwise, create a pending entry for the main frame. 736 // Otherwise, create a pending entry for the main frame.
736 if (!entry) { 737 if (!entry) {
737 entry = NavigationEntryImpl::FromNavigationEntry(CreateNavigationEntry( 738 entry = NavigationEntryImpl::FromNavigationEntry(CreateNavigationEntry(
738 params.url, params.referrer, params.transition_type, 739 params.url, params.referrer, params.transition_type,
739 params.is_renderer_initiated, params.extra_headers, browser_context_)); 740 params.is_renderer_initiated, params.extra_headers, browser_context_));
740 entry->set_source_site_instance( 741 entry->set_source_site_instance(
741 static_cast<SiteInstanceImpl*>(params.source_site_instance.get())); 742 static_cast<SiteInstanceImpl*>(params.source_site_instance.get()));
743 entry->SetRedirectChain(params.redirect_chain);
742 } 744 }
743 745
744 // Set the FTN ID (only used in non-site-per-process, for tests). 746 // Set the FTN ID (only used in non-site-per-process, for tests).
745 entry->set_frame_tree_node_id(frame_tree_node_id); 747 entry->set_frame_tree_node_id(frame_tree_node_id);
746 if (params.redirect_chain.size() > 0)
747 entry->SetRedirectChain(params.redirect_chain);
748 // Don't allow an entry replacement if there is no entry to replace. 748 // Don't allow an entry replacement if there is no entry to replace.
749 // http://crbug.com/457149 749 // http://crbug.com/457149
750 if (params.should_replace_current_entry && entries_.size() > 0) 750 if (params.should_replace_current_entry && entries_.size() > 0)
751 entry->set_should_replace_entry(true); 751 entry->set_should_replace_entry(true);
752 entry->set_should_clear_history_list(params.should_clear_history_list); 752 entry->set_should_clear_history_list(params.should_clear_history_list);
753 entry->SetIsOverridingUserAgent(override); 753 entry->SetIsOverridingUserAgent(override);
754 entry->set_transferred_global_request_id( 754 entry->set_transferred_global_request_id(
755 params.transferred_global_request_id); 755 params.transferred_global_request_id);
756 756
757 #if defined(OS_ANDROID) 757 #if defined(OS_ANDROID)
(...skipping 143 matching lines...) Expand 10 before | Expand all | Expand 10 after
901 base::debug::DumpWithoutCrashing(); 901 base::debug::DumpWithoutCrashing();
902 NOTREACHED() << "Shouldn't see an empty PageState at commit."; 902 NOTREACHED() << "Shouldn't see an empty PageState at commit.";
903 } 903 }
904 NavigationEntryImpl* active_entry = GetLastCommittedEntry(); 904 NavigationEntryImpl* active_entry = GetLastCommittedEntry();
905 active_entry->SetTimestamp(timestamp); 905 active_entry->SetTimestamp(timestamp);
906 active_entry->SetHttpStatusCode(params.http_status_code); 906 active_entry->SetHttpStatusCode(params.http_status_code);
907 907
908 FrameNavigationEntry* frame_entry = 908 FrameNavigationEntry* frame_entry =
909 active_entry->GetFrameEntry(rfh->frame_tree_node()); 909 active_entry->GetFrameEntry(rfh->frame_tree_node());
910 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { 910 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
911 // Update the frame-specific PageState. 911 // Update the frame-specific PageState and RedirectChain
912 // We may not find a frame_entry in some cases; ignore the PageState if so. 912 // We may not find a frame_entry in some cases; ignore the PageState if so.
913 // TODO(creis): Remove the "if" once https://crbug.com/522193 is fixed. 913 // TODO(creis): Remove the "if" once https://crbug.com/522193 is fixed.
914 if (frame_entry) 914 if (frame_entry) {
915 frame_entry->SetPageState(params.page_state); 915 frame_entry->SetPageState(params.page_state);
916 frame_entry->set_redirect_chain(params.redirects);
917 }
916 } else { 918 } else {
917 active_entry->SetPageState(params.page_state); 919 active_entry->SetPageState(params.page_state);
920 active_entry->SetRedirectChain(params.redirects);
918 } 921 }
919 active_entry->SetRedirectChain(params.redirects);
920 922
921 // Use histogram to track memory impact of redirect chain because it's now 923 // Use histogram to track memory impact of redirect chain because it's now
922 // not cleared for committed entries. 924 // not cleared for committed entries.
923 size_t redirect_chain_size = 0; 925 size_t redirect_chain_size = 0;
924 for (size_t i = 0; i < params.redirects.size(); ++i) { 926 for (size_t i = 0; i < params.redirects.size(); ++i) {
925 redirect_chain_size += params.redirects[i].spec().length(); 927 redirect_chain_size += params.redirects[i].spec().length();
926 } 928 }
927 UMA_HISTOGRAM_COUNTS("Navigation.RedirectChainSize", redirect_chain_size); 929 UMA_HISTOGRAM_COUNTS("Navigation.RedirectChainSize", redirect_chain_size);
928 930
929 // Once it is committed, we no longer need to track several pieces of state on 931 // Once it is committed, we no longer need to track several pieces of state on
(...skipping 311 matching lines...) Expand 10 before | Expand all | Expand 10 after
1241 // The site instance will normally be the same except during session restore, 1243 // The site instance will normally be the same except during session restore,
1242 // when no site instance will be assigned. 1244 // when no site instance will be assigned.
1243 DCHECK(entry->site_instance() == nullptr || 1245 DCHECK(entry->site_instance() == nullptr ||
1244 entry->site_instance() == rfh->GetSiteInstance()); 1246 entry->site_instance() == rfh->GetSiteInstance());
1245 1247
1246 // Update the existing FrameNavigationEntry to ensure all of its members 1248 // Update the existing FrameNavigationEntry to ensure all of its members
1247 // reflect the parameters coming from the renderer process. 1249 // reflect the parameters coming from the renderer process.
1248 entry->AddOrUpdateFrameEntry( 1250 entry->AddOrUpdateFrameEntry(
1249 rfh->frame_tree_node(), params.item_sequence_number, 1251 rfh->frame_tree_node(), params.item_sequence_number,
1250 params.document_sequence_number, rfh->GetSiteInstance(), nullptr, 1252 params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
1251 params.url, params.referrer, params.page_state, params.method, 1253 params.url, params.referrer, params.redirects, params.page_state,
1252 params.post_id); 1254 params.method, params.post_id);
1253 1255
1254 // The redirected to page should not inherit the favicon from the previous 1256 // The redirected to page should not inherit the favicon from the previous
1255 // page. 1257 // page.
1256 if (ui::PageTransitionIsRedirect(params.transition) && !is_in_page) 1258 if (ui::PageTransitionIsRedirect(params.transition) && !is_in_page)
1257 entry->GetFavicon() = FaviconStatus(); 1259 entry->GetFavicon() = FaviconStatus();
1258 1260
1259 // The entry we found in the list might be pending if the user hit 1261 // The entry we found in the list might be pending if the user hit
1260 // back/forward/reload. This load should commit it (since it's already in the 1262 // back/forward/reload. This load should commit it (since it's already in the
1261 // list, we can just discard the pending pointer). We should also discard the 1263 // list, we can just discard the pending pointer). We should also discard the
1262 // pending entry if it corresponds to a different navigation, since that one 1264 // pending entry if it corresponds to a different navigation, since that one
(...skipping 36 matching lines...) Expand 10 before | Expand all | Expand 10 after
1299 // If a user presses enter in the omnibox and the server redirects, the URL 1301 // If a user presses enter in the omnibox and the server redirects, the URL
1300 // might change (but it's still considered a SAME_PAGE navigation). So we must 1302 // might change (but it's still considered a SAME_PAGE navigation). So we must
1301 // update the SSL status. 1303 // update the SSL status.
1302 existing_entry->GetSSL() = rfh->navigation_handle()->ssl_status(); 1304 existing_entry->GetSSL() = rfh->navigation_handle()->ssl_status();
1303 1305
1304 // Update the existing FrameNavigationEntry to ensure all of its members 1306 // Update the existing FrameNavigationEntry to ensure all of its members
1305 // reflect the parameters coming from the renderer process. 1307 // reflect the parameters coming from the renderer process.
1306 existing_entry->AddOrUpdateFrameEntry( 1308 existing_entry->AddOrUpdateFrameEntry(
1307 rfh->frame_tree_node(), params.item_sequence_number, 1309 rfh->frame_tree_node(), params.item_sequence_number,
1308 params.document_sequence_number, rfh->GetSiteInstance(), nullptr, 1310 params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
1309 params.url, params.referrer, params.page_state, params.method, 1311 params.url, params.referrer, params.redirects, params.page_state,
1310 params.post_id); 1312 params.method, params.post_id);
1311 1313
1312 DiscardNonCommittedEntries(); 1314 DiscardNonCommittedEntries();
1313 } 1315 }
1314 1316
1315 void NavigationControllerImpl::RendererDidNavigateNewSubframe( 1317 void NavigationControllerImpl::RendererDidNavigateNewSubframe(
1316 RenderFrameHostImpl* rfh, 1318 RenderFrameHostImpl* rfh,
1317 const FrameHostMsg_DidCommitProvisionalLoad_Params& params, 1319 const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
1318 bool is_in_page, 1320 bool is_in_page,
1319 bool replace_entry) { 1321 bool replace_entry) {
1320 DCHECK(ui::PageTransitionCoreTypeIs(params.transition, 1322 DCHECK(ui::PageTransitionCoreTypeIs(params.transition,
(...skipping 77 matching lines...) Expand 10 before | Expand all | Expand 10 after
1398 } 1400 }
1399 } 1401 }
1400 1402
1401 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { 1403 if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
1402 // This may be a "new auto" case where we add a new FrameNavigationEntry, or 1404 // This may be a "new auto" case where we add a new FrameNavigationEntry, or
1403 // it may be a "history auto" case where we update an existing one. 1405 // it may be a "history auto" case where we update an existing one.
1404 NavigationEntryImpl* last_committed = GetLastCommittedEntry(); 1406 NavigationEntryImpl* last_committed = GetLastCommittedEntry();
1405 last_committed->AddOrUpdateFrameEntry( 1407 last_committed->AddOrUpdateFrameEntry(
1406 rfh->frame_tree_node(), params.item_sequence_number, 1408 rfh->frame_tree_node(), params.item_sequence_number,
1407 params.document_sequence_number, rfh->GetSiteInstance(), nullptr, 1409 params.document_sequence_number, rfh->GetSiteInstance(), nullptr,
1408 params.url, params.referrer, params.page_state, params.method, 1410 params.url, params.referrer, params.redirects, params.page_state,
1409 params.post_id); 1411 params.method, params.post_id);
1410 } 1412 }
1411 1413
1412 return send_commit_notification; 1414 return send_commit_notification;
1413 } 1415 }
1414 1416
1415 int NavigationControllerImpl::GetIndexOfEntry( 1417 int NavigationControllerImpl::GetIndexOfEntry(
1416 const NavigationEntryImpl* entry) const { 1418 const NavigationEntryImpl* entry) const {
1417 for (size_t i = 0; i < entries_.size(); ++i) { 1419 for (size_t i = 0; i < entries_.size(); ++i) {
1418 if (entries_[i].get() == entry) 1420 if (entries_[i].get() == entry)
1419 return i; 1421 return i;
(...skipping 722 matching lines...) Expand 10 before | Expand all | Expand 10 after
2142 } 2144 }
2143 } 2145 }
2144 } 2146 }
2145 2147
2146 void NavigationControllerImpl::SetGetTimestampCallbackForTest( 2148 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
2147 const base::Callback<base::Time()>& get_timestamp_callback) { 2149 const base::Callback<base::Time()>& get_timestamp_callback) {
2148 get_timestamp_callback_ = get_timestamp_callback; 2150 get_timestamp_callback_ = get_timestamp_callback;
2149 } 2151 }
2150 2152
2151 } // namespace content 2153 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698