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

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

Issue 1148953014: Fix the commit type for out-of-process iframes. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase for classifier revert Created 5 years, 6 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 #include "content/browser/frame_host/navigation_controller_impl.h" 5 #include "content/browser/frame_host/navigation_controller_impl.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/command_line.h" 8 #include "base/command_line.h"
9 #include "base/logging.h" 9 #include "base/logging.h"
10 #include "base/metrics/histogram.h" 10 #include "base/metrics/histogram.h"
(...skipping 403 matching lines...) Expand 10 before | Expand all | Expand 10 after
414 bool NavigationControllerImpl::IsInitialNavigation() const { 414 bool NavigationControllerImpl::IsInitialNavigation() const {
415 return is_initial_navigation_; 415 return is_initial_navigation_;
416 } 416 }
417 417
418 NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID( 418 NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID(
419 SiteInstance* instance, int32 page_id) const { 419 SiteInstance* instance, int32 page_id) const {
420 int index = GetEntryIndexWithPageID(instance, page_id); 420 int index = GetEntryIndexWithPageID(instance, page_id);
421 return (index != -1) ? entries_[index].get() : NULL; 421 return (index != -1) ? entries_[index].get() : NULL;
422 } 422 }
423 423
424 bool NavigationControllerImpl::HasCommittedRealLoad(
425 FrameTreeNode* frame_tree_node) const {
426 NavigationEntryImpl* last_committed = GetLastCommittedEntry();
427 return last_committed && last_committed->HasFrameEntry(frame_tree_node);
428 }
429
424 void NavigationControllerImpl::LoadEntry(NavigationEntryImpl* entry) { 430 void NavigationControllerImpl::LoadEntry(NavigationEntryImpl* entry) {
425 // When navigating to a new page, we don't know for sure if we will actually 431 // When navigating to a new page, we don't know for sure if we will actually
426 // end up leaving the current page. The new page load could for example 432 // end up leaving the current page. The new page load could for example
427 // result in a download or a 'no content' response (e.g., a mailto: URL). 433 // result in a download or a 'no content' response (e.g., a mailto: URL).
428 SetPendingEntry(entry); 434 SetPendingEntry(entry);
429 NavigateToPendingEntry(NO_RELOAD); 435 NavigateToPendingEntry(NO_RELOAD);
430 } 436 }
431 437
432 void NavigationControllerImpl::SetPendingEntry(NavigationEntryImpl* entry) { 438 void NavigationControllerImpl::SetPendingEntry(NavigationEntryImpl* entry) {
433 DiscardNonCommittedEntriesInternal(); 439 DiscardNonCommittedEntriesInternal();
(...skipping 392 matching lines...) Expand 10 before | Expand all | Expand 10 after
826 NavigationType new_type = ClassifyNavigationWithoutPageID(rfh, params); 832 NavigationType new_type = ClassifyNavigationWithoutPageID(rfh, params);
827 bool ignore_mismatch = false; 833 bool ignore_mismatch = false;
828 // There are disagreements on some Android bots over SAME_PAGE between the two 834 // There are disagreements on some Android bots over SAME_PAGE between the two
829 // classifiers so ignore disagreements if that's the case. 835 // classifiers so ignore disagreements if that's the case.
830 ignore_mismatch |= details->type == NAVIGATION_TYPE_EXISTING_PAGE && 836 ignore_mismatch |= details->type == NAVIGATION_TYPE_EXISTING_PAGE &&
831 new_type == NAVIGATION_TYPE_SAME_PAGE; 837 new_type == NAVIGATION_TYPE_SAME_PAGE;
832 ignore_mismatch |= details->type == NAVIGATION_TYPE_SAME_PAGE && 838 ignore_mismatch |= details->type == NAVIGATION_TYPE_SAME_PAGE &&
833 new_type == NAVIGATION_TYPE_EXISTING_PAGE; 839 new_type == NAVIGATION_TYPE_EXISTING_PAGE;
834 if (base::CommandLine::ForCurrentProcess()->HasSwitch( 840 if (base::CommandLine::ForCurrentProcess()->HasSwitch(
835 switches::kSitePerProcess)) { 841 switches::kSitePerProcess)) {
836 // In site-per-process mode, the new classifier sometimes correctly 842 // We know that the old classifier is wrong for OOPIFs, so use the new one
837 // classifies things as auto-subframe where the old classifier incorrectly 843 // in --site-per-process mode.
838 // ignored them or called them NEW_SUBFRAME. 844 details->type = new_type;
839 ignore_mismatch |= details->type == NAVIGATION_TYPE_NAV_IGNORE && 845 ignore_mismatch = true;
840 new_type == NAVIGATION_TYPE_AUTO_SUBFRAME;
841 ignore_mismatch |= details->type == NAVIGATION_TYPE_NEW_SUBFRAME &&
842 new_type == NAVIGATION_TYPE_AUTO_SUBFRAME;
843 } 846 }
844 if (!ignore_mismatch) { 847 if (!ignore_mismatch) {
845 DCHECK_EQ(details->type, new_type); 848 DCHECK_EQ(details->type, new_type);
846 } 849 }
847 #endif // DCHECK_IS_ON() 850 #endif // DCHECK_IS_ON()
848 851
849 // is_in_page must be computed before the entry gets committed. 852 // is_in_page must be computed before the entry gets committed.
850 details->is_in_page = AreURLsInPageNavigation(rfh->GetLastCommittedURL(), 853 details->is_in_page = AreURLsInPageNavigation(rfh->GetLastCommittedURL(),
851 params.url, params.was_within_same_page, rfh); 854 params.url, params.was_within_same_page, rfh);
852 855
(...skipping 83 matching lines...) Expand 10 before | Expand all | Expand 10 after
936 details->serialized_security_info = params.security_info; 939 details->serialized_security_info = params.security_info;
937 details->http_status_code = params.http_status_code; 940 details->http_status_code = params.http_status_code;
938 NotifyNavigationEntryCommitted(details); 941 NotifyNavigationEntryCommitted(details);
939 942
940 return true; 943 return true;
941 } 944 }
942 945
943 NavigationType NavigationControllerImpl::ClassifyNavigation( 946 NavigationType NavigationControllerImpl::ClassifyNavigation(
944 RenderFrameHostImpl* rfh, 947 RenderFrameHostImpl* rfh,
945 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { 948 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const {
946 // Hack; remove once http://crbug.com/464014 is fixed.
947 if (rfh->IsCrossProcessSubframe())
948 return NAVIGATION_TYPE_NEW_SUBFRAME;
949
950 if (params.page_id == -1) { 949 if (params.page_id == -1) {
951 // The renderer generates the page IDs, and so if it gives us the invalid 950 // The renderer generates the page IDs, and so if it gives us the invalid
952 // page ID (-1) we know it didn't actually navigate. This happens in a few 951 // page ID (-1) we know it didn't actually navigate. This happens in a few
953 // cases: 952 // cases:
954 // 953 //
955 // - If a page makes a popup navigated to about blank, and then writes 954 // - If a page makes a popup navigated to about blank, and then writes
956 // stuff like a subframe navigated to a real page. We'll get the commit 955 // stuff like a subframe navigated to a real page. We'll get the commit
957 // for the subframe, but there won't be any commit for the outer page. 956 // for the subframe, but there won't be any commit for the outer page.
958 // 957 //
959 // - We were also getting these for failed loads (for example, bug 21849). 958 // - We were also getting these for failed loads (for example, bug 21849).
(...skipping 129 matching lines...) Expand 10 before | Expand all | Expand 10 after
1089 } 1088 }
1090 1089
1091 // Since we weeded out "new" navigations above, we know this is an existing 1090 // Since we weeded out "new" navigations above, we know this is an existing
1092 // (back/forward) navigation. 1091 // (back/forward) navigation.
1093 return NAVIGATION_TYPE_EXISTING_PAGE; 1092 return NAVIGATION_TYPE_EXISTING_PAGE;
1094 } 1093 }
1095 1094
1096 NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID( 1095 NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
1097 RenderFrameHostImpl* rfh, 1096 RenderFrameHostImpl* rfh,
1098 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { 1097 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const {
1099 // Hack; remove once http://crbug.com/464014 is fixed.
1100 if (rfh->IsCrossProcessSubframe())
1101 return NAVIGATION_TYPE_NEW_SUBFRAME;
1102
1103 if (params.did_create_new_entry) { 1098 if (params.did_create_new_entry) {
1104 // A new entry. We may or may not have a pending entry for the page, and 1099 // A new entry. We may or may not have a pending entry for the page, and
1105 // this may or may not be the main frame. 1100 // this may or may not be the main frame.
1106 if (ui::PageTransitionIsMainFrame(params.transition)) { 1101 if (ui::PageTransitionIsMainFrame(params.transition)) {
1107 // TODO(avi): I want to use |if (!rfh->GetParent())| here but lots of unit 1102 // TODO(avi): I want to use |if (!rfh->GetParent())| here but lots of unit
1108 // tests fake auto subframe commits by sending the main frame a 1103 // tests fake auto subframe commits by sending the main frame a
1109 // PAGE_TRANSITION_AUTO_SUBFRAME transition. Fix those, and adjust here. 1104 // PAGE_TRANSITION_AUTO_SUBFRAME transition. Fix those, and adjust here.
1110 return NAVIGATION_TYPE_NEW_PAGE; 1105 return NAVIGATION_TYPE_NEW_PAGE;
1111 } 1106 }
1112 1107
(...skipping 330 matching lines...) Expand 10 before | Expand all | Expand 10 after
1443 RenderFrameHostImpl* rfh, 1438 RenderFrameHostImpl* rfh,
1444 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) { 1439 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
1445 DCHECK(ui::PageTransitionCoreTypeIs(params.transition, 1440 DCHECK(ui::PageTransitionCoreTypeIs(params.transition,
1446 ui::PAGE_TRANSITION_AUTO_SUBFRAME)); 1441 ui::PAGE_TRANSITION_AUTO_SUBFRAME));
1447 1442
1448 // We're guaranteed to have a previously committed entry, and we now need to 1443 // We're guaranteed to have a previously committed entry, and we now need to
1449 // handle navigation inside of a subframe in it without creating a new entry. 1444 // handle navigation inside of a subframe in it without creating a new entry.
1450 DCHECK(GetLastCommittedEntry()); 1445 DCHECK(GetLastCommittedEntry());
1451 1446
1452 if (params.nav_entry_id) { 1447 if (params.nav_entry_id) {
1453 // If the |nav_entry_id| is non-zero, this is a browser-initiated navigation 1448 int entry_index = GetEntryIndexWithUniqueID(params.nav_entry_id);
1454 // and is thus a "history auto" navigation. TODO(creis): Implement
1455 // back/forward this way for site-per-process.
1456 1449
1457 int entry_index = GetEntryIndexWithUniqueID(params.nav_entry_id); 1450 // If the |nav_entry_id| is non-zero and matches an existing entry, this is
1458 if (entry_index == -1) { 1451 // a history auto" navigation. Update the last committed index accordingly.
1459 NOTREACHED(); 1452 // If we don't recognize the |nav_entry_id|, it might be either a pending
1460 return false; 1453 // entry for a transfer or a recently pruned entry. We'll handle it below.
1461 } 1454 if (entry_index != -1 && entry_index != last_committed_entry_index_) {
1462
1463 // Update the current navigation entry in case we're going back/forward.
1464 if (entry_index != last_committed_entry_index_) {
1465 // Make sure that a subframe commit isn't changing the main frame's 1455 // Make sure that a subframe commit isn't changing the main frame's
1466 // origin. Otherwise the renderer process may be confused, leading to a 1456 // origin. Otherwise the renderer process may be confused, leading to a
1467 // URL spoof. We can't check the path since that may change 1457 // URL spoof. We can't check the path since that may change
1468 // (https://crbug.com/373041). 1458 // (https://crbug.com/373041).
1469 if (GetLastCommittedEntry()->GetURL().GetOrigin() != 1459 if (GetLastCommittedEntry()->GetURL().GetOrigin() !=
1470 GetEntryAtIndex(entry_index)->GetURL().GetOrigin()) { 1460 GetEntryAtIndex(entry_index)->GetURL().GetOrigin()) {
1471 // TODO(creis): This is unexpectedly being encountered in practice. If 1461 // TODO(creis): This is unexpectedly being encountered in practice. If
1472 // you encounter this in practice, please post details to 1462 // you encounter this in practice, please post details to
1473 // https://crbug.com/486916. Once that's resolved, we'll change this to 1463 // https://crbug.com/486916. Once that's resolved, we'll change this to
1474 // kill the renderer process with bad_message::NC_AUTO_SUBFRAME. 1464 // kill the renderer process with bad_message::NC_AUTO_SUBFRAME.
1475 NOTREACHED() << "Unexpected main frame origin change on AUTO_SUBFRAME."; 1465 NOTREACHED() << "Unexpected main frame origin change on AUTO_SUBFRAME.";
1476 } 1466 }
1467
1468 // TODO(creis): Update the FrameNavigationEntry in --site-per-process.
1477 last_committed_entry_index_ = entry_index; 1469 last_committed_entry_index_ = entry_index;
1478 DiscardNonCommittedEntriesInternal(); 1470 DiscardNonCommittedEntriesInternal();
1479 return true; 1471 return true;
1480 } 1472 }
1481 } 1473 }
1482 1474
1483 if (base::CommandLine::ForCurrentProcess()->HasSwitch( 1475 if (base::CommandLine::ForCurrentProcess()->HasSwitch(
1484 switches::kSitePerProcess)) { 1476 switches::kSitePerProcess)) {
1485 // This may be a "new auto" case where we add a new FrameNavigationEntry, or 1477 // This may be a "new auto" case where we add a new FrameNavigationEntry, or
1486 // it may be a "history auto" case where we update an existing one. 1478 // it may be a "history auto" case where we update an existing one.
1487 NavigationEntryImpl* last_committed = GetLastCommittedEntry(); 1479 NavigationEntryImpl* last_committed = GetLastCommittedEntry();
1488 last_committed->AddOrUpdateFrameEntry(rfh->frame_tree_node(), 1480 last_committed->AddOrUpdateFrameEntry(rfh->frame_tree_node(),
1489 rfh->GetSiteInstance(), params.url, 1481 rfh->GetSiteInstance(), params.url,
1490 params.referrer); 1482 params.referrer);
1483
1484 // Cross-process subframe navigations may leave a pending entry around.
1485 // TODO(creis): Don't use pending entries for subframe navigations.
1486 DiscardNonCommittedEntriesInternal();
1491 } 1487 }
1492 1488
1493 // We do not need to discard the pending entry in this case, since we will 1489 // We do not need to discard the pending entry in this case, since we will
1494 // not generate commit notifications for this auto-subframe navigation. 1490 // not generate commit notifications for this auto-subframe navigation.
1495 return false; 1491 return false;
1496 } 1492 }
1497 1493
1498 int NavigationControllerImpl::GetIndexOfEntry( 1494 int NavigationControllerImpl::GetIndexOfEntry(
1499 const NavigationEntryImpl* entry) const { 1495 const NavigationEntryImpl* entry) const {
1500 const NavigationEntries::const_iterator i(std::find( 1496 const NavigationEntries::const_iterator i(std::find(
(...skipping 528 matching lines...) Expand 10 before | Expand all | Expand 10 after
2029 } 2025 }
2030 } 2026 }
2031 } 2027 }
2032 2028
2033 void NavigationControllerImpl::SetGetTimestampCallbackForTest( 2029 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
2034 const base::Callback<base::Time()>& get_timestamp_callback) { 2030 const base::Callback<base::Time()>& get_timestamp_callback) {
2035 get_timestamp_callback_ = get_timestamp_callback; 2031 get_timestamp_callback_ = get_timestamp_callback;
2036 } 2032 }
2037 2033
2038 } // namespace content 2034 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698