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

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: Fix failing tests 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/debug/crash_logging.h" 9 #include "base/debug/crash_logging.h"
10 #include "base/logging.h" 10 #include "base/logging.h"
(...skipping 404 matching lines...) Expand 10 before | Expand all | Expand 10 after
415 bool NavigationControllerImpl::IsInitialNavigation() const { 415 bool NavigationControllerImpl::IsInitialNavigation() const {
416 return is_initial_navigation_; 416 return is_initial_navigation_;
417 } 417 }
418 418
419 NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID( 419 NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID(
420 SiteInstance* instance, int32 page_id) const { 420 SiteInstance* instance, int32 page_id) const {
421 int index = GetEntryIndexWithPageID(instance, page_id); 421 int index = GetEntryIndexWithPageID(instance, page_id);
422 return (index != -1) ? entries_[index].get() : NULL; 422 return (index != -1) ? entries_[index].get() : NULL;
423 } 423 }
424 424
425 bool NavigationControllerImpl::HasCommittedRealLoad(
426 FrameTreeNode* frame_tree_node) const {
427 NavigationEntryImpl* last_committed = GetLastCommittedEntry();
428 return last_committed && last_committed->HasFrameEntry(frame_tree_node);
429 }
430
425 void NavigationControllerImpl::LoadEntry(NavigationEntryImpl* entry) { 431 void NavigationControllerImpl::LoadEntry(NavigationEntryImpl* entry) {
426 // When navigating to a new page, we don't know for sure if we will actually 432 // When navigating to a new page, we don't know for sure if we will actually
427 // end up leaving the current page. The new page load could for example 433 // end up leaving the current page. The new page load could for example
428 // result in a download or a 'no content' response (e.g., a mailto: URL). 434 // result in a download or a 'no content' response (e.g., a mailto: URL).
429 SetPendingEntry(entry); 435 SetPendingEntry(entry);
430 NavigateToPendingEntry(NO_RELOAD); 436 NavigateToPendingEntry(NO_RELOAD);
431 } 437 }
432 438
433 void NavigationControllerImpl::SetPendingEntry(NavigationEntryImpl* entry) { 439 void NavigationControllerImpl::SetPendingEntry(NavigationEntryImpl* entry) {
434 DiscardNonCommittedEntriesInternal(); 440 DiscardNonCommittedEntriesInternal();
(...skipping 391 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 &&
840 new_type == NAVIGATION_TYPE_AUTO_SUBFRAME;
841 ignore_mismatch |= details->type == NAVIGATION_TYPE_NEW_SUBFRAME &&
842 new_type == NAVIGATION_TYPE_AUTO_SUBFRAME;
843 } 845 }
844 if (!ignore_mismatch) { 846 if (!ignore_mismatch) {
845 base::debug::SetCrashKeyValue("oldtype", base::IntToString(details->type)); 847 base::debug::SetCrashKeyValue("oldtype", base::IntToString(details->type));
846 base::debug::SetCrashKeyValue("newtype", base::IntToString(new_type)); 848 base::debug::SetCrashKeyValue("newtype", base::IntToString(new_type));
847 base::debug::SetCrashKeyValue("navurl", params.url.spec()); 849 base::debug::SetCrashKeyValue("navurl", params.url.spec());
848 if (base::CommandLine::ForCurrentProcess()->HasSwitch( 850 if (base::CommandLine::ForCurrentProcess()->HasSwitch(
849 switches::kSitePerProcess)) { 851 switches::kSitePerProcess)) {
850 base::debug::SetCrashKeyValue("spp", "yes"); 852 base::debug::SetCrashKeyValue("spp", "yes");
851 } 853 }
852 CHECK_EQ(details->type, new_type); 854 CHECK_EQ(details->type, new_type);
(...skipping 89 matching lines...) Expand 10 before | Expand all | Expand 10 after
942 details->serialized_security_info = params.security_info; 944 details->serialized_security_info = params.security_info;
943 details->http_status_code = params.http_status_code; 945 details->http_status_code = params.http_status_code;
944 NotifyNavigationEntryCommitted(details); 946 NotifyNavigationEntryCommitted(details);
945 947
946 return true; 948 return true;
947 } 949 }
948 950
949 NavigationType NavigationControllerImpl::ClassifyNavigation( 951 NavigationType NavigationControllerImpl::ClassifyNavigation(
950 RenderFrameHostImpl* rfh, 952 RenderFrameHostImpl* rfh,
951 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { 953 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const {
952 // Hack; remove once http://crbug.com/464014 is fixed.
953 if (rfh->IsCrossProcessSubframe())
954 return NAVIGATION_TYPE_NEW_SUBFRAME;
955
956 if (params.page_id == -1) { 954 if (params.page_id == -1) {
957 // The renderer generates the page IDs, and so if it gives us the invalid 955 // The renderer generates the page IDs, and so if it gives us the invalid
958 // page ID (-1) we know it didn't actually navigate. This happens in a few 956 // page ID (-1) we know it didn't actually navigate. This happens in a few
959 // cases: 957 // cases:
960 // 958 //
961 // - If a page makes a popup navigated to about blank, and then writes 959 // - If a page makes a popup navigated to about blank, and then writes
962 // stuff like a subframe navigated to a real page. We'll get the commit 960 // stuff like a subframe navigated to a real page. We'll get the commit
963 // for the subframe, but there won't be any commit for the outer page. 961 // for the subframe, but there won't be any commit for the outer page.
964 // 962 //
965 // - We were also getting these for failed loads (for example, bug 21849). 963 // - We were also getting these for failed loads (for example, bug 21849).
(...skipping 129 matching lines...) Expand 10 before | Expand all | Expand 10 after
1095 } 1093 }
1096 1094
1097 // Since we weeded out "new" navigations above, we know this is an existing 1095 // Since we weeded out "new" navigations above, we know this is an existing
1098 // (back/forward) navigation. 1096 // (back/forward) navigation.
1099 return NAVIGATION_TYPE_EXISTING_PAGE; 1097 return NAVIGATION_TYPE_EXISTING_PAGE;
1100 } 1098 }
1101 1099
1102 NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID( 1100 NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID(
1103 RenderFrameHostImpl* rfh, 1101 RenderFrameHostImpl* rfh,
1104 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { 1102 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const {
1105 // Hack; remove once http://crbug.com/464014 is fixed.
1106 if (rfh->IsCrossProcessSubframe())
1107 return NAVIGATION_TYPE_NEW_SUBFRAME;
1108
1109 if (params.did_create_new_entry) { 1103 if (params.did_create_new_entry) {
1110 // A new entry. We may or may not have a pending entry for the page, and 1104 // A new entry. We may or may not have a pending entry for the page, and
1111 // this may or may not be the main frame. 1105 // this may or may not be the main frame.
1112 if (ui::PageTransitionIsMainFrame(params.transition)) { 1106 if (ui::PageTransitionIsMainFrame(params.transition)) {
1113 // TODO(avi): I want to use |if (!rfh->GetParent())| here but lots of unit 1107 // TODO(avi): I want to use |if (!rfh->GetParent())| here but lots of unit
1114 // tests fake auto subframe commits by sending the main frame a 1108 // tests fake auto subframe commits by sending the main frame a
1115 // PAGE_TRANSITION_AUTO_SUBFRAME transition. Fix those, and adjust here. 1109 // PAGE_TRANSITION_AUTO_SUBFRAME transition. Fix those, and adjust here.
1116 return NAVIGATION_TYPE_NEW_PAGE; 1110 return NAVIGATION_TYPE_NEW_PAGE;
1117 } 1111 }
1118 1112
(...skipping 330 matching lines...) Expand 10 before | Expand all | Expand 10 after
1449 RenderFrameHostImpl* rfh, 1443 RenderFrameHostImpl* rfh,
1450 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) { 1444 const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
1451 DCHECK(ui::PageTransitionCoreTypeIs(params.transition, 1445 DCHECK(ui::PageTransitionCoreTypeIs(params.transition,
1452 ui::PAGE_TRANSITION_AUTO_SUBFRAME)); 1446 ui::PAGE_TRANSITION_AUTO_SUBFRAME));
1453 1447
1454 // We're guaranteed to have a previously committed entry, and we now need to 1448 // We're guaranteed to have a previously committed entry, and we now need to
1455 // handle navigation inside of a subframe in it without creating a new entry. 1449 // handle navigation inside of a subframe in it without creating a new entry.
1456 DCHECK(GetLastCommittedEntry()); 1450 DCHECK(GetLastCommittedEntry());
1457 1451
1458 if (params.nav_entry_id) { 1452 if (params.nav_entry_id) {
1459 // If the |nav_entry_id| is non-zero, this is a browser-initiated navigation 1453 int entry_index = GetEntryIndexWithUniqueID(params.nav_entry_id);
1460 // and is thus a "history auto" navigation. TODO(creis): Implement
1461 // back/forward this way for site-per-process.
1462 1454
1463 int entry_index = GetEntryIndexWithUniqueID(params.nav_entry_id); 1455 // If the |nav_entry_id| is non-zero and matches an existing entry, this is
1464 if (entry_index == -1) { 1456 // a history auto" navigation. Update the last committed index accordingly.
1465 NOTREACHED(); 1457 // If we don't recognize the |nav_entry_id|, it might be either a pending
1466 return false; 1458 // entry for a transfer or a recently pruned entry. We'll handle it below.
1467 } 1459 if (entry_index != -1 && entry_index != last_committed_entry_index_) {
1468
1469 // Update the current navigation entry in case we're going back/forward.
1470 if (entry_index != last_committed_entry_index_) {
1471 // Make sure that a subframe commit isn't changing the main frame's 1460 // Make sure that a subframe commit isn't changing the main frame's
1472 // origin. Otherwise the renderer process may be confused, leading to a 1461 // origin. Otherwise the renderer process may be confused, leading to a
1473 // URL spoof. We can't check the path since that may change 1462 // URL spoof. We can't check the path since that may change
1474 // (https://crbug.com/373041). 1463 // (https://crbug.com/373041).
1475 if (GetLastCommittedEntry()->GetURL().GetOrigin() != 1464 if (GetLastCommittedEntry()->GetURL().GetOrigin() !=
1476 GetEntryAtIndex(entry_index)->GetURL().GetOrigin()) { 1465 GetEntryAtIndex(entry_index)->GetURL().GetOrigin()) {
1477 // TODO(creis): This is unexpectedly being encountered in practice. If 1466 // TODO(creis): This is unexpectedly being encountered in practice. If
1478 // you encounter this in practice, please post details to 1467 // you encounter this in practice, please post details to
1479 // https://crbug.com/486916. Once that's resolved, we'll change this to 1468 // https://crbug.com/486916. Once that's resolved, we'll change this to
1480 // kill the renderer process with bad_message::NC_AUTO_SUBFRAME. 1469 // kill the renderer process with bad_message::NC_AUTO_SUBFRAME.
1481 NOTREACHED() << "Unexpected main frame origin change on AUTO_SUBFRAME."; 1470 NOTREACHED() << "Unexpected main frame origin change on AUTO_SUBFRAME.";
1482 } 1471 }
1472
1473 // TODO(creis): Update the FrameNavigationEntry in --site-per-process.
1483 last_committed_entry_index_ = entry_index; 1474 last_committed_entry_index_ = entry_index;
1484 DiscardNonCommittedEntriesInternal(); 1475 DiscardNonCommittedEntriesInternal();
1485 return true; 1476 return true;
1486 } 1477 }
1487 } 1478 }
1488 1479
1489 if (base::CommandLine::ForCurrentProcess()->HasSwitch( 1480 if (base::CommandLine::ForCurrentProcess()->HasSwitch(
1490 switches::kSitePerProcess)) { 1481 switches::kSitePerProcess)) {
1491 // This may be a "new auto" case where we add a new FrameNavigationEntry, or 1482 // This may be a "new auto" case where we add a new FrameNavigationEntry, or
1492 // it may be a "history auto" case where we update an existing one. 1483 // it may be a "history auto" case where we update an existing one.
(...skipping 542 matching lines...) Expand 10 before | Expand all | Expand 10 after
2035 } 2026 }
2036 } 2027 }
2037 } 2028 }
2038 2029
2039 void NavigationControllerImpl::SetGetTimestampCallbackForTest( 2030 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
2040 const base::Callback<base::Time()>& get_timestamp_callback) { 2031 const base::Callback<base::Time()>& get_timestamp_callback) {
2041 get_timestamp_callback_ = get_timestamp_callback; 2032 get_timestamp_callback_ = get_timestamp_callback;
2042 } 2033 }
2043 2034
2044 } // namespace content 2035 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698