diff --git a/DEPS b/DEPS index c32122a..2beb82a0 100644 --- a/DEPS +++ b/DEPS
@@ -78,7 +78,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '03a527c718edc3b39331437822f7131aec71ed0b', + 'skia_revision': '1fa9c434fc0dbe28342c28b7d08cb9361f80c4e2', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. @@ -327,7 +327,7 @@ }, 'src/third_party/depot_tools': - Var('chromium_git') + '/chromium/tools/depot_tools.git' + '@' + 'b2af9586a0f6571ac53a1736914ecb6c15d74541', + Var('chromium_git') + '/chromium/tools/depot_tools.git' + '@' + 'f2cb0f5b3ea5ba5ccd899494163ca7b264c7d445', # DevTools node modules. Used on Linux buildbots only. 'src/third_party/devtools-node-modules': {
diff --git a/chrome/browser/chromeos/smb_client/smb_file_system.cc b/chrome/browser/chromeos/smb_client/smb_file_system.cc index 575688a..d8eee5b 100644 --- a/chrome/browser/chromeos/smb_client/smb_file_system.cc +++ b/chrome/browser/chromeos/smb_client/smb_file_system.cc
@@ -143,8 +143,8 @@ void SmbFileSystem::HandleRequestUnmountCallback( const storage::AsyncFileUtil::StatusCallback& callback, - smbprovider::ErrorType smb_error) const { - callback.Run(TranslateError(smb_error)); + smbprovider::ErrorType error) const { + callback.Run(TranslateError(error)); } AbortCallback SmbFileSystem::GetMetadata( @@ -188,17 +188,39 @@ AbortCallback SmbFileSystem::OpenFile(const base::FilePath& file_path, file_system_provider::OpenFileMode mode, const OpenFileCallback& callback) { - NOTIMPLEMENTED(); + bool writeable = + mode == file_system_provider::OPEN_FILE_MODE_WRITE ? true : false; + GetSmbProviderClient()->OpenFile( + GetMountId(), file_path, writeable, + base::BindOnce(&SmbFileSystem::HandleRequestOpenFileCallback, + weak_ptr_factory_.GetWeakPtr(), callback)); return AbortCallback(); } +void SmbFileSystem::HandleRequestOpenFileCallback( + const OpenFileCallback& callback, + smbprovider::ErrorType error, + int32_t file_id) const { + callback.Run(file_id, TranslateError(error)); +} + AbortCallback SmbFileSystem::CloseFile( int file_handle, const storage::AsyncFileUtil::StatusCallback& callback) { - NOTIMPLEMENTED(); + GetSmbProviderClient()->CloseFile( + file_handle, + base::BindOnce(&SmbFileSystem::HandleRequestCloseFileCallback, + weak_ptr_factory_.GetWeakPtr(), callback)); return AbortCallback(); } +// TODO(baileyberro): refactor basic error callbacks out into one function. +void SmbFileSystem::HandleRequestCloseFileCallback( + const storage::AsyncFileUtil::StatusCallback& callback, + smbprovider::ErrorType error) const { + callback.Run(TranslateError(error)); +} + AbortCallback SmbFileSystem::ReadFile( int file_handle, net::IOBuffer* buffer,
diff --git a/chrome/browser/chromeos/smb_client/smb_file_system.h b/chrome/browser/chromeos/smb_client/smb_file_system.h index 2513fc67..58f68ea 100644 --- a/chrome/browser/chromeos/smb_client/smb_file_system.h +++ b/chrome/browser/chromeos/smb_client/smb_file_system.h
@@ -169,7 +169,7 @@ private: void HandleRequestUnmountCallback( const storage::AsyncFileUtil::StatusCallback& callback, - smbprovider::ErrorType smb_error) const; + smbprovider::ErrorType error) const; void HandleRequestReadDirectoryCallback( const storage::AsyncFileUtil::ReadDirectoryCallback& callback, @@ -182,6 +182,14 @@ smbprovider::ErrorType error, const smbprovider::DirectoryEntry& entry) const; + void HandleRequestOpenFileCallback(const OpenFileCallback& callback, + smbprovider::ErrorType error, + int32_t file_id) const; + + void HandleRequestCloseFileCallback( + const storage::AsyncFileUtil::StatusCallback& callback, + smbprovider::ErrorType error) const; + int32_t GetMountId() const; SmbProviderClient* GetSmbProviderClient() const;
diff --git a/chrome/browser/resources/chromeos/login/oobe_change_picture.js b/chrome/browser/resources/chromeos/login/oobe_change_picture.js index 9204ee60..410c2a8 100644 --- a/chrome/browser/resources/chromeos/login/oobe_change_picture.js +++ b/chrome/browser/resources/chromeos/login/oobe_change_picture.js
@@ -68,7 +68,7 @@ */ cameraVideoModeEnabled: { type: Boolean, - value: true, + value: false, }, /**
diff --git a/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js b/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js index 09f5631..991de6bc 100644 --- a/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js +++ b/chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js
@@ -17,6 +17,7 @@ USER_DECLINED_CLEANUP: 'user_declined_cleanup', CLEANING_FAILED: 'cleaning_failed', CLEANING_SUCCEEDED: 'cleaning_succeeded', + CLEANER_DOWNLOAD_FAILED: 'cleaner_download_failed', }; /** @@ -47,6 +48,7 @@ SCANNING_FAILED: 'scanning_failed', CLEANUP_SUCCEEDED: 'cleanup_succeeded', CLEANING_FAILED: 'cleanup_failed', + CLEANER_DOWNLOAD_FAILED: 'cleaner_download_failed', }; /** @@ -486,6 +488,11 @@ settings.ChromeCleanerCardState.CLEANUP_SUCCEEDED); break; + case settings.ChromeCleanupIdleReason.CLEANER_DOWNLOAD_FAILED: + this.renderCleanupCard_( + settings.ChromeCleanerCardState.CLEANER_DOWNLOAD_FAILED); + break; + default: assert(false, `Unknown idle reason: ${idleReason}`); } @@ -779,7 +786,14 @@ doAction: this.dismiss_.bind( this, settings.ChromeCleanupDismissSource.CLEANUP_FAILURE_DONE_BUTTON), - } + }, + + TRY_SCAN_AGAIN: { + label: this.i18n('chromeCleanupTitleTryAgainButtonLabel'), + // TODO(crbug.com/776538): do not run the reporter component again. + // Try downloading the cleaner and scan with it instead. + doAction: this.startScanning_.bind(this), + }, }; // If user-initiated cleanups are enabled, there is no need for a custom @@ -889,6 +903,15 @@ flags: learnMoreIfUserInitiatedCleanupsDisabled, } ], + [ + settings.ChromeCleanerCardState.CLEANER_DOWNLOAD_FAILED, { + title: this.i18n('chromeCleanupTitleNoInternet'), + explanation: this.i18n('chromeCleanupExplanationNoInternet'), + icon: icons.WARNING, + actionButton: actionButtons.TRY_SCAN_AGAIN, + flags: learnMoreIfUserInitiatedCleanupsDisabled, + }, + ], ]); }, });
diff --git a/chrome/browser/resources/settings/people_page/change_picture.js b/chrome/browser/resources/settings/people_page/change_picture.js index 21f94b3..33100bd 100644 --- a/chrome/browser/resources/settings/people_page/change_picture.js +++ b/chrome/browser/resources/settings/people_page/change_picture.js
@@ -60,7 +60,7 @@ */ cameraVideoModeEnabled_: { type: Boolean, - value: true, + value: false, }, },
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.cc b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.cc index 56287e4..5404a63d 100644 --- a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.cc +++ b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.cc
@@ -576,7 +576,7 @@ DCHECK(reporter_invocation_); if (executable_path.empty()) { - idle_reason_ = IdleReason::kScanningFailed; + idle_reason_ = IdleReason::kCleanerDownloadFailed; SetStateAndNotifyObservers(State::kIdle); RecordPromptNotShownWithReasonHistogram( NO_PROMPT_REASON_CLEANER_DOWNLOAD_FAILED);
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win_unittest.cc b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win_unittest.cc index ebab1aa9..98e440c 100644 --- a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win_unittest.cc +++ b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win_unittest.cc
@@ -393,6 +393,10 @@ ChromeCleanerController::IdleReason ExpectedIdleReason() { EXPECT_EQ(ExpectedFinalState(), State::kIdle); + if (process_status_ == CleanerProcessStatus::kFetchFailure) { + return IdleReason::kCleanerDownloadFailed; + } + if (process_status_ != CleanerProcessStatus::kFetchSuccessValidProcess || crash_point_ == CrashPoint::kOnStartup || crash_point_ == CrashPoint::kAfterConnection) {
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h index 0bab622..e4ade5f 100644 --- a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h +++ b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h
@@ -78,6 +78,7 @@ kUserDeclinedCleanup, kCleaningFailed, kCleaningSucceeded, + kCleanerDownloadFailed, }; enum class UserResponse {
diff --git a/chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc b/chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc index a0e3aad..535fbb2 100644 --- a/chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc +++ b/chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc
@@ -91,6 +91,8 @@ return "cleaning_failed"; case ChromeCleanerController::IdleReason::kCleaningSucceeded: return "cleaning_succeeded"; + case ChromeCleanerController::IdleReason::kCleanerDownloadFailed: + return "cleaner_download_failed"; } NOTREACHED(); return "";
diff --git a/chrome/test/data/webui/settings/chrome_cleanup_page_test.js b/chrome/test/data/webui/settings/chrome_cleanup_page_test.js index 61040ee..ca18619 100644 --- a/chrome/test/data/webui/settings/chrome_cleanup_page_test.js +++ b/chrome/test/data/webui/settings/chrome_cleanup_page_test.js
@@ -75,7 +75,7 @@ var chromeCleanupPage = null; /** @type {?TestDownloadsBrowserProxy} */ -var ChromeCleanupProxy = null; +var chromeCleanupProxy = null; var shortFileList = ['file 1', 'file 2', 'file 3']; var exactSizeFileList = ['file 1', 'file 2', 'file 3', 'file 4']; @@ -95,8 +95,8 @@ * cleanup feature is enabled. */ function initParametrizedTest(userInitiatedCleanupsEnabled) { - ChromeCleanupProxy = new TestChromeCleanupProxy(); - settings.ChromeCleanupProxyImpl.instance_ = ChromeCleanupProxy; + chromeCleanupProxy = new TestChromeCleanupProxy(); + settings.ChromeCleanupProxyImpl.instance_ = chromeCleanupProxy; PolymerTest.clearBody(); @@ -179,7 +179,7 @@ var actionButton = chromeCleanupPage.$$('#action-button'); assertTrue(!!actionButton); MockInteractions.tap(actionButton); - return ChromeCleanupProxy.whenCalled('startCleanup') + return chromeCleanupProxy.whenCalled('startCleanup') .then(function(logsUploadEnabled) { assertFalse(logsUploadEnabled); cr.webUIListenerCallback( @@ -198,7 +198,7 @@ var actionButton = chromeCleanupPage.$$('#action-button'); assertTrue(!!actionButton); MockInteractions.tap(actionButton); - return ChromeCleanupProxy.whenCalled('restartComputer'); + return chromeCleanupProxy.whenCalled('restartComputer'); } /** @@ -219,7 +219,7 @@ } else { assertTrue(!!actionButton); MockInteractions.tap(actionButton); - return ChromeCleanupProxy.whenCalled('dismissCleanupPage'); + return chromeCleanupProxy.whenCalled('dismissCleanupPage'); } } @@ -240,7 +240,7 @@ } else { assertTrue(!!actionButton); MockInteractions.tap(actionButton); - return ChromeCleanupProxy.whenCalled('dismissCleanupPage'); + return chromeCleanupProxy.whenCalled('dismissCleanupPage'); } } @@ -270,7 +270,7 @@ assertFalse(logsControl.checked); MockInteractions.tap(logsControl.$.control); - return ChromeCleanupProxy.whenCalled('setLogsUploadPermission') + return chromeCleanupProxy.whenCalled('setLogsUploadPermission') .then(function(logsUploadEnabled) { assertTrue(logsUploadEnabled); }); @@ -422,6 +422,24 @@ settings.ChromeCleanupIdleReason.CLEANING_SUCCEEDED); }); + test('scanOfferedOnInitiallyIdle_CleanerDownloadFailed', function() { + scanOfferedOnInitiallyIdle( + settings.ChromeCleanupIdleReason.CLEANER_DOWNLOAD_FAILED); + }); + + test('cleanerDownloadFailure', function() { + cr.webUIListenerCallback('chrome-cleanup-on-reporter-running'); + cr.webUIListenerCallback( + 'chrome-cleanup-on-idle', + settings.ChromeCleanupIdleReason.CLEANER_DOWNLOAD_FAILED); + Polymer.dom.flush(); + + var actionButton = chromeCleanupPage.$$('#action-button'); + assertTrue(!!actionButton); + MockInteractions.tap(actionButton); + return chromeCleanupProxy.whenCalled('startScanning'); + }); + test('reporterFoundNothing', function() { cr.webUIListenerCallback('chrome-cleanup-on-reporter-running'); cr.webUIListenerCallback( @@ -453,7 +471,7 @@ var actionButton = chromeCleanupPage.$$('#action-button'); assertTrue(!!actionButton); MockInteractions.tap(actionButton); - return ChromeCleanupProxy.whenCalled('startScanning') + return chromeCleanupProxy.whenCalled('startScanning') .then(function(logsUploadEnabled) { assertFalse(logsUploadEnabled); cr.webUIListenerCallback('chrome-cleanup-on-scanning', false);
diff --git a/components/data_reduction_proxy/core/browser/BUILD.gn b/components/data_reduction_proxy/core/browser/BUILD.gn index dd81bb3..c8d6b73 100644 --- a/components/data_reduction_proxy/core/browser/BUILD.gn +++ b/components/data_reduction_proxy/core/browser/BUILD.gn
@@ -168,6 +168,7 @@ "data_reduction_proxy_config_unittest.cc", "data_reduction_proxy_configurator_unittest.cc", "data_reduction_proxy_data_unittest.cc", + "data_reduction_proxy_data_use_observer_unittest.cc", "data_reduction_proxy_delegate_unittest.cc", "data_reduction_proxy_interceptor_unittest.cc", "data_reduction_proxy_io_data_unittest.cc", @@ -192,6 +193,7 @@ "//base/test:test_support", "//components/data_reduction_proxy/core/common:test_support", "//components/data_reduction_proxy/proto:data_reduction_proxy_proto", + "//components/data_use_measurement/core:ascriber", "//components/prefs:test_support", "//components/previews/core", "//components/variations",
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc index 2cecfe72..c6dbccb 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
@@ -4,6 +4,8 @@ #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h" +#include <string> + #include "base/memory/ptr_util.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h" @@ -13,6 +15,7 @@ #include "components/data_reduction_proxy/core/common/lofi_decider.h" #include "components/data_use_measurement/core/data_use.h" #include "components/data_use_measurement/core/data_use_ascriber.h" +#include "components/previews/core/previews_user_data.h" #include "net/http/http_response_headers.h" #include "net/url_request/url_request.h" #include "url/gurl.h" @@ -48,6 +51,8 @@ const void* const DataUseUserDataBytes::kUserDataKey = &DataUseUserDataBytes::kUserDataKey; +const void* const kDataUsePreviewsUserDataKey = &kDataUsePreviewsUserDataKey; + } // namespace namespace data_reduction_proxy { @@ -93,6 +98,13 @@ return; } + previews::PreviewsUserData* previews_user_data = + previews::PreviewsUserData::GetData(request); + if (previews_user_data) { + data_use->SetUserData(kDataUsePreviewsUserDataKey, + previews_user_data->DeepCopy()); + } + if (request.GetTotalReceivedBytes() <= 0) return; @@ -134,8 +146,38 @@ void DataReductionProxyDataUseObserver::OnPageDidFinishLoad( data_use_measurement::DataUse* data_use) { + // This is good place to update data savings based on the overall page + // load. If we waited until the |OnPageLoadConcluded| callback, we would miss + // cases where the page loaded but the user doesn't navigate away before + // Android kills chrome. DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); - // TODO(dougarnett): 781885 - estimate data saving for NoScript. + previews::PreviewsUserData* previews_user_data = + reinterpret_cast<previews::PreviewsUserData*>( + data_use->GetUserData(kDataUsePreviewsUserDataKey)); + if (previews_user_data) { + // Report estimated data savings for NOSCRIPT if applicable. + if (previews_user_data->GetCommittedPreviewsType() == + previews::PreviewsType::NOSCRIPT) { + int inflated_bytes = + (data_use->total_bytes_received() * + previews::params::NoScriptPreviewsInflationPercent()) / + 100 + + previews::params::NoScriptPreviewsInflationBytes(); + // Report for overall usage. + DCHECK(data_use->url().SchemeIs(url::kHttpsScheme)); + data_reduction_proxy_io_data_->UpdateContentLengths( + 0, inflated_bytes, data_reduction_proxy_io_data_->IsEnabled(), HTTPS, + std::string()); + // Report for host usage. + data_reduction_proxy_io_data_->UpdateDataUseForHost( + 0, inflated_bytes, data_use->url().HostNoBrackets()); + } + } +} + +const void* +DataReductionProxyDataUseObserver::GetDataUsePreviewsUserDataKeyForTesting() { + return kDataUsePreviewsUserDataKey; } } // namespace data_reduction_proxy
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h index efef0bd97..f64d037 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h
@@ -33,6 +33,8 @@ ~DataReductionProxyDataUseObserver() override; private: + friend class DataReductionProxyDataUseObserverTest; + // PageLoadObserver methods. void OnPageLoadCommit(data_use_measurement::DataUse* data_use) override; void OnPageResourceLoad(const net::URLRequest& request, @@ -40,6 +42,8 @@ void OnPageDidFinishLoad(data_use_measurement::DataUse* data_use) override; void OnPageLoadConcluded(data_use_measurement::DataUse* data_use) override {} + const void* GetDataUsePreviewsUserDataKeyForTesting(); + // |data_reduction_proxy_io_data_| owns |this| and is destroyed only after // |this| is destroyed in the IO thread. DataReductionProxyIOData* data_reduction_proxy_io_data_;
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer_unittest.cc new file mode 100644 index 0000000..df50e87 --- /dev/null +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer_unittest.cc
@@ -0,0 +1,145 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h" + +#include <map> + +#include "base/memory/ptr_util.h" +#include "base/run_loop.h" +#include "base/strings/string_number_conversions.h" +#include "base/test/scoped_feature_list.h" +#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h" +#include "components/data_use_measurement/core/data_use.h" +#include "components/data_use_measurement/core/data_use_ascriber.h" +#include "components/data_use_measurement/core/data_use_recorder.h" +#include "components/data_use_measurement/core/url_request_classifier.h" +#include "components/previews/core/previews_experiments.h" +#include "components/previews/core/previews_features.h" +#include "components/previews/core/previews_user_data.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using ::testing::Mock; +using ::testing::Return; +using ::testing::_; + +namespace { + +class MockDataUseAscriber : public data_use_measurement::DataUseAscriber { + public: + MOCK_METHOD1( + GetOrCreateDataUseRecorder, + data_use_measurement::DataUseRecorder*(net::URLRequest* request)); + MOCK_METHOD1( + GetDataUseRecorder, + data_use_measurement::DataUseRecorder*(const net::URLRequest& request)); + MOCK_CONST_METHOD0( + CreateURLRequestClassifier, + std::unique_ptr<data_use_measurement::URLRequestClassifier>()); +}; + +const int kInflationPercent = 50; +const int kInflationBytes = 1000; + +} // namespace + +namespace data_reduction_proxy { + +class DataReductionProxyDataUseObserverTest : public testing::Test { + public: + DataReductionProxyDataUseObserverTest() {} + + void SetUp() override { + std::map<std::string, std::string> parameters = { + {"NoScriptInflationPercent", base::IntToString(kInflationPercent)}, + {"NoScriptInflationBytes", base::IntToString(kInflationBytes)}}; + scoped_feature_list_.InitAndEnableFeatureWithParameters( + previews::features::kNoScriptPreviews, parameters); + test_context_ = DataReductionProxyTestContext::Builder() + .WithConfigClient() + .WithMockDataReductionProxyService() + .Build(); + ascriber_ = std::make_unique<MockDataUseAscriber>(); + data_use_observer_ = std::make_unique<DataReductionProxyDataUseObserver>( + test_context_->io_data(), ascriber_.get()); + } + + MockDataReductionProxyService* mock_drp_service() { + return test_context_->mock_data_reduction_proxy_service(); + } + + void SetPreviewsUserData(data_use_measurement::DataUse* data_use, + previews::PreviewsUserData* previews_user_data) { + data_use->SetUserData( + data_use_observer_->GetDataUsePreviewsUserDataKeyForTesting(), + previews_user_data->DeepCopy()); + } + + void DidFinishLoad(data_use_measurement::DataUse* data_use) { + data_use_observer_->OnPageDidFinishLoad(data_use); + } + + void RunUntilIdle() { test_context_->RunUntilIdle(); } + + private: + base::test::ScopedFeatureList scoped_feature_list_; + base::MessageLoopForIO message_loop_; + std::unique_ptr<DataReductionProxyTestContext> test_context_; + std::unique_ptr<MockDataUseAscriber> ascriber_; + std::unique_ptr<DataReductionProxyDataUseObserver> data_use_observer_; + + DISALLOW_COPY_AND_ASSIGN(DataReductionProxyDataUseObserverTest); +}; + +TEST_F(DataReductionProxyDataUseObserverTest, + OnPageDidFinishLoadNotNoScriptPreview) { + std::unique_ptr<data_use_measurement::DataUse> data_use = + std::make_unique<data_use_measurement::DataUse>( + data_use_measurement::DataUse::TrafficType::USER_TRAFFIC); + data_use->IncrementTotalBytes(500000, 300); + data_use->set_url(GURL("https://testsite.com")); + + // Verify with no committed preview. + previews::PreviewsUserData previews_user_data(3 /* page_id */); + SetPreviewsUserData(data_use.get(), &previews_user_data); + EXPECT_CALL(*mock_drp_service(), UpdateContentLengths(_, _, _, _, _)) + .Times(0); + EXPECT_CALL(*mock_drp_service(), UpdateDataUseForHost(_, _, _)).Times(0); + DidFinishLoad(data_use.get()); + RunUntilIdle(); + Mock::VerifyAndClearExpectations(mock_drp_service()); + + // Now verify with LOFI as committed preview. + previews_user_data.SetCommittedPreviewsType(previews::PreviewsType::LOFI); + SetPreviewsUserData(data_use.get(), &previews_user_data); + EXPECT_CALL(*mock_drp_service(), UpdateContentLengths(_, _, _, _, _)) + .Times(0); + EXPECT_CALL(*mock_drp_service(), UpdateDataUseForHost(_, _, _)).Times(0); + DidFinishLoad(data_use.get()); + RunUntilIdle(); +} + +TEST_F(DataReductionProxyDataUseObserverTest, + OnPageDidFinishLoadHasCommittedNoScriptPreview) { + std::unique_ptr<data_use_measurement::DataUse> data_use = + std::make_unique<data_use_measurement::DataUse>( + data_use_measurement::DataUse::TrafficType::USER_TRAFFIC); + data_use->IncrementTotalBytes(500000, 300); + data_use->set_url(GURL("https://testsite.com")); + previews::PreviewsUserData previews_user_data(7 /* page_id */); + previews_user_data.SetCommittedPreviewsType(previews::PreviewsType::NOSCRIPT); + SetPreviewsUserData(data_use.get(), &previews_user_data); + int inflation_value = 500000 * kInflationPercent / 100 + kInflationBytes; + EXPECT_CALL(*mock_drp_service(), + UpdateContentLengths(0, inflation_value, true, _, _)) + .Times(1); + EXPECT_CALL(*mock_drp_service(), + UpdateDataUseForHost(0, 251000, "testsite.com")) + .Times(1); + DidFinishLoad(data_use.get()); + RunUntilIdle(); +} + +} // namespace data_reduction_proxy
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h index 240d5e3..563fe87 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
@@ -77,16 +77,18 @@ bool Initialized() const; // Records data usage per host. - void UpdateDataUseForHost(int64_t network_bytes, - int64_t original_bytes, - const std::string& host); + // Virtual for testing. + virtual void UpdateDataUseForHost(int64_t network_bytes, + int64_t original_bytes, + const std::string& host); // Records daily data savings statistics in |compression_stats_|. - void UpdateContentLengths(int64_t data_used, - int64_t original_size, - bool data_reduction_proxy_enabled, - DataReductionProxyRequestType request_type, - const std::string& mime_type); + // Virtual for testing. + virtual void UpdateContentLengths(int64_t data_used, + int64_t original_size, + bool data_reduction_proxy_enabled, + DataReductionProxyRequestType request_type, + const std::string& mime_type); // Overrides of DataReductionProxyEventStorageDelegate. void AddEvent(std::unique_ptr<base::Value> event) override;
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h index 0adb3ffd..c4a3962b 100644 --- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h +++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
@@ -187,6 +187,17 @@ ~MockDataReductionProxyService() override; MOCK_METHOD2(SetProxyPrefs, void(bool enabled, bool at_startup)); + MOCK_METHOD5( + UpdateContentLengths, + void(int64_t data_used, + int64_t original_size, + bool data_reduction_proxy_enabled, + data_reduction_proxy::DataReductionProxyRequestType request_type, + const std::string& mime_type)); + MOCK_METHOD3(UpdateDataUseForHost, + void(int64_t network_bytes, + int64_t original_bytes, + const std::string& host)); }; // Test version of |DataReductionProxyIOData|, which bypasses initialization in
diff --git a/components/previews/core/previews_experiments.cc b/components/previews/core/previews_experiments.cc index 1c20d9e..dffb9c6e 100644 --- a/components/previews/core/previews_experiments.cc +++ b/components/previews/core/previews_experiments.cc
@@ -36,6 +36,10 @@ const char kClientLoFiExperimentName[] = "PreviewsClientLoFi"; +// Inflation parameters for estimating NoScript data savings. +const char kNoScriptInflationPercent[] = "NoScriptInflationPercent"; +const char kNoScriptInflationBytes[] = "NoScriptInflationBytes"; + size_t GetParamValueAsSizeT(const std::string& trial_name, const std::string& param_name, size_t default_value) { @@ -200,6 +204,19 @@ ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); } +int NoScriptPreviewsInflationPercent() { + // The default value was determined from lab experiment data of whitelisted + // URLs. It may be improved once there is enough UKM live experiment data + // via the field trial param. + return GetFieldTrialParamByFeatureAsInt(features::kNoScriptPreviews, + kNoScriptInflationPercent, 80); +} + +int NoScriptPreviewsInflationBytes() { + return GetFieldTrialParamByFeatureAsInt(features::kNoScriptPreviews, + kNoScriptInflationBytes, 0); +} + } // namespace params std::string GetStringNameForType(PreviewsType type) {
diff --git a/components/previews/core/previews_experiments.h b/components/previews/core/previews_experiments.h index f1572557..1755106 100644 --- a/components/previews/core/previews_experiments.h +++ b/components/previews/core/previews_experiments.h
@@ -103,6 +103,14 @@ // Returns the hosts that are blacklisted by the Client Lo-Fi field trial. std::vector<std::string> GetBlackListedHostsForClientLoFiFieldTrial(); +// For estimating NoScript data savings, this is the percentage factor to +// multiple by the network bytes for inflating the original_bytes count. +int NoScriptPreviewsInflationPercent(); + +// For estimating NoScript data savings, this is the number of bytes to +// for inflating the original_bytes count. +int NoScriptPreviewsInflationBytes(); + } // namespace params } // namespace previews
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc index a1e4484..210f076 100644 --- a/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -235,29 +235,14 @@ } bool HasNavigationRequest() { - if (IsBrowserSideNavigationEnabled()) { - return contents()->GetFrameTree()->root()->navigation_request() != - nullptr; - } - return process()->sink().GetFirstMessageMatching(FrameMsg_Navigate::ID) - != nullptr; + return contents()->GetFrameTree()->root()->navigation_request() != nullptr; } const GURL GetLastNavigationURL() { - if (IsBrowserSideNavigationEnabled()) { - NavigationRequest* navigation_request = - contents()->GetFrameTree()->root()->navigation_request(); - CHECK(navigation_request); - return navigation_request->common_params().url; - } - const IPC::Message* message = - process()->sink().GetFirstMessageMatching(FrameMsg_Navigate::ID); - CHECK(message); - std::tuple<CommonNavigationParams, StartNavigationParams, - RequestNavigationParams> - nav_params; - FrameMsg_Navigate::Read(message, &nav_params); - return std::get<0>(nav_params).url; + NavigationRequest* navigation_request = + contents()->GetFrameTree()->root()->navigation_request(); + CHECK(navigation_request); + return navigation_request->common_params().url; } protected:
diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc index 4d7272e..411d336 100644 --- a/content/browser/frame_host/navigation_entry_impl.cc +++ b/content/browser/frame_host/navigation_entry_impl.cc
@@ -701,13 +701,6 @@ has_started_from_context_menu(), user_gesture); } -StartNavigationParams NavigationEntryImpl::ConstructStartNavigationParams() - const { - return StartNavigationParams(extra_headers(), - transferred_global_request_id().child_id, - transferred_global_request_id().request_id); -} - RequestNavigationParams NavigationEntryImpl::ConstructRequestNavigationParams( const FrameNavigationEntry& frame_entry, const GURL& original_url,
diff --git a/content/browser/frame_host/navigation_entry_impl.h b/content/browser/frame_host/navigation_entry_impl.h index 6d54c4b..ff1e717 100644 --- a/content/browser/frame_host/navigation_entry_impl.h +++ b/content/browser/frame_host/navigation_entry_impl.h
@@ -36,7 +36,6 @@ class ResourceRequestBody; struct CommonNavigationParams; struct RequestNavigationParams; -struct StartNavigationParams; class CONTENT_EXPORT NavigationEntryImpl : public NavigationEntry { public: @@ -188,7 +187,6 @@ FrameMsg_Navigate_Type::Value navigation_type, PreviewsState previews_state, const base::TimeTicks& navigation_start) const; - StartNavigationParams ConstructStartNavigationParams() const; RequestNavigationParams ConstructRequestNavigationParams( const FrameNavigationEntry& frame_entry, const GURL& original_url,
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index 498eb98..1693d92 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h
@@ -400,12 +400,6 @@ IPC_STRUCT_TRAITS_MEMBER(has_user_gesture) IPC_STRUCT_TRAITS_END() -IPC_STRUCT_TRAITS_BEGIN(content::StartNavigationParams) - IPC_STRUCT_TRAITS_MEMBER(extra_headers) - IPC_STRUCT_TRAITS_MEMBER(transferred_request_child_id) - IPC_STRUCT_TRAITS_MEMBER(transferred_request_request_id) -IPC_STRUCT_TRAITS_END() - IPC_STRUCT_TRAITS_BEGIN(content::NavigationTiming) IPC_STRUCT_TRAITS_MEMBER(redirect_start) IPC_STRUCT_TRAITS_MEMBER(redirect_end) @@ -742,13 +736,6 @@ // Instructs the renderer to delete the RenderFrame. IPC_MESSAGE_ROUTED0(FrameMsg_Delete) -// Tells the renderer to perform the specified navigation, interrupting any -// existing navigation. -IPC_MESSAGE_ROUTED3(FrameMsg_Navigate, - content::CommonNavigationParams, /* common_params */ - content::StartNavigationParams, /* start_params */ - content::RequestNavigationParams /* request_params */) - // Instructs the renderer to invoke the frame's beforeunload event handler. // Expects the result to be returned via FrameHostMsg_BeforeUnload_ACK. IPC_MESSAGE_ROUTED1(FrameMsg_BeforeUnload, bool /* is_reload */)
diff --git a/content/common/navigation_params.cc b/content/common/navigation_params.cc index 0842f26c..e42ab19a 100644 --- a/content/common/navigation_params.cc +++ b/content/common/navigation_params.cc
@@ -89,26 +89,6 @@ CommonNavigationParams::~CommonNavigationParams() { } -StartNavigationParams::StartNavigationParams() - : transferred_request_child_id(-1), - transferred_request_request_id(-1) { -} - -StartNavigationParams::StartNavigationParams( - const std::string& extra_headers, - int transferred_request_child_id, - int transferred_request_request_id) - : extra_headers(extra_headers), - transferred_request_child_id(transferred_request_child_id), - transferred_request_request_id(transferred_request_request_id) { -} - -StartNavigationParams::StartNavigationParams( - const StartNavigationParams& other) = default; - -StartNavigationParams::~StartNavigationParams() { -} - RequestNavigationParams::RequestNavigationParams() : is_overriding_user_agent(false), can_load_local_resources(false), @@ -170,10 +150,8 @@ NavigationParams::NavigationParams( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params) : common_params(common_params), - start_params(start_params), request_params(request_params) { }
diff --git a/content/common/navigation_params.h b/content/common/navigation_params.h index 2f3e46d..9e38981 100644 --- a/content/common/navigation_params.h +++ b/content/common/navigation_params.h
@@ -157,37 +157,6 @@ }; // Provided by the browser ----------------------------------------------------- -// -// These structs are sent by the browser to the renderer to start/commit a -// navigation depending on whether browser-side navigation is enabled. -// Parameters used both in the current architecture and PlzNavigate should be -// put in RequestNavigationParams. Parameters only used by the current -// architecture should go in StartNavigationParams. - -// Used by FrameMsg_Navigate. Holds the parameters needed by the renderer to -// start a browser-initiated navigation besides those in CommonNavigationParams. -// The difference with the RequestNavigationParams below is that they are only -// used in the current architecture of navigation, and will not be used by -// PlzNavigate. -// PlzNavigate: These are not used. -struct CONTENT_EXPORT StartNavigationParams { - StartNavigationParams(); - StartNavigationParams(const std::string& extra_headers, - int transferred_request_child_id, - int transferred_request_request_id); - StartNavigationParams(const StartNavigationParams& other); - ~StartNavigationParams(); - - // Extra headers (separated by \n) to send during the request. - std::string extra_headers; - - // The following two members identify a previous request that has been - // created before this navigation is being transferred to a new process. - // This serves the purpose of recycling the old request. - // Unless this refers to a transferred navigation, these values are -1 and -1. - int transferred_request_child_id; - int transferred_request_request_id; -}; // PlzNavigate // Timings collected in the browser during navigation for the @@ -338,12 +307,10 @@ // needs to provide to the renderer. struct NavigationParams { NavigationParams(const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params); ~NavigationParams(); CommonNavigationParams common_params; - StartNavigationParams start_params; RequestNavigationParams request_params; };
diff --git a/content/network/cookie_manager.cc b/content/network/cookie_manager.cc index 00c4f20..3214409 100644 --- a/content/network/cookie_manager.cc +++ b/content/network/cookie_manager.cc
@@ -49,36 +49,35 @@ // Return true if the given cookie should be deleted. bool Predicate(const net::CanonicalCookie& cookie) { // Ignore begin/end times; they're handled by method args. - bool result = true; + // Deleted cookies must satisfy all conditions, so if any of the + // below matches fail return false early. // Delete if the cookie is not in excluding_domains_. - if (use_excluding_domains_ && result) - result &= !DomainMatches(cookie, excluding_domains_); + if (use_excluding_domains_ && DomainMatches(cookie, excluding_domains_)) + return false; // Delete if the cookie is in including_domains_. - if (use_including_domains_ && result) - result &= DomainMatches(cookie, including_domains_); + if (use_including_domains_ && !DomainMatches(cookie, including_domains_)) + return false; // Delete if the cookie has a specified name. - if (use_cookie_name_ && result) - result &= (cookie_name_ == cookie.Name()); + if (use_cookie_name_ && !(cookie_name_ == cookie.Name())) + return false; // Delete if the cookie matches the URL. - if (use_url_ && result) - result &= cookie.IncludeForRequestURL(url_, options_); + if (use_url_ && !cookie.IncludeForRequestURL(url_, options_)) + return false; // Delete if the cookie is not the correct persistent or session type. if (session_control_ != network::mojom::CookieDeletionSessionControl::IGNORE_CONTROL && - result) { - // Relies on the assumption that there are only three values possible for - // session_control. - result &= - (cookie.IsPersistent() == - (session_control_ == - network::mojom::CookieDeletionSessionControl::PERSISTENT_COOKIES)); + (cookie.IsPersistent() != + (session_control_ == + network::mojom::CookieDeletionSessionControl::PERSISTENT_COOKIES))) { + return false; } - return result; + + return true; } private:
diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc index 43dbf8b..88a7a50 100644 --- a/content/public/test/render_view_test.cc +++ b/content/public/test/render_view_test.cc
@@ -531,8 +531,7 @@ RenderViewImpl* impl = static_cast<RenderViewImpl*>(view_); TestRenderFrame* frame = static_cast<TestRenderFrame*>(impl->GetMainRenderFrame()); - frame->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame->Navigate(common_params, RequestNavigationParams()); FrameLoadWaiter(frame).Wait(); view_->GetWebView()->UpdateAllLifecyclePhases(); } @@ -678,7 +677,7 @@ TestRenderFrame* frame = static_cast<TestRenderFrame*>(impl->GetMainRenderFrame()); - frame->Navigate(common_params, StartNavigationParams(), request_params); + frame->Navigate(common_params, request_params); // The load actually happens asynchronously, so we pump messages to process // the pending continuation.
diff --git a/content/renderer/loader/request_extra_data.cc b/content/renderer/loader/request_extra_data.cc index 4202a41b..a18495c 100644 --- a/content/renderer/loader/request_extra_data.cc +++ b/content/renderer/loader/request_extra_data.cc
@@ -20,8 +20,6 @@ allow_download_(true), transition_type_(ui::PAGE_TRANSITION_LINK), should_replace_current_entry_(false), - transferred_request_child_id_(-1), - transferred_request_request_id_(-1), service_worker_provider_id_(kInvalidServiceWorkerProviderId), originated_from_service_worker_(false), initiated_in_secure_context_(false), @@ -41,8 +39,6 @@ request->allow_download = allow_download_; request->transition_type = transition_type_; request->should_replace_current_entry = should_replace_current_entry_; - request->transferred_request_child_id = transferred_request_child_id_; - request->transferred_request_request_id = transferred_request_request_id_; request->service_worker_provider_id = service_worker_provider_id_; request->originated_from_service_worker = originated_from_service_worker_;
diff --git a/content/renderer/loader/request_extra_data.h b/content/renderer/loader/request_extra_data.h index d2b78f4..bf0273860 100644 --- a/content/renderer/loader/request_extra_data.h +++ b/content/renderer/loader/request_extra_data.h
@@ -56,14 +56,6 @@ bool should_replace_current_entry) { should_replace_current_entry_ = should_replace_current_entry; } - void set_transferred_request_child_id( - int transferred_request_child_id) { - transferred_request_child_id_ = transferred_request_child_id; - } - void set_transferred_request_request_id( - int transferred_request_request_id) { - transferred_request_request_id_ = transferred_request_request_id; - } int service_worker_provider_id() const { return service_worker_provider_id_; } @@ -127,7 +119,6 @@ block_mixed_plugin_content_ = block_mixed_plugin_content; } - // PlzNavigate // Indicates whether a navigation was initiated by the browser or renderer. bool navigation_initiated_by_renderer() const { return navigation_initiated_by_renderer_; @@ -154,8 +145,6 @@ bool allow_download_; ui::PageTransition transition_type_; bool should_replace_current_entry_; - int transferred_request_child_id_; - int transferred_request_request_id_; int service_worker_provider_id_; bool originated_from_service_worker_; blink::WebString custom_user_agent_;
diff --git a/content/renderer/navigation_state_impl.cc b/content/renderer/navigation_state_impl.cc index 659c27c..bd7fa791 100644 --- a/content/renderer/navigation_state_impl.cc +++ b/content/renderer/navigation_state_impl.cc
@@ -11,15 +11,12 @@ NavigationStateImpl* NavigationStateImpl::CreateBrowserInitiated( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params) { - return new NavigationStateImpl(common_params, start_params, request_params, - false); + return new NavigationStateImpl(common_params, request_params, false); } NavigationStateImpl* NavigationStateImpl::CreateContentInitiated() { return new NavigationStateImpl(CommonNavigationParams(), - StartNavigationParams(), RequestNavigationParams(), true); } @@ -37,14 +34,12 @@ NavigationStateImpl::NavigationStateImpl( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params, bool is_content_initiated) : request_committed_(false), was_within_same_document_(false), is_content_initiated_(is_content_initiated), common_params_(common_params), - start_params_(start_params), request_params_(request_params) {} } // namespace content
diff --git a/content/renderer/navigation_state_impl.h b/content/renderer/navigation_state_impl.h index 87aad584..66461e2c 100644 --- a/content/renderer/navigation_state_impl.h +++ b/content/renderer/navigation_state_impl.h
@@ -19,7 +19,6 @@ static NavigationStateImpl* CreateBrowserInitiated( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params); static NavigationStateImpl* CreateContentInitiated(); @@ -30,7 +29,6 @@ bool IsContentInitiated() override; const CommonNavigationParams& common_params() const { return common_params_; } - const StartNavigationParams& start_params() const { return start_params_; } const RequestNavigationParams& request_params() const { return request_params_; } @@ -46,7 +44,6 @@ private: NavigationStateImpl(const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params, bool is_content_initiated); @@ -57,7 +54,6 @@ const bool is_content_initiated_; CommonNavigationParams common_params_; - const StartNavigationParams start_params_; // Note: if IsContentInitiated() is false, whether this navigation should // replace the current entry in the back/forward history list is determined by
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 1ea6e43..9069bc75 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -65,7 +65,6 @@ #include "content/public/common/appcache_info.h" #include "content/public/common/bind_interface_helpers.h" #include "content/public/common/bindings_policy.h" -#include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/content_constants.h" #include "content/public/common/content_features.h" #include "content/public/common/content_switches.h" @@ -402,18 +401,15 @@ std::unique_ptr<StreamOverrideParameters> stream_override, bool is_view_source_mode_enabled, bool is_same_document_navigation) { - // PlzNavigate: use the original navigation url to construct the - // WebURLRequest. The WebURLloaderImpl will replay the redirects afterwards - // and will eventually commit the final url. - const GURL navigation_url = IsBrowserSideNavigationEnabled() && - !request_params.original_url.is_empty() + // Use the original navigation url to construct the WebURLRequest. The + // WebURLloaderImpl will replay the redirects afterwards and will eventually + // commit the final url. + const GURL navigation_url = !request_params.original_url.is_empty() ? request_params.original_url : common_params.url; - const std::string navigation_method = - IsBrowserSideNavigationEnabled() && - !request_params.original_method.empty() - ? request_params.original_method - : common_params.method; + const std::string navigation_method = !request_params.original_method.empty() + ? request_params.original_method + : common_params.method; WebURLRequest request(navigation_url); request.SetHTTPMethod(WebString::FromUTF8(navigation_method)); @@ -476,7 +472,6 @@ return std::min(browser_navigation_start, renderer_navigation_start); } -// PlzNavigate CommonNavigationParams MakeCommonNavigationParams( const blink::WebFrameClient::NavigationPolicyInfo& info, int load_flags) { @@ -1713,7 +1708,6 @@ bool handled = true; IPC_BEGIN_MESSAGE_MAP(RenderFrameImpl, msg) - IPC_MESSAGE_HANDLER(FrameMsg_Navigate, OnNavigate) IPC_MESSAGE_HANDLER(FrameMsg_BeforeUnload, OnBeforeUnload) IPC_MESSAGE_HANDLER(FrameMsg_SwapOut, OnSwapOut) IPC_MESSAGE_HANDLER(FrameMsg_SwapIn, OnSwapIn) @@ -1836,20 +1830,6 @@ associated_interfaces_.BindRequest(interface_name, std::move(handle)); } -void RenderFrameImpl::OnNavigate( - const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, - const RequestNavigationParams& request_params) { - DCHECK(!IsBrowserSideNavigationEnabled()); - TRACE_EVENT2("navigation,rail", "RenderFrameImpl::OnNavigate", "id", - routing_id_, "url", common_params.url.possibly_invalid_spec()); - NavigateInternal(common_params, start_params, request_params, - std::unique_ptr<StreamOverrideParameters>(), - /*subresource_loader_factories=*/base::nullopt, - /* non-plznavigate navigations are not traced */ - base::UnguessableToken::Create()); -} - void RenderFrameImpl::BindEngagement( blink::mojom::EngagementClientAssociatedRequest request) { engagement_binding_.Bind(std::move(request)); @@ -2667,8 +2647,7 @@ // page load is regarded as the same browser initiated request. if (!navigation_state->IsContentInitiated()) { pending_navigation_params_.reset(new NavigationParams( - navigation_state->common_params(), navigation_state->start_params(), - navigation_state->request_params())); + navigation_state->common_params(), navigation_state->request_params())); } // Load an error page. @@ -3065,7 +3044,6 @@ mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, base::Optional<URLLoaderFactoryBundle> subresource_loader_factories, const base::UnguessableToken& devtools_navigation_token) { - CHECK(IsBrowserSideNavigationEnabled()); // If this was a renderer-initiated navigation (nav_entry_id == 0) from this // frame, but it was aborted, then ignore it. if (!browser_side_navigation_pending_ && @@ -3076,29 +3054,17 @@ return; } - // This will override the url requested by the WebURLLoader, as well as - // provide it with the response to the request. - std::unique_ptr<StreamOverrideParameters> stream_override( - new StreamOverrideParameters()); - stream_override->stream_url = body_url; - stream_override->url_loader_client_endpoints = - std::move(url_loader_client_endpoints); - stream_override->response = head; - stream_override->redirects = request_params.redirects; - stream_override->redirect_responses = request_params.redirect_response; - stream_override->redirect_infos = request_params.redirect_infos; - - // Used to notify the browser that it can release its |stream_handle_| when - // the |stream_override| object isn't used anymore. - // TODO(clamy): Remove this when we switch to Mojo streams. - stream_override->on_delete = base::BindOnce( - [](base::WeakPtr<RenderFrameImpl> weak_self, const GURL& url) { - if (RenderFrameImpl* self = weak_self.get()) { - self->Send( - new FrameHostMsg_StreamHandleConsumed(self->routing_id_, url)); - } - }, - weak_factory_.GetWeakPtr()); + // First, check if this is a Debug URL. If so, handle it and stop the + // navigation right away. + base::WeakPtr<RenderFrameImpl> weak_this = weak_factory_.GetWeakPtr(); + if (MaybeHandleDebugURL(common_params.url)) { + // The browser expects the frame to be loading the requested URL. Inform it + // that the load stopped if needed, while leaving the debug URL visible in + // the address bar. + if (weak_this && frame_ && !frame_->IsLoading()) + Send(new FrameHostMsg_DidStopLoading(routing_id_)); + return; + } // If the request was initiated in the context of a user gesture then make // sure that the navigation also executes in the context of a user gesture. @@ -3109,13 +3075,219 @@ browser_side_navigation_pending_ = false; browser_side_navigation_pending_url_ = GURL(); - NavigateInternal(common_params, StartNavigationParams(), request_params, - std::move(stream_override), - std::move(subresource_loader_factories), + // Clear pending navigations which weren't sent to the browser because we + // did not get a didStartProvisionalLoad() notification for them. + pending_navigation_info_.reset(nullptr); + + // Lower bound for browser initiated navigation start time. + base::TimeTicks renderer_navigation_start = base::TimeTicks::Now(); + + bool is_reload = + FrameMsg_Navigate_Type::IsReload(common_params.navigation_type); + bool is_history_navigation = request_params.page_state.IsValid(); + auto cache_mode = blink::mojom::FetchCacheMode::kDefault; + RenderFrameImpl::PrepareRenderViewForNavigation(common_params.url, + request_params); + + GetContentClient()->SetActiveURL( + common_params.url, frame_->Top()->GetSecurityOrigin().ToString().Utf8()); + + // If this frame is navigating cross-process, it may naively assume that this + // is the first navigation in the frame, but this may not actually be the + // case. Inform the frame's state machine if this frame has already committed + // other loads. + if (request_params.has_committed_real_load) + frame_->SetCommittedFirstRealLoad(); + + // TODO(clamy): This may not be needed now that PlzNavigate has shipped. + if (is_reload && current_history_item_.IsNull()) { + // We cannot reload if we do not have any history state. This happens, for + // example, when recovering from a crash. + is_reload = false; + cache_mode = blink::mojom::FetchCacheMode::kValidateCache; + } + + // If the navigation is for "view source", the WebLocalFrame needs to be put + // in a special mode. + if (request_params.is_view_source) + frame_->EnableViewSourceMode(true); + + pending_navigation_params_.reset( + new NavigationParams(common_params, request_params)); + + // Sanitize navigation start and store in |pending_navigation_params_|. + // It will be picked up in UpdateNavigationState. + pending_navigation_params_->common_params.navigation_start = + SanitizeNavigationTiming(common_params.navigation_start, + renderer_navigation_start); + + // Create parameters for a standard navigation, indicating whether it should + // replace the current NavigationEntry. + blink::WebFrameLoadType load_type = + common_params.should_replace_current_entry + ? blink::WebFrameLoadType::kReplaceCurrentItem + : blink::WebFrameLoadType::kStandard; + blink::WebHistoryLoadType history_load_type = + blink::kWebHistoryDifferentDocumentLoad; + bool should_load_request = false; + WebHistoryItem item_for_history_navigation; + bool is_same_document = + FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type); + + // Sanity check that the browser always sends us new loader factories on + // cross-document navigations with the Network Service enabled. + DCHECK(is_same_document || + common_params.url.SchemeIs(url::kJavaScriptScheme) || + !base::FeatureList::IsEnabled(features::kNetworkService) || + subresource_loader_factories.has_value()); + + if (subresource_loader_factories) + subresource_loader_factories_ = std::move(subresource_loader_factories); + + // If the Network Service is enabled, by this point the frame should always + // have subresource loader factories, even if they're from a previous (but + // same-document) commit. + DCHECK(!base::FeatureList::IsEnabled(features::kNetworkService) || + subresource_loader_factories_.has_value()); + + // Used to determine whether this frame is actually loading a request as part + // of a history navigation. + bool has_history_navigation_in_frame = false; + + // If we are reloading, then use the history state of the current frame. + // Otherwise, if we have history state, then we need to navigate to it, which + // corresponds to a back/forward navigation event. Update the parameters + // depending on the navigation type. + if (is_reload) { + load_type = ReloadFrameLoadTypeFor(common_params.navigation_type); + should_load_request = true; + } else if (is_history_navigation) { + // We must know the nav entry ID of the page we are navigating back to, + // which should be the case because history navigations are routed via the + // browser. + DCHECK_NE(0, request_params.nav_entry_id); + std::unique_ptr<HistoryEntry> entry = + PageStateToHistoryEntry(request_params.page_state); + if (entry) { + // The browser process sends a single WebHistoryItem for this frame. + // TODO(creis): Change PageState to FrameState. In the meantime, we + // store the relevant frame's WebHistoryItem in the root of the + // PageState. + item_for_history_navigation = entry->root(); + switch (common_params.navigation_type) { + case FrameMsg_Navigate_Type::RELOAD: + case FrameMsg_Navigate_Type::RELOAD_BYPASSING_CACHE: + case FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL: + case FrameMsg_Navigate_Type::RESTORE: + case FrameMsg_Navigate_Type::RESTORE_WITH_POST: + case FrameMsg_Navigate_Type::HISTORY_DIFFERENT_DOCUMENT: + history_load_type = blink::kWebHistoryDifferentDocumentLoad; + break; + case FrameMsg_Navigate_Type::HISTORY_SAME_DOCUMENT: + history_load_type = blink::kWebHistorySameDocumentLoad; + break; + default: + NOTREACHED(); + history_load_type = blink::kWebHistoryDifferentDocumentLoad; + } + load_type = request_params.is_history_navigation_in_new_child + ? blink::WebFrameLoadType::kInitialHistoryLoad + : blink::WebFrameLoadType::kBackForward; + should_load_request = true; + + // Keep track of which subframes the browser process has history items + // for during a history navigation. + history_subframe_unique_names_ = request_params.subframe_unique_names; + + if (history_load_type == blink::kWebHistorySameDocumentLoad) { + // If this is marked as a same document load but we haven't committed + // anything, treat it as a new load. The browser shouldn't let this + // happen. + if (current_history_item_.IsNull()) { + history_load_type = blink::kWebHistoryDifferentDocumentLoad; + NOTREACHED(); + } else { + // Additionally, if the |current_history_item_|'s document + // sequence number doesn't match the one sent from the browser, it + // is possible that this renderer has committed a different + // document. In such case, don't use WebHistorySameDocumentLoad. + if (current_history_item_.DocumentSequenceNumber() != + item_for_history_navigation.DocumentSequenceNumber()) { + history_load_type = blink::kWebHistoryDifferentDocumentLoad; + } + } + } + + // If this navigation is to a history item for a new child frame, we may + // want to ignore it in some cases. If a Javascript navigation (i.e., + // client redirect) interrupted it and has either been scheduled, + // started loading, or has committed, we should ignore the history item. + bool interrupted_by_client_redirect = + frame_->IsNavigationScheduledWithin(0) || + frame_->GetProvisionalDocumentLoader() || + !current_history_item_.IsNull(); + if (request_params.is_history_navigation_in_new_child && + interrupted_by_client_redirect) { + should_load_request = false; + has_history_navigation_in_frame = false; + } + } + } else { + // Navigate to the given URL. + should_load_request = true; + } + + if (should_load_request) { + // Check if the navigation being committed originated as a client redirect. + bool is_client_redirect = + !!(common_params.transition & ui::PAGE_TRANSITION_CLIENT_REDIRECT); + + // Perform a navigation for loadDataWithBaseURL if needed (for main frames). + // Note: the base URL might be invalid, so also check the data URL string. + bool should_load_data_url = !common_params.base_url_for_data_url.is_empty(); +#if defined(OS_ANDROID) + should_load_data_url |= !request_params.data_url_as_string.empty(); +#endif + if (is_main_frame_ && should_load_data_url) { + LoadDataURL(common_params, request_params, frame_, load_type, + item_for_history_navigation, history_load_type, + is_client_redirect); + } else { + WebURLRequest request = CreateURLRequestForCommit( + common_params, request_params, std::move(url_loader_client_endpoints), + head, body_url, is_same_document); + + // Load the request. + frame_->Load(request, load_type, item_for_history_navigation, + history_load_type, is_client_redirect, devtools_navigation_token); - // Don't add code after this since NavigateInternal may have destroyed this - // RenderFrameImpl. + // The load of the URL can result in this frame being removed. Use a + // WeakPtr as an easy way to detect whether this has occured. If so, this + // method should return immediately and not touch any part of the object, + // otherwise it will result in a use-after-free bug. + if (!weak_this) + return; + } + } else { + // The browser expects the frame to be loading this navigation. Inform it + // that the load stopped if needed. + // Note: in the case of history navigations, |should_load_request| will be + // false, and the frame may not have been set in a loading state. Do not + // send a stop message if a history navigation is loading in this frame + // nonetheless. This behavior will go away with subframe navigation + // entries. + if (frame_ && !frame_->IsLoading() && !has_history_navigation_in_frame) + Send(new FrameHostMsg_DidStopLoading(routing_id_)); + } + + // In case LoadRequest failed before DidCreateDocumentLoader was called. + pending_navigation_params_.reset(); + + // Reset the source location now that the commit checks have been processed. + frame_->GetDocumentLoader()->ResetSourceLocation(); + if (frame_->GetProvisionalDocumentLoader()) + frame_->GetProvisionalDocumentLoader()->ResetSourceLocation(); } void RenderFrameImpl::CommitFailedNavigation( @@ -3125,7 +3297,6 @@ int error_code, const base::Optional<std::string>& error_page_content, base::Optional<URLLoaderFactoryBundle> subresource_loader_factories) { - DCHECK(IsBrowserSideNavigationEnabled()); bool is_reload = FrameMsg_Navigate_Type::IsReload(common_params.navigation_type); RenderFrameImpl::PrepareRenderViewForNavigation(common_params.url, @@ -3144,8 +3315,8 @@ if (request_params.has_committed_real_load) frame_->SetCommittedFirstRealLoad(); - pending_navigation_params_.reset(new NavigationParams( - common_params, StartNavigationParams(), request_params)); + pending_navigation_params_.reset( + new NavigationParams(common_params, request_params)); // Send the provisional load failure. WebURLError error( @@ -3792,14 +3963,6 @@ blink::WebDocumentLoader* document_loader) { bool content_initiated = !pending_navigation_params_.get(); - // Make sure any previous redirect URLs end up in our new data source. - if (pending_navigation_params_.get() && !IsBrowserSideNavigationEnabled()) { - for (const auto& i : - pending_navigation_params_->request_params.redirects) { - document_loader->AppendRedirect(i); - } - } - DocumentState* document_state = DocumentState::FromDocumentLoader(document_loader); if (!document_state) { @@ -3840,10 +4003,9 @@ document_loader->SetNavigationStartTime( ConvertToBlinkTime(navigation_state->common_params().navigation_start)); - // PlzNavigate: if an actual navigation took place, inform the document - // loader of what happened in the browser. - if (IsBrowserSideNavigationEnabled() && - !navigation_state->request_params() + // If an actual navigation took place, inform the document loader of what + // happened in the browser. + if (!navigation_state->request_params() .navigation_timing.fetch_start.is_null()) { // Set timing of several events that happened during navigation. // They will be used in blink for the Navigation Timing API. @@ -3859,10 +4021,8 @@ !navigation_state->request_params().redirects.empty()); } - // PlzNavigate: update the source location before processing the navigation - // commit. - if (IsBrowserSideNavigationEnabled() && - navigation_state->common_params().source_location.has_value()) { + // Update the source location before processing the navigation commit. + if (navigation_state->common_params().source_location.has_value()) { blink::WebSourceLocation source_location; source_location.url = WebString::FromLatin1( navigation_state->common_params().source_location->url); @@ -3899,10 +4059,8 @@ "RenderFrameImpl::didStartProvisionalLoad", "id", routing_id_, "url", document_loader->GetRequest().Url().GetString().Utf8()); - // PlzNavigate: // If we have a pending navigation to be sent to the browser send it here. if (pending_navigation_info_.get()) { - DCHECK(IsBrowserSideNavigationEnabled()); NavigationPolicyInfo info(request); info.navigation_type = pending_navigation_info_->navigation_type; info.default_policy = pending_navigation_info_->policy; @@ -4670,7 +4828,7 @@ // user agent on its own. Similarly, it may indicate that we should set an // X-Requested-With header. This must be done here to avoid breaking CORS // checks. - // PlzNavigate: there may also be a stream url associated with the request. + // There may also be a stream url associated with the request. WebString custom_user_agent; WebString requested_with; std::unique_ptr<StreamOverrideParameters> stream_override; @@ -4736,11 +4894,6 @@ bool is_navigational_request = request.GetFrameType() != network::mojom::RequestContextFrameType::kNone; if (is_navigational_request) { - extra_data->set_transferred_request_child_id( - navigation_state->start_params().transferred_request_child_id); - extra_data->set_transferred_request_request_id( - navigation_state->start_params().transferred_request_request_id); - // For navigation requests, we should copy the flag which indicates if this // was a navigation initiated by the renderer to the new RequestExtraData // instance. @@ -4785,28 +4938,6 @@ request.SetHasUserGesture( WebUserGestureIndicator::IsProcessingUserGesture(frame_)); - // StartNavigationParams should only apply to navigational requests (and not - // to subresource requests). For example - Content-Type header provided via - // OpenURLParams::extra_headers should only be applied to the original POST - // navigation request (and not to subresource requests). - if (is_navigational_request && - !navigation_state->start_params().extra_headers.empty()) { - for (net::HttpUtil::HeadersIterator i( - navigation_state->start_params().extra_headers.begin(), - navigation_state->start_params().extra_headers.end(), "\n"); - i.GetNext();) { - if (base::LowerCaseEqualsASCII(i.name(), "referer")) { - WebString referrer = WebSecurityPolicy::GenerateReferrerHeader( - blink::kWebReferrerPolicyDefault, request.Url(), - WebString::FromUTF8(i.values())); - request.SetHTTPReferrer(referrer, blink::kWebReferrerPolicyDefault); - } else { - request.SetHTTPHeaderField(WebString::FromUTF8(i.name()), - WebString::FromUTF8(i.values())); - } - } - } - if (!render_view_->renderer_preferences_.enable_referrers) request.SetHTTPReferrer(WebString(), blink::kWebReferrerPolicyDefault); } @@ -5516,9 +5647,10 @@ "id", routing_id_); render_view_->FrameDidStartLoading(frame_); - // PlzNavigate: the browser is responsible for knowing the start of all - // non-synchronous navigations. - if (!IsBrowserSideNavigationEnabled() || !to_different_document) + // The browser is responsible for knowing the start of all non-synchronous + // navigations. + // TODO(clamy): Remove this IPC. + if (!to_different_document) Send(new FrameHostMsg_DidStartLoading(routing_id_, to_different_document)); } @@ -5581,7 +5713,6 @@ render_accessibility()->AccessibilityFocusedNodeChanged(node); } -// PlzNavigate void RenderFrameImpl::OnReportContentSecurityPolicyViolation( const content::CSPViolationParams& violation_params) { frame_->ReportContentSecurityPolicyViolation( @@ -5606,22 +5737,20 @@ // context and they're trying to navigate to a different context. const GURL& url = info.url_request.Url(); - // With PlzNavigate, the redirect list is available for the first url. So - // maintain the old behavior of not classifying the first URL in the chain as - // a redirect. + // The redirect list is available for the first url. We maintain the old + // behavior of not classifying the first URL in the chain as a redirect. bool is_redirect = info.extra_data || (pending_navigation_params_ && !pending_navigation_params_->request_params.redirects.empty() && - (!IsBrowserSideNavigationEnabled() || - url != pending_navigation_params_->request_params.redirects[0])); + url != pending_navigation_params_->request_params.redirects[0]); #ifdef OS_ANDROID bool render_view_was_created_by_renderer = render_view_->was_created_by_renderer_; // The handlenavigation API is deprecated and will be removed once // crbug.com/325351 is resolved. - if ((!IsBrowserSideNavigationEnabled() || !IsURLHandledByNetworkStack(url)) && + if (!IsURLHandledByNetworkStack(url) && GetContentClient()->renderer()->HandleNavigation( this, is_content_initiated, render_view_was_created_by_renderer, frame_, info.url_request, info.navigation_type, info.default_policy, @@ -5778,9 +5907,8 @@ // There is no need to execute the BeforeUnload event during a redirect, // since it was already executed at the start of the navigation. !is_redirect && - // PlzNavigate: this should not be executed when commiting the navigation. - (!IsBrowserSideNavigationEnabled() || - info.url_request.CheckForBrowserSideNavigation()) && + // This should not be executed when commiting the navigation. + info.url_request.CheckForBrowserSideNavigation() && // No need to dispatch beforeunload if the frame has not committed a // navigation and contains an empty initial document. (has_accessed_initial_document_ || !current_history_item_.IsNull()); @@ -5812,10 +5940,9 @@ (info.archive_status == NavigationPolicyInfo::ArchiveStatus::Present) && !url.SchemeIs(url::kDataScheme); - // PlzNavigate: if the navigation is not synchronous, send it to the browser. - // This includes navigations with no request being sent to the network stack. - if (IsBrowserSideNavigationEnabled() && - info.url_request.CheckForBrowserSideNavigation() && + // If the navigation is not synchronous, send it to the browser. This + // includes navigations with no request being sent to the network stack. + if (info.url_request.CheckForBrowserSideNavigation() && IsURLHandledByNetworkStack(url) && !use_archive) { if (info.default_policy == blink::kWebNavigationPolicyCurrentTab) { // The BeginNavigation() call happens in didStartProvisionalLoad(). We @@ -6242,113 +6369,45 @@ Send(new FrameHostMsg_OpenURL(routing_id_, params)); } -void RenderFrameImpl::NavigateInternal( +WebURLRequest RenderFrameImpl::CreateURLRequestForCommit( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params, - std::unique_ptr<StreamOverrideParameters> stream_params, - base::Optional<URLLoaderFactoryBundle> subresource_loader_factories, - const base::UnguessableToken& devtools_navigation_token) { - bool browser_side_navigation = IsBrowserSideNavigationEnabled(); + mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + const ResourceResponseHead& head, + const GURL& body_url, + bool is_same_document_navigation) { + // This will override the url requested by the WebURLLoader, as well as + // provide it with the response to the request. + std::unique_ptr<StreamOverrideParameters> stream_override( + new StreamOverrideParameters()); + stream_override->stream_url = body_url; + stream_override->url_loader_client_endpoints = + std::move(url_loader_client_endpoints); + stream_override->response = head; + stream_override->redirects = request_params.redirects; + stream_override->redirect_responses = request_params.redirect_response; + stream_override->redirect_infos = request_params.redirect_infos; - // First, check if this is a Debug URL. If so, handle it and stop the - // navigation right away. - base::WeakPtr<RenderFrameImpl> weak_this = weak_factory_.GetWeakPtr(); - if (MaybeHandleDebugURL(common_params.url)) { - // The browser expects the frame to be loading the requested URL. Inform it - // that the load stopped if needed, while leaving the debug URL visible in - // the address bar. - if (weak_this && frame_ && !frame_->IsLoading()) - Send(new FrameHostMsg_DidStopLoading(routing_id_)); - return; - } - - // PlzNavigate - // Clear pending navigations which weren't sent to the browser because we - // did not get a didStartProvisionalLoad() notification for them. - pending_navigation_info_.reset(nullptr); - - // Lower bound for browser initiated navigation start time. - base::TimeTicks renderer_navigation_start = base::TimeTicks::Now(); - bool is_reload = - FrameMsg_Navigate_Type::IsReload(common_params.navigation_type); - bool is_history_navigation = request_params.page_state.IsValid(); - auto cache_mode = blink::mojom::FetchCacheMode::kDefault; - RenderFrameImpl::PrepareRenderViewForNavigation( - common_params.url, request_params); - - GetContentClient()->SetActiveURL( - common_params.url, frame_->Top()->GetSecurityOrigin().ToString().Utf8()); - - // If this frame is navigating cross-process, it may naively assume that this - // is the first navigation in the frame, but this may not actually be the - // case. Inform the frame's state machine if this frame has already committed - // other loads. - if (request_params.has_committed_real_load) - frame_->SetCommittedFirstRealLoad(); - - if (is_reload && current_history_item_.IsNull()) { - // We cannot reload if we do not have any history state. This happens, for - // example, when recovering from a crash. - is_reload = false; - cache_mode = blink::mojom::FetchCacheMode::kValidateCache; - } - - // If the navigation is for "view source", the WebLocalFrame needs to be put - // in a special mode. - if (request_params.is_view_source) - frame_->EnableViewSourceMode(true); - - pending_navigation_params_.reset( - new NavigationParams(common_params, start_params, request_params)); - - // Sanitize navigation start and store in |pending_navigation_params_|. - // It will be picked up in UpdateNavigationState. - pending_navigation_params_->common_params.navigation_start = - SanitizeNavigationTiming(common_params.navigation_start, - renderer_navigation_start); - - // Create parameters for a standard navigation, indicating whether it should - // replace the current NavigationEntry. - blink::WebFrameLoadType load_type = - common_params.should_replace_current_entry - ? blink::WebFrameLoadType::kReplaceCurrentItem - : blink::WebFrameLoadType::kStandard; - blink::WebHistoryLoadType history_load_type = - blink::kWebHistoryDifferentDocumentLoad; - bool should_load_request = false; - WebHistoryItem item_for_history_navigation; - - // Enforce same-document navigation from the browser only if - // browser-side-navigation is enabled. - bool is_same_document = - IsBrowserSideNavigationEnabled() && - FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type); - - // Sanity check that the browser always sends us new loader factories on - // cross-document navigations with the Network Service enabled. - DCHECK(is_same_document || - common_params.url.SchemeIs(url::kJavaScriptScheme) || - !base::FeatureList::IsEnabled(features::kNetworkService) || - subresource_loader_factories.has_value()); - - if (subresource_loader_factories) - subresource_loader_factories_ = std::move(subresource_loader_factories); - - // If the Network Service is enabled, by this point the frame should always - // have subresource loader factories, even if they're from a previous (but - // same-document) commit. - DCHECK(!base::FeatureList::IsEnabled(features::kNetworkService) || - subresource_loader_factories_.has_value()); + // Used to notify the browser that it can release its |stream_handle_| when + // the |stream_override| object isn't used anymore. + // TODO(clamy): Remove this when we switch to Mojo streams. + stream_override->on_delete = base::BindOnce( + [](base::WeakPtr<RenderFrameImpl> weak_self, const GURL& url) { + if (RenderFrameImpl* self = weak_self.get()) { + self->Send( + new FrameHostMsg_StreamHandleConsumed(self->routing_id_, url)); + } + }, + weak_factory_.GetWeakPtr()); WebURLRequest request = CreateURLRequestForNavigation( - common_params, request_params, std::move(stream_params), - frame_->IsViewSourceModeEnabled(), is_same_document); + common_params, request_params, std::move(stream_override), + frame_->IsViewSourceModeEnabled(), is_same_document_navigation); request.SetFrameType(IsTopLevelNavigation(frame_) ? network::mojom::RequestContextFrameType::kTopLevel : network::mojom::RequestContextFrameType::kNested); - if (IsBrowserSideNavigationEnabled() && common_params.post_data) { + if (common_params.post_data) { request.SetHTTPBody(GetWebHTTPBodyForRequestBody(*common_params.post_data)); if (!request_params.post_content_type.empty()) { request.AddHTTPHeaderField( @@ -6357,193 +6416,18 @@ } } - // Used to determine whether this frame is actually loading a request as part - // of a history navigation. - bool has_history_navigation_in_frame = false; - #if defined(OS_ANDROID) request.SetHasUserGesture(common_params.has_user_gesture); #endif - if (browser_side_navigation) { - // PlzNavigate: Make sure that Blink's loader will not try to use browser - // side navigation for this request (since it already went to the browser). - request.SetCheckForBrowserSideNavigation(false); + // Make sure that Blink's loader will not try to use browser side navigation + // for this request (since it already went to the browser). + request.SetCheckForBrowserSideNavigation(false); - request.SetNavigationStartTime( - ConvertToBlinkTime(common_params.navigation_start)); - } + request.SetNavigationStartTime( + ConvertToBlinkTime(common_params.navigation_start)); - // If we are reloading, then use the history state of the current frame. - // Otherwise, if we have history state, then we need to navigate to it, which - // corresponds to a back/forward navigation event. Update the parameters - // depending on the navigation type. - if (is_reload) { - load_type = ReloadFrameLoadTypeFor(common_params.navigation_type); - - if (!browser_side_navigation) { - const GURL override_url = - (common_params.navigation_type == - FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL) - ? common_params.url - : GURL(); - request = frame_->RequestForReload(load_type, override_url); - } - should_load_request = true; - } else if (is_history_navigation) { - // We must know the nav entry ID of the page we are navigating back to, - // which should be the case because history navigations are routed via the - // browser. - DCHECK_NE(0, request_params.nav_entry_id); - std::unique_ptr<HistoryEntry> entry = - PageStateToHistoryEntry(request_params.page_state); - if (entry) { - // The browser process sends a single WebHistoryItem for this frame. - // TODO(creis): Change PageState to FrameState. In the meantime, we - // store the relevant frame's WebHistoryItem in the root of the - // PageState. - item_for_history_navigation = entry->root(); - switch (common_params.navigation_type) { - case FrameMsg_Navigate_Type::RELOAD: - case FrameMsg_Navigate_Type::RELOAD_BYPASSING_CACHE: - case FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL: - case FrameMsg_Navigate_Type::RESTORE: - case FrameMsg_Navigate_Type::RESTORE_WITH_POST: - case FrameMsg_Navigate_Type::HISTORY_DIFFERENT_DOCUMENT: - history_load_type = blink::kWebHistoryDifferentDocumentLoad; - break; - case FrameMsg_Navigate_Type::HISTORY_SAME_DOCUMENT: - history_load_type = blink::kWebHistorySameDocumentLoad; - break; - default: - NOTREACHED(); - history_load_type = blink::kWebHistoryDifferentDocumentLoad; - } - load_type = request_params.is_history_navigation_in_new_child - ? blink::WebFrameLoadType::kInitialHistoryLoad - : blink::WebFrameLoadType::kBackForward; - should_load_request = true; - - // Keep track of which subframes the browser process has history items - // for during a history navigation. - history_subframe_unique_names_ = request_params.subframe_unique_names; - - if (history_load_type == blink::kWebHistorySameDocumentLoad) { - // If this is marked as a same document load but we haven't committed - // anything, treat it as a new load. The browser shouldn't let this - // happen. - if (current_history_item_.IsNull()) { - history_load_type = blink::kWebHistoryDifferentDocumentLoad; - NOTREACHED(); - } else { - // Additionally, if the |current_history_item_|'s document - // sequence number doesn't match the one sent from the browser, it - // is possible that this renderer has committed a different - // document. In such case, don't use WebHistorySameDocumentLoad. - if (current_history_item_.DocumentSequenceNumber() != - item_for_history_navigation.DocumentSequenceNumber()) { - history_load_type = blink::kWebHistoryDifferentDocumentLoad; - } - } - } - - // If this navigation is to a history item for a new child frame, we may - // want to ignore it in some cases. If a Javascript navigation (i.e., - // client redirect) interrupted it and has either been scheduled, - // started loading, or has committed, we should ignore the history item. - bool interrupted_by_client_redirect = - frame_->IsNavigationScheduledWithin(0) || - frame_->GetProvisionalDocumentLoader() || - !current_history_item_.IsNull(); - if (request_params.is_history_navigation_in_new_child && - interrupted_by_client_redirect) { - should_load_request = false; - has_history_navigation_in_frame = false; - } - - // Generate the request for the load from the HistoryItem. - // PlzNavigate: use the data sent by the browser for the url and the - // HTTP state. The restoration of user state such as scroll position - // will be done based on the history item during the load. - if (!browser_side_navigation && should_load_request) { - request = frame_->RequestFromHistoryItem(item_for_history_navigation, - cache_mode); - } - } - } else { - // Navigate to the given URL. - if (!start_params.extra_headers.empty() && !browser_side_navigation) { - for (net::HttpUtil::HeadersIterator i(start_params.extra_headers.begin(), - start_params.extra_headers.end(), - "\n"); - i.GetNext();) { - request.AddHTTPHeaderField(WebString::FromUTF8(i.name()), - WebString::FromUTF8(i.values())); - } - } - - if (common_params.method == "POST" && !browser_side_navigation && - common_params.post_data) { - request.SetHTTPBody( - GetWebHTTPBodyForRequestBody(*common_params.post_data)); - } - - should_load_request = true; - } - - if (should_load_request) { - // PlzNavigate: check if the navigation being committed originated as a - // client redirect. - bool is_client_redirect = - browser_side_navigation - ? !!(common_params.transition & ui::PAGE_TRANSITION_CLIENT_REDIRECT) - : false; - - // Perform a navigation for loadDataWithBaseURL if needed (for main frames). - // Note: the base URL might be invalid, so also check the data URL string. - bool should_load_data_url = !common_params.base_url_for_data_url.is_empty(); -#if defined(OS_ANDROID) - should_load_data_url |= !request_params.data_url_as_string.empty(); -#endif - if (is_main_frame_ && should_load_data_url) { - LoadDataURL(common_params, request_params, frame_, load_type, - item_for_history_navigation, history_load_type, - is_client_redirect); - } else { - // Load the request. - frame_->Load(request, load_type, item_for_history_navigation, - history_load_type, is_client_redirect, - devtools_navigation_token); - - // The load of the URL can result in this frame being removed. Use a - // WeakPtr as an easy way to detect whether this has occured. If so, this - // method should return immediately and not touch any part of the object, - // otherwise it will result in a use-after-free bug. - if (!weak_this) - return; - } - } else { - // The browser expects the frame to be loading this navigation. Inform it - // that the load stopped if needed. - // Note: in the case of history navigations, |should_load_request| will be - // false, and the frame may not have been set in a loading state. Do not - // send a stop message if a history navigation is loading in this frame - // nonetheless. This behavior will go away with subframe navigation - // entries. - if (frame_ && !frame_->IsLoading() && !has_history_navigation_in_frame) - Send(new FrameHostMsg_DidStopLoading(routing_id_)); - } - - // In case LoadRequest failed before DidCreateDocumentLoader was called. - pending_navigation_params_.reset(); - - // PlzNavigate: reset the source location now that the commit checks have been - // processed. - if (IsBrowserSideNavigationEnabled()) { - frame_->GetDocumentLoader()->ResetSourceLocation(); - if (frame_->GetProvisionalDocumentLoader()) - frame_->GetProvisionalDocumentLoader()->ResetSourceLocation(); - } + return request; } URLLoaderFactoryBundle& RenderFrameImpl::GetSubresourceLoaderFactories() { @@ -6699,7 +6583,6 @@ } void RenderFrameImpl::BeginNavigation(const NavigationPolicyInfo& info) { - CHECK(IsBrowserSideNavigationEnabled()); browser_side_navigation_pending_ = true; browser_side_navigation_pending_url_ = info.url_request.Url(); @@ -6936,7 +6819,6 @@ if (IsBrowserInitiated(pending_navigation_params_.get())) { return NavigationStateImpl::CreateBrowserInitiated( pending_navigation_params_->common_params, - pending_navigation_params_->start_params, pending_navigation_params_->request_params); } return NavigationStateImpl::CreateContentInitiated();
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index ab19f4f7..e587bb93 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h
@@ -175,8 +175,6 @@ struct RequestNavigationParams; struct ResourceResponseHead; struct ScreenInfo; -struct StartNavigationParams; -struct StreamOverrideParameters; class CONTENT_EXPORT RenderFrameImpl : public RenderFrame, @@ -967,9 +965,6 @@ // // The documentation for these functions should be in // content/common/*_messages.h for the message that the function is handling. - void OnNavigate(const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, - const RequestNavigationParams& request_params); void OnBeforeUnload(bool is_reload); void OnSwapIn(); void OnSwapOut(int proxy_routing_id, @@ -1092,21 +1087,14 @@ bool send_referrer, bool is_history_navigation_in_new_child); - // Performs a navigation in the frame. This provides a unified function for - // the current code path and the browser-side navigation path (in - // development). Currently used by OnNavigate, with all *NavigationParams - // provided by the browser. |stream_params| should be null. - // PlzNavigate: used by CommitNavigation, with |common_params| and - // |request_params| received by the browser. |stream_params| should be non - // null and created from the information provided by the browser. - // |start_params| is not used. - void NavigateInternal( + // Creates a WebURLRequest to use fo the commit of a navigation. + blink::WebURLRequest CreateURLRequestForCommit( const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params, - std::unique_ptr<StreamOverrideParameters> stream_params, - base::Optional<URLLoaderFactoryBundle> subresource_loader_factories, - const base::UnguessableToken& devtools_navigation_token); + mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints, + const ResourceResponseHead& head, + const GURL& body_url, + bool is_same_document_navigation); // Returns a URLLoaderFactoryBundle which can be used to request subresources // for this frame. Only valid to call when the Network Service is enabled. @@ -1139,7 +1127,7 @@ base::string16* result); // Loads the appropriate error page for the specified failure into the frame. - // |entry| is only used by PlzNavigate when navigating to a history item. + // |entry| is only when navigating to a history item. void LoadNavigationErrorPage( const blink::WebURLRequest& failed_request, const blink::WebURLError& error, @@ -1175,7 +1163,6 @@ const GURL& url, const RequestNavigationParams& request_params); - // PlzNavigate // Sends a FrameHostMsg_BeginNavigation to the browser void BeginNavigation(const NavigationPolicyInfo& info); @@ -1540,7 +1527,6 @@ // See BindingsPolicy for details. int enabled_bindings_ = 0; - // PlzNavigate: // Contains information about a pending navigation to be sent to the browser. // We save information about the navigation in decidePolicyForNavigation(). // The navigation is sent to the browser in didStartProvisionalLoad(). @@ -1559,9 +1545,9 @@ explicit PendingNavigationInfo(const NavigationPolicyInfo& info); }; - // PlzNavigate: Contains information about a pending navigation to be sent to - // the browser. This state is allocated in decidePolicyForNavigation() and - // is used and released in didStartProvisionalLoad(). + // Contains information about a pending navigation to be sent to the browser. + // This state is allocated in decidePolicyForNavigation() and is used and + // released in didStartProvisionalLoad(). std::unique_ptr<PendingNavigationInfo> pending_navigation_info_; service_manager::BindSourceInfo browser_info_;
diff --git a/content/renderer/render_frame_impl_browsertest.cc b/content/renderer/render_frame_impl_browsertest.cc index 10d237324..316a83d 100644 --- a/content/renderer/render_frame_impl_browsertest.cc +++ b/content/renderer/render_frame_impl_browsertest.cc
@@ -406,10 +406,7 @@ common_params.url = GURL("data:text/html,min_zoomlimit_test"); common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; GetMainRenderFrame()->SetHostZoomLevel(common_params.url, kMinZoomLevel); - GetMainRenderFrame()->NavigateInternal( - common_params, StartNavigationParams(), RequestNavigationParams(), - std::unique_ptr<StreamOverrideParameters>(), URLLoaderFactoryBundle(), - base::UnguessableToken::Create()); + GetMainRenderFrame()->Navigate(common_params, RequestNavigationParams()); base::RunLoop().RunUntilIdle(); EXPECT_DOUBLE_EQ(kMinZoomLevel, view_->GetWebView()->ZoomLevel()); @@ -418,10 +415,7 @@ ZoomFactorToZoomLevel(1.0)); common_params.url = GURL("data:text/html,max_zoomlimit_test"); GetMainRenderFrame()->SetHostZoomLevel(common_params.url, kMaxZoomLevel); - GetMainRenderFrame()->NavigateInternal( - common_params, StartNavigationParams(), RequestNavigationParams(), - std::unique_ptr<StreamOverrideParameters>(), URLLoaderFactoryBundle(), - base::UnguessableToken::Create()); + GetMainRenderFrame()->Navigate(common_params, RequestNavigationParams()); base::RunLoop().RunUntilIdle(); EXPECT_DOUBLE_EQ(kMaxZoomLevel, view_->GetWebView()->ZoomLevel()); }
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc index b1def01..b9cf940 100644 --- a/content/renderer/render_view_browsertest.cc +++ b/content/renderer/render_view_browsertest.cc
@@ -246,7 +246,6 @@ void GoToOffsetWithParams(int offset, const PageState& state, const CommonNavigationParams common_params, - const StartNavigationParams start_params, RequestNavigationParams request_params) { EXPECT_TRUE(common_params.transition & ui::PAGE_TRANSITION_FORWARD_BACK); int pending_offset = offset + view()->history_list_offset_; @@ -256,7 +255,7 @@ request_params.pending_history_list_offset = pending_offset; request_params.current_history_list_offset = view()->history_list_offset_; request_params.current_history_list_length = view()->history_list_length_; - frame()->Navigate(common_params, start_params, request_params); + frame()->Navigate(common_params, request_params); // The load actually happens asynchronously, so we pump messages to process // the pending continuation. @@ -514,7 +513,6 @@ TEST_F(RenderViewImplTest, OnNavigationHttpPost) { // An http url will trigger a resource load so cannot be used here. CommonNavigationParams common_params; - StartNavigationParams start_params; RequestNavigationParams request_params; common_params.url = GURL("data:text/html,<div>Page</div>"); common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; @@ -528,7 +526,7 @@ post_data->AppendBytes(raw_data, length); common_params.post_data = post_data; - frame()->Navigate(common_params, start_params, request_params); + frame()->Navigate(common_params, request_params); base::RunLoop().RunUntilIdle(); auto last_commit_params = frame()->TakeLastCommitParams(); @@ -570,8 +568,7 @@ "data:text/html,<html><head><title>Data page</title></head></html>"; render_thread_->sink().ClearMessages(); - frame()->Navigate(common_params, StartNavigationParams(), - request_params); + frame()->Navigate(common_params, request_params); const IPC::Message* frame_title_msg = nullptr; do { base::RunLoop().RunUntilIdle(); @@ -807,13 +804,12 @@ // Navigate to other page, which triggers the swap in. CommonNavigationParams common_params; - StartNavigationParams start_params; RequestNavigationParams request_params; common_params.url = GURL("data:text/html,<div>Page</div>"); common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.transition = ui::PAGE_TRANSITION_TYPED; - provisional_frame->Navigate(common_params, start_params, request_params); + provisional_frame->Navigate(common_params, request_params); EXPECT_EQ(device_scale, view()->GetDeviceScaleFactor()); EXPECT_EQ(device_scale, view()->webview()->ZoomFactorForDeviceScaleFactor()); @@ -953,7 +949,7 @@ request_params_C.pending_history_list_offset = 2; request_params_C.nav_entry_id = 3; request_params_C.page_state = state_C; - frame()->Navigate(common_params_C, StartNavigationParams(), request_params_C); + frame()->Navigate(common_params_C, request_params_C); base::RunLoop().RunUntilIdle(); render_thread_->sink().ClearMessages(); @@ -972,7 +968,7 @@ request_params_B.pending_history_list_offset = 1; request_params_B.nav_entry_id = 2; request_params_B.page_state = state_B; - frame()->Navigate(common_params_B, StartNavigationParams(), request_params_B); + frame()->Navigate(common_params_B, request_params_B); // Back to page A and commit. CommonNavigationParams common_params; @@ -985,7 +981,7 @@ request_params.pending_history_list_offset = 0; request_params.nav_entry_id = 1; request_params.page_state = state_A; - frame()->Navigate(common_params, StartNavigationParams(), request_params); + frame()->Navigate(common_params, request_params); base::RunLoop().RunUntilIdle(); // Now ensure that the UpdateState message we receive is consistent @@ -1311,8 +1307,7 @@ CommonNavigationParams common_params; common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.url = GURL("data:text/html,test data"); - frame()->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(common_params, RequestNavigationParams()); // An error occurred. view()->GetMainRenderFrame()->DidFailProvisionalLoad( @@ -1331,8 +1326,7 @@ CommonNavigationParams common_params; common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.url = GURL("data:text/html,test data"); - frame()->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(common_params, RequestNavigationParams()); // A cancellation occurred. view()->GetMainRenderFrame()->DidFailProvisionalLoad( @@ -1703,7 +1697,7 @@ TestRenderFrame* subframe = static_cast<TestRenderFrame*>(RenderFrameImpl::FromWebFrame( frame()->GetWebFrame()->FindFrameByName("frame"))); - subframe->Navigate(common_params, StartNavigationParams(), request_params); + subframe->Navigate(common_params, request_params); FrameLoadWaiter(subframe).Wait(); // Copy the document content to std::wstring and compare with the @@ -1812,8 +1806,7 @@ common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.url = GURL("data:text/html,test data"); TestRenderFrame* main_frame = static_cast<TestRenderFrame*>(frame()); - main_frame->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + main_frame->Navigate(common_params, RequestNavigationParams()); // An error occurred. main_frame->DidFailProvisionalLoad(error, blink::kWebStandardCommit); @@ -1840,8 +1833,7 @@ common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.url = GURL("data:text/html,test data"); TestRenderFrame* main_frame = static_cast<TestRenderFrame*>(frame()); - main_frame->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + main_frame->Navigate(common_params, RequestNavigationParams()); // An error occurred. main_frame->DidFailProvisionalLoad(error, blink::kWebStandardCommit); @@ -1872,8 +1864,7 @@ common_params.navigation_type = FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT; common_params.url = GURL("data:text/html,test data"); TestRenderFrame* main_frame = static_cast<TestRenderFrame*>(frame()); - main_frame->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + main_frame->Navigate(common_params, RequestNavigationParams()); // Emulate a 4xx/5xx main resource response with an empty body. main_frame->DidReceiveResponse(response); @@ -2026,8 +2017,7 @@ TEST_F(RenderViewImplTest, BrowserNavigationStart) { auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1)); - frame()->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(common_params, RequestNavigationParams()); FrameHostMsg_DidStartProvisionalLoad::Param nav_params = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); EXPECT_EQ(common_params.navigation_start, std::get<2>(nav_params)); @@ -2043,8 +2033,7 @@ auto late_common_params = MakeCommonNavigationParams(TimeDelta::FromDays(42)); late_common_params.method = "POST"; - frame()->Navigate(late_common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(late_common_params, RequestNavigationParams()); base::RunLoop().RunUntilIdle(); base::Time after_navigation = base::Time::Now() + base::TimeDelta::FromDays(1); @@ -2064,8 +2053,7 @@ ExecuteJavaScriptForTests("document.title = 'Hi!';"); auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1)); - frame()->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(common_params, RequestNavigationParams()); FrameHostMsg_DidStartProvisionalLoad::Param nav_params = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); @@ -2090,8 +2078,7 @@ // The browser navigation_start should not be used because beforeunload will // be fired during Navigate. - frame()->Navigate(common_params, StartNavigationParams(), - RequestNavigationParams()); + frame()->Navigate(common_params, RequestNavigationParams()); FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); @@ -2124,7 +2111,7 @@ common_params_back.navigation_type = FrameMsg_Navigate_Type::HISTORY_DIFFERENT_DOCUMENT; GoToOffsetWithParams(-1, back_state, common_params_back, - StartNavigationParams(), RequestNavigationParams()); + RequestNavigationParams()); FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); if (!IsBrowserSideNavigationEnabled()) { @@ -2147,7 +2134,7 @@ common_params_forward.navigation_type = FrameMsg_Navigate_Type::HISTORY_DIFFERENT_DOCUMENT; GoToOffsetWithParams(1, forward_state, common_params_forward, - StartNavigationParams(), RequestNavigationParams()); + RequestNavigationParams()); FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params2 = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); if (!IsBrowserSideNavigationEnabled()) { @@ -2172,7 +2159,7 @@ request_params.pending_history_list_offset = 1; request_params.current_history_list_offset = 0; request_params.current_history_list_length = 1; - frame()->Navigate(common_params, StartNavigationParams(), request_params); + frame()->Navigate(common_params, request_params); FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params = ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>(); @@ -2225,8 +2212,7 @@ request_params.current_history_list_length = 2; request_params.current_history_list_offset = 1; request_params.pending_history_list_offset = 2; - frame()->Navigate(CommonNavigationParams(), StartNavigationParams(), - request_params); + frame()->Navigate(CommonNavigationParams(), request_params); // The history list in RenderView should have been updated. EXPECT_EQ(1, view()->HistoryBackListCount());
diff --git a/content/test/content_browser_test_test.cc b/content/test/content_browser_test_test.cc index 36bc096..7fbe345 100644 --- a/content/test/content_browser_test_test.cc +++ b/content/test/content_browser_test_test.cc
@@ -92,7 +92,7 @@ // "#0 0x0000007ea911 (...content_browsertests+0x7ea910)" std::string crash_string = #if !USE_EXTERNAL_SYMBOLIZER - "content::RenderFrameImpl::NavigateInternal"; + "content::RenderFrameImpl::CommitNavigation"; #else "#0 "; #endif
diff --git a/content/test/test_render_frame.cc b/content/test/test_render_frame.cc index ec055d31..bbf2703 100644 --- a/content/test/test_render_frame.cc +++ b/content/test/test_render_frame.cc
@@ -137,17 +137,10 @@ } void TestRenderFrame::Navigate(const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params) { - // PlzNavigate - if (IsBrowserSideNavigationEnabled()) { - CommitNavigation(ResourceResponseHead(), GURL(), common_params, - request_params, mojom::URLLoaderClientEndpointsPtr(), - URLLoaderFactoryBundle(), - base::UnguessableToken::Create()); - } else { - OnNavigate(common_params, start_params, request_params); - } + CommitNavigation(ResourceResponseHead(), GURL(), common_params, + request_params, mojom::URLLoaderClientEndpointsPtr(), + URLLoaderFactoryBundle(), base::UnguessableToken::Create()); } void TestRenderFrame::SwapOut(
diff --git a/content/test/test_render_frame.h b/content/test/test_render_frame.h index c6bd440b..1af576e 100644 --- a/content/test/test_render_frame.h +++ b/content/test/test_render_frame.h
@@ -22,7 +22,6 @@ struct CommonNavigationParams; class MockFrameHost; struct RequestNavigationParams; -struct StartNavigationParams; // A test class to use in RenderViewTests. class TestRenderFrame : public RenderFrameImpl { @@ -42,7 +41,6 @@ void WillSendRequest(blink::WebURLRequest& request) override; void Navigate(const CommonNavigationParams& common_params, - const StartNavigationParams& start_params, const RequestNavigationParams& request_params); void SwapOut(int proxy_routing_id, bool is_loading,
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h index 3455e3d7..27fc512 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.h
@@ -35,6 +35,9 @@ extern const CGFloat kLocationBarShadowInset; extern const CGFloat kIcongnitoLocationBackgroundColor; +// Location bar StackView constraints. +extern const CGFloat klocationBarStackViewBottomMargin; + // Progress Bar Height. extern const CGFloat kProgressBarHeight;
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm index ce3e7e2..56333210 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_constants.mm
@@ -25,6 +25,8 @@ const CGFloat kLocationBarShadowInset = 1.0f; const CGFloat kIcongnitoLocationBackgroundColor = 0x737373; +const CGFloat klocationBarStackViewBottomMargin = 8.0f; + const CGFloat kProgressBarHeight = 2.0f; const CGFloat kToolsMenuButtonWidth = 44.0f;
diff --git a/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm b/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm index c8b1a60..f7a029f 100644 --- a/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm +++ b/ios/chrome/browser/ui/toolbar/clean/toolbar_view_controller.mm
@@ -572,22 +572,18 @@ // LocationBarContainer View. UILayoutGuide* locationBarContainerSafeAreaGuide = SafeAreaLayoutGuideForView(self.locationBarContainer); - NSLayoutConstraint* locationBarContainerStackViewTopConstraint = - [self.locationBarContainerStackView.topAnchor - constraintEqualToAnchor:self.locationBarContainer.topAnchor]; [NSLayoutConstraint activateConstraints:@[ [self.locationBarContainerStackView.bottomAnchor - constraintEqualToAnchor:self.locationBarContainer.bottomAnchor], + constraintEqualToAnchor:self.view.bottomAnchor + constant:-(klocationBarStackViewBottomMargin + + kLocationBarVerticalMargin)], [self.locationBarContainerStackView.trailingAnchor constraintEqualToAnchor:locationBarContainerSafeAreaGuide .trailingAnchor], [self.locationBarContainerStackView.leadingAnchor constraintEqualToAnchor:locationBarContainerSafeAreaGuide .leadingAnchor], - locationBarContainerStackViewTopConstraint, ]]; - [self.regularToolbarConstraints - addObject:locationBarContainerStackViewTopConstraint]; } #pragma mark - Components Setup @@ -1064,9 +1060,6 @@ constraintEqualToAnchor:self.view.leadingAnchor], [self.locationBarContainer.trailingAnchor constraintEqualToAnchor:self.view.trailingAnchor], - [self.locationBarContainerStackView.topAnchor - constraintEqualToAnchor:self.topSafeAnchor - constant:kExpandedLocationBarVerticalMargin], ]; } return _expandedToolbarConstraints;
diff --git a/tools/mb/mb_config.pyl b/tools/mb/mb_config.pyl index 73b1aa47..1859af0 100644 --- a/tools/mb/mb_config.pyl +++ b/tools/mb/mb_config.pyl
@@ -1841,7 +1841,8 @@ 'gn_args': ('disable_file_support=true disable_ftp_support=true ' 'enable_websockets=false use_platform_icu_alternatives=true ' 'use_partition_alloc=false enable_reporting=false ' - 'include_transport_security_state_preload_list=false'), + 'include_transport_security_state_preload_list=false ' + 'clang_use_default_sample_profile=false'), }, 'cros_chrome_sdk': {
diff --git a/tools/perf/core/stacktrace_unittest.py b/tools/perf/core/stacktrace_unittest.py index d6f3cc29..7e222c3 100644 --- a/tools/perf/core/stacktrace_unittest.py +++ b/tools/perf/core/stacktrace_unittest.py
@@ -37,7 +37,7 @@ def testCrashMinimalSymbols(self): with self.assertRaises(exceptions.DevtoolsTargetCrashException) as c: self._tab.Navigate('chrome://crash', timeout=5) - self.assertIn('NavigateInternal', + self.assertIn('CommitNavigation', '\n'.join(c.exception.stack_trace)) # The breakpad file specific test only apply to platforms which use the