diff --git a/DEPS b/DEPS index c5e93403..1d44672 100644 --- a/DEPS +++ b/DEPS
@@ -40,7 +40,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': 'b66fa526b882f2472d731b42cba65fe05cea4268', + 'skia_revision': 'f49b1e0ad955c437675eae6e8bd64a2e0941e204', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. @@ -232,7 +232,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + '7867a4ee6b46931789aae005f33823ed250ae52d', # commit position 17226 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'c0a0f8d5f2d5b837b4c6ea447b0cdce86723e0d6', # commit position 17267 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index 5a54a77..5f4ec84 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc
@@ -483,6 +483,17 @@ RequestGlobalDump(dump_type, level_of_detail, MemoryDumpCallback()); } +bool MemoryDumpManager::IsDumpProviderRegisteredForTesting( + MemoryDumpProvider* provider) { + AutoLock lock(lock_); + + for (const auto& info : dump_providers_) { + if (info->dump_provider == provider) + return true; + } + return false; +} + void MemoryDumpManager::CreateProcessDump(const MemoryDumpRequestArgs& args, const MemoryDumpCallback& callback) { TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(kTraceCategory, "ProcessMemoryDump",
diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h index 92cc2f4..a50c7ac 100644 --- a/base/trace_event/memory_dump_manager.h +++ b/base/trace_event/memory_dump_manager.h
@@ -123,6 +123,9 @@ // Returns true if the dump mode is allowed for current tracing session. bool IsDumpModeAllowed(MemoryDumpLevelOfDetail dump_mode); + // Lets tests see if a dump provider is registered. + bool IsDumpProviderRegisteredForTesting(MemoryDumpProvider*); + // Returns the MemoryDumpSessionState object, which is shared by all the // ProcessMemoryDump and MemoryAllocatorDump instances through all the tracing // session lifetime.
diff --git a/chrome/android/java/res/layout/suggestions_site_tile_grid.xml b/chrome/android/java/res/layout/suggestions_site_tile_grid.xml index c95ef0b..e3cbb85e 100644 --- a/chrome/android/java/res/layout/suggestions_site_tile_grid.xml +++ b/chrome/android/java/res/layout/suggestions_site_tile_grid.xml
@@ -6,7 +6,7 @@ <org.chromium.chrome.browser.suggestions.TileGridLayout xmlns:android="http://schemas.android.com/apk/res/android" android:id="@+id/tile_grid_layout" - android:layout_width="wrap_content" + android:layout_width="match_parent" android:layout_height="wrap_content" android:layout_marginBottom="16dp" android:layout_marginStart="12dp"
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java index 2baaf90..87a5bf68 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java
@@ -38,10 +38,10 @@ * @param title The title of the WebAPK to display during installation. * @param token The token from WebAPK Minter Server. * @param url The start URL of the WebAPK to install. - * @param callback The callback to invoke when the install is either completed or failed. + * @param callback The callback to invoke when the install completes, times out or fails. */ void installAsync(String packageName, int version, String title, String token, String url, - Callback<Boolean> callback); + Callback<Integer> callback); /** * Calls the callback once the installation either succeeded or failed. @@ -49,10 +49,4 @@ * @param event The result of the install. */ void onGotInstallEvent(String packageName, @InstallerPackageEvent int event); - - /** - * Checks whether Google Play Install API is available. - * @param callback The callback to invoke when the check is done. - */ - void canInstallWebApk(Callback<Boolean> callback); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java index 6dbc0ea..1a00ca7 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
@@ -120,13 +120,10 @@ return; } - Callback<Boolean> callback = new Callback<Boolean>() { + Callback<Integer> callback = new Callback<Integer>() { @Override - public void onResult(Boolean success) { - // TODO(pkotwicz): Send WebApkInstallResult.PROBABLE_FAILURE result if - // install timed out. - WebApkInstaller.this.notify( - success ? WebApkInstallResult.SUCCESS : WebApkInstallResult.FAILURE); + public void onResult(Integer result) { + WebApkInstaller.this.notify(result); } }; mGooglePlayWebApkInstallDelegate.installAsync( @@ -180,13 +177,10 @@ return; } - Callback<Boolean> callback = new Callback<Boolean>() { + Callback<Integer> callback = new Callback<Integer>() { @Override - public void onResult(Boolean success) { - // TODO(pkotwicz): Send WebApkInstallResult.PROBABLE_FAILURE result if - // update timed out. - WebApkInstaller.this.notify( - success ? WebApkInstallResult.SUCCESS : WebApkInstallResult.FAILURE); + public void onResult(Integer result) { + WebApkInstaller.this.notify(result); } }; mGooglePlayWebApkInstallDelegate.installAsync(
diff --git a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc b/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc index 5bf14b1..0771664 100644 --- a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc +++ b/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
@@ -112,7 +112,9 @@ TestParameter(NOT_IN_GUEST_MODE, "audioRepeatAllModeSingleFileDrive"), TestParameter(NOT_IN_GUEST_MODE, "audioNoRepeatModeSingleFileDrive"), TestParameter(NOT_IN_GUEST_MODE, "audioRepeatOneModeSingleFileDrive"), - TestParameter(NOT_IN_GUEST_MODE, "audioRepeatAllModeMultipleFileDrive"), + // TODO(crbug.com/701922) Revive this flaky test. + // TestParameter(NOT_IN_GUEST_MODE, + // "audioRepeatAllModeMultipleFileDrive"), TestParameter(NOT_IN_GUEST_MODE, "audioNoRepeatModeMultipleFileDrive"), TestParameter(NOT_IN_GUEST_MODE, "audioRepeatOneModeMultipleFileDrive"))); @@ -175,8 +177,9 @@ #else #define MAYBE_Delete Delete #endif +// Flaky: crbug.com/699426 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_Delete, + DISABLE_Delete, FileManagerBrowserTest, ::testing::Values( TestParameter(NOT_IN_GUEST_MODE, @@ -193,6 +196,7 @@ #else #define MAYBE_DirectoryTreeContextMenu DirectoryTreeContextMenu #endif +// Flaky: crbug.com/700156, crbug.com/699083 WRAPPED_INSTANTIATE_TEST_CASE_P( DISABLED_DirectoryTreeContextMenu, FileManagerBrowserTest, @@ -254,8 +258,9 @@ #else #define MAYBE_DriveSpecific DriveSpecific #endif +// Flaky: crbug.com/698834 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_DriveSpecific, + DISABLED_DriveSpecific, FileManagerBrowserTest, ::testing::Values( TestParameter(NOT_IN_GUEST_MODE, "openSidebarRecent"), @@ -340,8 +345,9 @@ #else #define MAYBE_SuggestAppDialog SuggestAppDialog #endif +// Flaky: crbug.com/701923 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_SuggestAppDialog, + DISABLE_SuggestAppDialog, FileManagerBrowserTest, ::testing::Values(TestParameter(NOT_IN_GUEST_MODE, "suggestAppDialog"))); @@ -351,8 +357,9 @@ #else #define MAYBE_ExecuteDefaultTaskOnDownloads ExecuteDefaultTaskOnDownloads #endif +// Flaky: crbug.com/699171 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_ExecuteDefaultTaskOnDownloads, + DISABLED_ExecuteDefaultTaskOnDownloads, FileManagerBrowserTest, ::testing::Values( TestParameter(NOT_IN_GUEST_MODE, "executeDefaultTaskOnDownloads"), @@ -444,8 +451,9 @@ #else #define MAYBE_TabindexFocusDownloads TabindexFocusDownloads #endif +// Flaky: crbug.com/699534 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_TabindexFocusDownloads, + DISABLED_TabindexFocusDownloads, FileManagerBrowserTestWithLegacyEventDispatch, ::testing::Values(TestParameter(NOT_IN_GUEST_MODE, "tabindexFocusDownloads"), @@ -536,8 +544,9 @@ #else #define MAYBE_ShowGridView ShowGridView #endif +// Flaky: crbug.com/698772 WRAPPED_INSTANTIATE_TEST_CASE_P( - MAYBE_ShowGridView, + DISABLED_ShowGridView, FileManagerBrowserTest, ::testing::Values(TestParameter(NOT_IN_GUEST_MODE, "showGridViewDownloads"), TestParameter(IN_GUEST_MODE, "showGridViewDownloads"),
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.cc b/chrome/browser/predictors/resource_prefetch_predictor.cc index dd33e10..920674e2 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor.cc
@@ -52,6 +52,8 @@ const size_t kNumSampleHosts = 50; const size_t kReportReadinessThreshold = 50; +bool g_allow_port_in_urls = false; + // For reporting events of interest that are not tied to any navigation. enum ReportingEvent { REPORTING_EVENT_ALL_HISTORY_CLEARED = 0, @@ -273,16 +275,21 @@ // static bool ResourcePrefetchPredictor::IsHandledMainPage(net::URLRequest* request) { - return request->url().SchemeIsHTTPOrHTTPS(); + const GURL& url = request->url(); + bool bad_port = !g_allow_port_in_urls && url.has_port(); + return url.SchemeIsHTTPOrHTTPS() && !bad_port; } // static bool ResourcePrefetchPredictor::IsHandledSubresource( net::URLRequest* response, content::ResourceType resource_type) { + const GURL& url = response->url(); + bool bad_port = !g_allow_port_in_urls && url.has_port(); if (!response->first_party_for_cookies().SchemeIsHTTPOrHTTPS() || - !response->url().SchemeIsHTTPOrHTTPS()) + !url.SchemeIsHTTPOrHTTPS() || bad_port) { return false; + } std::string mime_type; response->GetMimeType(&mime_type); @@ -398,6 +405,11 @@ return true; } +// static +void ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(bool state) { + g_allow_port_in_urls = state; +} + //////////////////////////////////////////////////////////////////////////////// // ResourcePrefetchPredictor nested types.
diff --git a/chrome/browser/predictors/resource_prefetch_predictor.h b/chrome/browser/predictors/resource_prefetch_predictor.h index fa2cfd1..f38ecf81 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor.h +++ b/chrome/browser/predictors/resource_prefetch_predictor.h
@@ -294,6 +294,8 @@ const RedirectDataMap& redirect_data_map, std::string* redirect_endpoint); + static void SetAllowPortInUrlsForTesting(bool state); + // KeyedService methods override. void Shutdown() override;
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc index b5ed25f1..0c1437b9 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
@@ -331,10 +331,16 @@ predictor_ = ResourcePrefetchPredictorFactory::GetForProfile(browser()->profile()); ASSERT_TRUE(predictor_); + // URLs from the test server contain a port number. + ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(true); EnsurePredictorInitialized(); histogram_tester_.reset(new base::HistogramTester()); } + void TearDownOnMainThread() override { + ResourcePrefetchPredictor::SetAllowPortInUrlsForTesting(false); + } + void TestLearningAndPrefetching(const GURL& main_frame_url) { // Navigate to |main_frame_url| and check all the expectations. NavigateToURLAndCheckSubresources(main_frame_url);
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc index 1e4e487..a2b1c53a 100644 --- a/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc +++ b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
@@ -1250,6 +1250,12 @@ content::RESOURCE_TYPE_IMAGE, true); EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( file_request.get(), content::RESOURCE_TYPE_MAIN_FRAME)); + + std::unique_ptr<net::URLRequest> https_request_with_port = + CreateURLRequest(GURL("https://www.google.com:666"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( + https_request_with_port.get(), content::RESOURCE_TYPE_MAIN_FRAME)); } TEST_F(ResourcePrefetchPredictorTest, ShouldRecordRequestSubResource) { @@ -1270,6 +1276,12 @@ content::RESOURCE_TYPE_IMAGE, false); EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( file_request.get(), content::RESOURCE_TYPE_IMAGE)); + + std::unique_ptr<net::URLRequest> https_request_with_port = + CreateURLRequest(GURL("https://www.google.com:666/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, false); + EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordRequest( + https_request_with_port.get(), content::RESOURCE_TYPE_IMAGE)); } TEST_F(ResourcePrefetchPredictorTest, ShouldRecordResponseMainFrame) { @@ -1294,6 +1306,12 @@ content::RESOURCE_TYPE_MAIN_FRAME, true); EXPECT_FALSE( ResourcePrefetchPredictor::ShouldRecordResponse(file_request.get())); + + std::unique_ptr<net::URLRequest> https_request_with_port = + CreateURLRequest(GURL("https://www.google.com:666"), net::MEDIUM, + content::RESOURCE_TYPE_MAIN_FRAME, true); + EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( + https_request_with_port.get())); } TEST_F(ResourcePrefetchPredictorTest, ShouldRecordResponseSubresource) { @@ -1316,6 +1334,12 @@ EXPECT_TRUE(ResourcePrefetchPredictor::ShouldRecordResponse( https_image_request.get())); + std::unique_ptr<net::URLRequest> https_image_request_with_port = + CreateURLRequest(GURL("https://www.google.com:666/cat.png"), net::MEDIUM, + content::RESOURCE_TYPE_IMAGE, true); + EXPECT_FALSE(ResourcePrefetchPredictor::ShouldRecordResponse( + https_image_request_with_port.get())); + std::unique_ptr<net::URLRequest> file_image_request = CreateURLRequest(GURL("file://www.google.com/cat.png"), net::MEDIUM, content::RESOURCE_TYPE_IMAGE, true);
diff --git a/components/autofill/core/browser/autofill_download_manager.cc b/components/autofill/core/browser/autofill_download_manager.cc index 07084c5..854c5ff 100644 --- a/components/autofill/core/browser/autofill_download_manager.cc +++ b/components/autofill/core/browser/autofill_download_manager.cc
@@ -24,9 +24,82 @@ #include "net/http/http_request_headers.h" #include "net/http/http_response_headers.h" #include "net/http/http_status_code.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_fetcher.h" #include "url/gurl.h" +namespace { + +net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotation( + const autofill::AutofillDownloadManager::RequestType& request_type) { + if (request_type == autofill::AutofillDownloadManager::REQUEST_QUERY) { + return net::DefineNetworkTrafficAnnotation("autofill_query", R"( + semantics { + sender: "Autofill" + description: + "Chromium can automatically fill in web forms. If the feature is " + "enabled, Chromium will send a non-identifying description of the " + "form to Google's servers, which will respond with the type of " + "data required by each of the form's fields, if known. I.e., if a " + "field expects to receive a name, phone number, street address, " + "etc." + trigger: "User encounters a web form." + data: + "Hashed descriptions of the form and its fields. User data is not " + "sent." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "You can enable or disable this feature via 'Enable autofill to " + "fill out web forms in a single click.' in Chromium's settings " + "under 'Passwords and forms'. The feature is enabled by default." + chrome_policy { + AutofillEnabled { + policy_options {mode: MANDATORY} + AutofillEnabled: false + } + } + })"); + } + + DCHECK_EQ(request_type, autofill::AutofillDownloadManager::REQUEST_UPLOAD); + return net::DefineNetworkTrafficAnnotation("autofill_upload", R"( + semantics { + sender: "Autofill" + description: + "Chromium relies on crowd-sourced field type classifications to " + "help it automatically fill in web forms. If the feature is " + "enabled, Chromium will send a non-identifying description of the " + "form to Google's servers along with the type of data Chromium " + "observed being given to the form. I.e., if you entered your first " + "name into a form field, Chromium will 'vote' for that form field " + "being a first name field." + trigger: "User submits a web form." + data: + "Hashed descriptions of the form and its fields along with type of " + "data given to each field, if recognized from the user's " + "profile(s). User data is not sent." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "You can enable or disable this feature via 'Enable autofill to " + "fill out web forms in a single click.' in Chromium's settings " + "under 'Passwords and forms'. The feature is enabled by default." + chrome_policy { + AutofillEnabled { + policy_options {mode: MANDATORY} + AutofillEnabled: false + } + } + })"); +} + +} // namespace + namespace autofill { namespace { @@ -234,7 +307,8 @@ // Id is ignored for regular chrome, in unit test id's for fake fetcher // factory will be 0, 1, 2, ... std::unique_ptr<net::URLFetcher> owned_fetcher = net::URLFetcher::Create( - fetcher_id_for_unittest_++, request_url, net::URLFetcher::POST, this); + fetcher_id_for_unittest_++, request_url, net::URLFetcher::POST, this, + GetNetworkTrafficAnnotation(request_data.request_type)); net::URLFetcher* fetcher = owned_fetcher.get(); data_use_measurement::DataUseUserData::AttachToFetcher( fetcher, data_use_measurement::DataUseUserData::AUTOFILL);
diff --git a/components/cronet/android/BUILD.gn b/components/cronet/android/BUILD.gn index 94aa5cf..1418ce3 100644 --- a/components/cronet/android/BUILD.gn +++ b/components/cronet/android/BUILD.gn
@@ -705,7 +705,6 @@ "test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java", "test/javatests/src/org/chromium/net/CronetUrlRequestTest.java", "test/javatests/src/org/chromium/net/DiskStorageTest.java", - "test/javatests/src/org/chromium/net/ExperimentalOptionsTest.java", "test/javatests/src/org/chromium/net/GetStatusTest.java", "test/javatests/src/org/chromium/net/MetricsTestUtil.java", "test/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java",
diff --git a/components/cronet/android/cronet_url_request_context_adapter.cc b/components/cronet/android/cronet_url_request_context_adapter.cc index f195c04..dda9ce4 100644 --- a/components/cronet/android/cronet_url_request_context_adapter.cc +++ b/components/cronet/android/cronet_url_request_context_adapter.cc
@@ -58,6 +58,7 @@ #include "net/http/http_server_properties_manager.h" #include "net/log/file_net_log_observer.h" #include "net/log/net_log_util.h" +#include "net/log/write_to_file_net_log_observer.h" #include "net/nqe/external_estimate_provider.h" #include "net/nqe/network_qualities_prefs_manager.h" #include "net/proxy/proxy_config_service_android.h" @@ -505,7 +506,7 @@ } // Stop NetLog observer if there is one. - StopNetLogOnNetworkThread(); + StopNetLogHelper(); } void CronetURLRequestContextAdapter::InitRequestContextOnMainThread( @@ -609,7 +610,6 @@ DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); DCHECK(!is_context_initialized_); DCHECK(proxy_config_service_); - // TODO(mmenke): Add method to have the builder enable SPDY. net::URLRequestContextBuilder context_builder; @@ -648,9 +648,6 @@ g_net_log.Get().net_log(), GetFileThread()->task_runner()); - effective_experimental_options_ = - std::move(config->effective_experimental_options); - // Set up pref file if storage path is specified. if (!config->storage_path.empty()) { base::FilePath storage_path(config->storage_path); @@ -882,17 +879,28 @@ const JavaParamRef<jobject>& jcaller, const JavaParamRef<jstring>& jfile_name, jboolean jlog_all) { - base::FilePath file_path( - base::android::ConvertJavaStringToUTF8(env, jfile_name)); + base::AutoLock lock(write_to_file_observer_lock_); + // Do nothing if already logging to a file. + if (write_to_file_observer_) + return true; + std::string file_name = + base::android::ConvertJavaStringToUTF8(env, jfile_name); + base::FilePath file_path(file_name); base::ScopedFILE file(base::OpenFile(file_path, "w")); if (!file) { LOG(ERROR) << "Failed to open NetLog file for writing."; return false; } - PostTaskToNetworkThread( - FROM_HERE, - base::Bind(&CronetURLRequestContextAdapter::StartNetLogOnNetworkThread, - base::Unretained(this), file_path, jlog_all == JNI_TRUE)); + + write_to_file_observer_.reset(new net::WriteToFileNetLogObserver()); + if (jlog_all == JNI_TRUE) { + write_to_file_observer_->set_capture_mode( + net::NetLogCaptureMode::IncludeSocketBytes()); + } + write_to_file_observer_->StartObserving( + g_net_log.Get().net_log(), std::move(file), + /*constants=*/nullptr, /*url_request_context=*/nullptr); + return true; } @@ -914,11 +922,7 @@ void CronetURLRequestContextAdapter::StopNetLog( JNIEnv* env, const JavaParamRef<jobject>& jcaller) { - DCHECK(!GetNetworkTaskRunner()->BelongsToCurrentThread()); - PostTaskToNetworkThread( - FROM_HERE, - base::Bind(&CronetURLRequestContextAdapter::StopNetLogOnNetworkThread, - base::Unretained(this))); + StopNetLogHelper(); } void CronetURLRequestContextAdapter::GetCertVerifierData( @@ -1005,25 +1009,6 @@ (timestamp - base::TimeTicks::UnixEpoch()).InMilliseconds(), source); } -void CronetURLRequestContextAdapter::StartNetLogOnNetworkThread( - const base::FilePath& file_path, - bool include_socket_bytes) { - DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); - - // Do nothing if already logging to a file. - if (net_log_file_observer_) - return; - net_log_file_observer_ = net::FileNetLogObserver::CreateUnbounded( - GetFileThread()->task_runner(), file_path, /*constants=*/nullptr); - CreateNetLogEntriesForActiveObjects({context_.get()}, - net_log_file_observer_.get()); - net::NetLogCaptureMode capture_mode = - include_socket_bytes ? net::NetLogCaptureMode::IncludeSocketBytes() - : net::NetLogCaptureMode::Default(); - net_log_file_observer_->StartObserving(g_net_log.Get().net_log(), - capture_mode); -} - void CronetURLRequestContextAdapter::StartNetLogToBoundedFileOnNetworkThread( const std::string& dir_path, bool include_socket_bytes, @@ -1031,37 +1016,34 @@ DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); // Do nothing if already logging to a directory. - if (net_log_file_observer_) + if (bounded_file_observer_) return; // Filepath for NetLog files must exist and be writable. base::FilePath file_path(dir_path); DCHECK(base::PathIsWritable(file_path)); - net_log_file_observer_ = net::FileNetLogObserver::CreateBounded( + bounded_file_observer_ = net::FileNetLogObserver::CreateBounded( GetFileThread()->task_runner(), file_path, size, kNumNetLogEventFiles, /*constants=*/nullptr); CreateNetLogEntriesForActiveObjects({context_.get()}, - net_log_file_observer_.get()); + bounded_file_observer_.get()); net::NetLogCaptureMode capture_mode = include_socket_bytes ? net::NetLogCaptureMode::IncludeSocketBytes() : net::NetLogCaptureMode::Default(); - net_log_file_observer_->StartObserving(g_net_log.Get().net_log(), + bounded_file_observer_->StartObserving(g_net_log.Get().net_log(), capture_mode); } -void CronetURLRequestContextAdapter::StopNetLogOnNetworkThread() { +void CronetURLRequestContextAdapter::StopBoundedFileNetLogOnNetworkThread() { DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread()); - - if (!net_log_file_observer_) - return; - net_log_file_observer_->StopObserving( - GetNetLogInfo(), + bounded_file_observer_->StopObserving( + net::GetNetInfo(context_.get(), net::NET_INFO_ALL_SOURCES), base::Bind(&CronetURLRequestContextAdapter::StopNetLogCompleted, base::Unretained(this))); - net_log_file_observer_.reset(); + bounded_file_observer_.reset(); } void CronetURLRequestContextAdapter::StopNetLogCompleted() { @@ -1069,15 +1051,18 @@ base::android::AttachCurrentThread(), jcronet_url_request_context_.obj()); } -std::unique_ptr<base::DictionaryValue> -CronetURLRequestContextAdapter::GetNetLogInfo() const { - std::unique_ptr<base::DictionaryValue> net_info = - net::GetNetInfo(context_.get(), net::NET_INFO_ALL_SOURCES); - if (effective_experimental_options_) { - net_info->Set("cronetExperimentalParams", - effective_experimental_options_->CreateDeepCopy()); +void CronetURLRequestContextAdapter::StopNetLogHelper() { + base::AutoLock lock(write_to_file_observer_lock_); + DCHECK(!(write_to_file_observer_ && bounded_file_observer_)); + if (write_to_file_observer_) { + write_to_file_observer_->StopObserving(/*url_request_context=*/nullptr); + write_to_file_observer_.reset(); + } else if (bounded_file_observer_) { + PostTaskToNetworkThread(FROM_HERE, + base::Bind(&CronetURLRequestContextAdapter:: + StopBoundedFileNetLogOnNetworkThread, + base::Unretained(this))); } - return net_info; } // Create a URLRequestContextConfig from the given parameters.
diff --git a/components/cronet/android/cronet_url_request_context_adapter.h b/components/cronet/android/cronet_url_request_context_adapter.h index 664c8f3..400c640 100644 --- a/components/cronet/android/cronet_url_request_context_adapter.h +++ b/components/cronet/android/cronet_url_request_context_adapter.h
@@ -36,6 +36,7 @@ class ProxyConfigService; class SdchOwner; class URLRequestContext; +class WriteToFileNetLogObserver; class FileNetLogObserver; } // namespace net @@ -81,10 +82,8 @@ net::URLRequestContext* GetURLRequestContext(); - // TODO(xunjieli): Keep only one version of StartNetLog(). - - // Starts NetLog logging to file. This can be called on any thread. - // Return false if |jfile_name| cannot be opened. + // Starts NetLog logging to file. This can be called on any thread. Returns + // false if it fails to open log file. bool StartNetLogToFile(JNIEnv* env, const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jstring>& jfile_name, @@ -200,18 +199,17 @@ bool include_socket_bytes, int size); - // Same as StartNetLogToFile, but called only on the network thread. - void StartNetLogOnNetworkThread(const base::FilePath& file_path, - bool include_socket_bytes); - - // Stops NetLog logging on the network thread. - void StopNetLogOnNetworkThread(); + // Stops NetLog logging to file by calling StopObserving() and destroying + // the |bounded_file_observer_|. + void StopBoundedFileNetLogOnNetworkThread(); // Callback for StopObserving() that unblocks the Java ConditionVariable and // signals that it is safe to access the NetLog files. void StopNetLogCompleted(); - std::unique_ptr<base::DictionaryValue> GetNetLogInfo() const; + // Helper method to stop NetLog logging to file. This can be called on any + // thread. This will flush any remaining writes to disk. + void StopNetLogHelper(); // Network thread is owned by |this|, but is destroyed from java thread. base::Thread* network_thread_; @@ -219,7 +217,12 @@ // File thread should be destroyed last. std::unique_ptr<base::Thread> file_thread_; - std::unique_ptr<net::FileNetLogObserver> net_log_file_observer_; + // |write_to_file_observer_| should only be accessed with + // |write_to_file_observer_lock_|. + std::unique_ptr<net::WriteToFileNetLogObserver> write_to_file_observer_; + base::Lock write_to_file_observer_lock_; + + std::unique_ptr<net::FileNetLogObserver> bounded_file_observer_; // |pref_service_| should outlive the HttpServerPropertiesManager owned by // |context_|. @@ -237,9 +240,6 @@ // Context config is only valid until context is initialized. std::unique_ptr<URLRequestContextConfig> context_config_; - // Effective experimental options. Kept for NetLog. - std::unique_ptr<base::DictionaryValue> effective_experimental_options_; - // A queue of tasks that need to be run after context has been initialized. std::queue<base::Closure> tasks_waiting_for_context_; bool is_context_initialized_;
diff --git a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java index 629b8d9e..16824f9 100644 --- a/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java
@@ -148,6 +148,13 @@ @GuardedBy("mLock") private boolean mIsLogging; + /** + * True if a NetLog observer that writes to disk with a bounded amount of space has been + * activated by calling StartNetLogToDisk(). + */ + @GuardedBy("mLock") + private boolean mNetLogToDisk; + @UsedByReflection("CronetEngine.java") public CronetUrlRequestContext(final CronetEngineBuilderImpl builder) { CronetLibraryLoader.ensureInitialized(builder.getContext(), builder); @@ -287,6 +294,7 @@ checkHaveAdapter(); nativeStartNetLogToDisk(mUrlRequestContextAdapter, dirPath, logAll, maxSize); mIsLogging = true; + mNetLogToDisk = true; } } @@ -299,6 +307,9 @@ checkHaveAdapter(); nativeStopNetLog(mUrlRequestContextAdapter); mIsLogging = false; + if (!mNetLogToDisk) { + return; + } mStopNetLogCompleted = new ConditionVariable(); } mStopNetLogCompleted.block(); @@ -306,6 +317,9 @@ @CalledByNative public void stopNetLogCompleted() { + synchronized (mLock) { + mNetLogToDisk = false; + } mStopNetLogCompleted.open(); }
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/ExperimentalOptionsTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/ExperimentalOptionsTest.java index 5347964..165f83c2 100644 --- a/components/cronet/android/test/javatests/src/org/chromium/net/ExperimentalOptionsTest.java +++ b/components/cronet/android/test/javatests/src/org/chromium/net/ExperimentalOptionsTest.java
@@ -46,50 +46,6 @@ @SmallTest @Feature({"Cronet"}) @OnlyRunNativeCronet - // Tests that NetLog writes effective experimental options to NetLog. - public void testNetLog() throws Exception { - File directory = new File(PathUtils.getDataDirectory()); - File logfile = File.createTempFile("cronet", "json", directory); - JSONObject hostResolverParams = CronetTestUtil.generateHostResolverRules(); - JSONObject experimentalOptions = - new JSONObject().put("HostResolverRules", hostResolverParams); - mBuilder.setExperimentalOptions(experimentalOptions.toString()); - - mTestFramework = new CronetTestFramework(null, null, getContext(), mBuilder); - mTestFramework.mCronetEngine.startNetLogToFile(logfile.getPath(), false); - String url = Http2TestServer.getEchoMethodUrl(); - TestUrlRequestCallback callback = new TestUrlRequestCallback(); - UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder( - url, callback, callback.getExecutor()); - UrlRequest urlRequest = builder.build(); - urlRequest.start(); - callback.blockForDone(); - assertEquals(200, callback.mResponseInfo.getHttpStatusCode()); - assertEquals("GET", callback.mResponseAsString); - mTestFramework.mCronetEngine.stopNetLog(); - assertTrue(logfile.exists()); - assertTrue(logfile.length() != 0); - BufferedReader logReader = new BufferedReader(new FileReader(logfile)); - boolean validFile = false; - try { - String logLine; - while ((logLine = logReader.readLine()) != null) { - if (logLine.contains("HostResolverRules")) { - validFile = true; - break; - } - } - } finally { - logReader.close(); - } - assertTrue(validFile); - assertTrue(logfile.delete()); - assertTrue(!logfile.exists()); - } - - @SmallTest - @Feature({"Cronet"}) - @OnlyRunNativeCronet public void testSetSSLKeyLogFile() throws Exception { String url = Http2TestServer.getEchoMethodUrl(); File dir = new File(PathUtils.getDataDirectory());
diff --git a/components/cronet/url_request_context_config.cc b/components/cronet/url_request_context_config.cc index 196b969..00979d3 100644 --- a/components/cronet/url_request_context_config.cc +++ b/components/cronet/url_request_context_config.cc
@@ -89,14 +89,13 @@ const char kSSLKeyLogFile[] = "ssl_key_log_file"; -// Returns the effective experimental options. -std::unique_ptr<base::DictionaryValue> ParseAndSetExperimentalOptions( +void ParseAndSetExperimentalOptions( const std::string& experimental_options, net::URLRequestContextBuilder* context_builder, net::NetLog* net_log, const scoped_refptr<base::SequencedTaskRunner>& file_task_runner) { if (experimental_options.empty()) - return nullptr; + return; DCHECK(net_log); @@ -107,7 +106,7 @@ if (!options) { DCHECK(false) << "Parsing experimental options failed: " << experimental_options; - return nullptr; + return; } std::unique_ptr<base::DictionaryValue> dict = @@ -116,7 +115,101 @@ if (!dict) { DCHECK(false) << "Experimental options string is not a dictionary: " << experimental_options; - return nullptr; + return; + } + + const base::DictionaryValue* quic_args = nullptr; + if (dict->GetDictionary(kQuicFieldTrialName, &quic_args)) { + std::string quic_connection_options; + if (quic_args->GetString(kQuicConnectionOptions, + &quic_connection_options)) { + context_builder->set_quic_connection_options( + net::ParseQuicConnectionOptions(quic_connection_options)); + } + + // TODO(rtenneti): Delete this option after apps stop using it. + // Added this for backward compatibility. + bool quic_store_server_configs_in_properties = false; + if (quic_args->GetBoolean(kQuicStoreServerConfigsInProperties, + &quic_store_server_configs_in_properties)) { + context_builder->set_quic_max_server_configs_stored_in_properties( + net::kMaxQuicServersToPersist); + } + + int quic_max_server_configs_stored_in_properties = 0; + if (quic_args->GetInteger(kQuicMaxServerConfigsStoredInProperties, + &quic_max_server_configs_stored_in_properties)) { + context_builder->set_quic_max_server_configs_stored_in_properties( + static_cast<size_t>(quic_max_server_configs_stored_in_properties)); + } + + bool quic_delay_tcp_race = false; + if (quic_args->GetBoolean(kQuicDelayTcpRace, &quic_delay_tcp_race)) { + context_builder->set_quic_delay_tcp_race(quic_delay_tcp_race); + } + + int quic_idle_connection_timeout_seconds = 0; + if (quic_args->GetInteger(kQuicIdleConnectionTimeoutSeconds, + &quic_idle_connection_timeout_seconds)) { + context_builder->set_quic_idle_connection_timeout_seconds( + quic_idle_connection_timeout_seconds); + } + + std::string quic_host_whitelist; + if (quic_args->GetString(kQuicHostWhitelist, &quic_host_whitelist)) { + std::unordered_set<std::string> hosts; + for (const std::string& host : + base::SplitString(quic_host_whitelist, ",", base::TRIM_WHITESPACE, + base::SPLIT_WANT_ALL)) { + hosts.insert(host); + } + context_builder->set_quic_host_whitelist(hosts); + } + + bool quic_close_sessions_on_ip_change = false; + if (quic_args->GetBoolean(kQuicCloseSessionsOnIpChange, + &quic_close_sessions_on_ip_change)) { + context_builder->set_quic_close_sessions_on_ip_change( + quic_close_sessions_on_ip_change); + } + + bool quic_migrate_sessions_on_network_change = false; + if (quic_args->GetBoolean(kQuicMigrateSessionsOnNetworkChange, + &quic_migrate_sessions_on_network_change)) { + context_builder->set_quic_migrate_sessions_on_network_change( + quic_migrate_sessions_on_network_change); + } + + bool quic_prefer_aes = false; + if (quic_args->GetBoolean(kQuicPreferAes, &quic_prefer_aes)) { + context_builder->set_quic_prefer_aes(quic_prefer_aes); + } + + std::string quic_user_agent_id; + if (quic_args->GetString(kQuicUserAgentId, &quic_user_agent_id)) { + context_builder->set_quic_user_agent_id(quic_user_agent_id); + } + + bool quic_migrate_sessions_early = false; + if (quic_args->GetBoolean(kQuicMigrateSessionsEarly, + &quic_migrate_sessions_early)) { + context_builder->set_quic_migrate_sessions_early( + quic_migrate_sessions_early); + } + + bool quic_disable_bidirectional_streams = false; + if (quic_args->GetBoolean(kQuicDisableBidirectionalStreams, + &quic_disable_bidirectional_streams)) { + context_builder->set_quic_disable_bidirectional_streams( + quic_disable_bidirectional_streams); + } + + bool quic_race_cert_verification = false; + if (quic_args->GetBoolean(kQuicRaceCertVerification, + &quic_race_cert_verification)) { + context_builder->set_quic_race_cert_verification( + quic_race_cert_verification); + } } bool async_dns_enable = false; @@ -125,185 +218,49 @@ bool disable_ipv6 = false; StaleHostResolver::StaleOptions stale_dns_options; std::string host_resolver_rules_string; - for (base::DictionaryValue::Iterator it(*dict.get()); !it.IsAtEnd(); - it.Advance()) { - if (it.key() == kQuicFieldTrialName) { - const base::DictionaryValue* quic_args = nullptr; - if (!it.value().GetAsDictionary(&quic_args)) { - LOG(ERROR) << "Quic config params \"" << it.value() - << "\" is not a dictionary value"; - dict->Remove(it.key(), nullptr); - continue; - } - std::string quic_connection_options; - if (quic_args->GetString(kQuicConnectionOptions, - &quic_connection_options)) { - context_builder->set_quic_connection_options( - net::ParseQuicConnectionOptions(quic_connection_options)); - } - // TODO(rtenneti): Delete this option after apps stop using it. - // Added this for backward compatibility. - bool quic_store_server_configs_in_properties = false; - if (quic_args->GetBoolean(kQuicStoreServerConfigsInProperties, - &quic_store_server_configs_in_properties)) { - context_builder->set_quic_max_server_configs_stored_in_properties( - net::kMaxQuicServersToPersist); - } + const base::DictionaryValue* async_dns_args = nullptr; + if (dict->GetDictionary(kAsyncDnsFieldTrialName, &async_dns_args)) + async_dns_args->GetBoolean(kAsyncDnsEnable, &async_dns_enable); - int quic_max_server_configs_stored_in_properties = 0; - if (quic_args->GetInteger( - kQuicMaxServerConfigsStoredInProperties, - &quic_max_server_configs_stored_in_properties)) { - context_builder->set_quic_max_server_configs_stored_in_properties( - static_cast<size_t>(quic_max_server_configs_stored_in_properties)); + const base::DictionaryValue* stale_dns_args = nullptr; + if (dict->GetDictionary(kStaleDnsFieldTrialName, &stale_dns_args)) { + if (stale_dns_args->GetBoolean(kStaleDnsEnable, &stale_dns_enable) && + stale_dns_enable) { + int delay; + if (stale_dns_args->GetInteger(kStaleDnsDelayMs, &delay)) + stale_dns_options.delay = base::TimeDelta::FromMilliseconds(delay); + int max_expired_time_ms; + if (stale_dns_args->GetInteger(kStaleDnsMaxExpiredTimeMs, + &max_expired_time_ms)) { + stale_dns_options.max_expired_time = + base::TimeDelta::FromMilliseconds(max_expired_time_ms); } - - bool quic_delay_tcp_race = false; - if (quic_args->GetBoolean(kQuicDelayTcpRace, &quic_delay_tcp_race)) { - context_builder->set_quic_delay_tcp_race(quic_delay_tcp_race); + int max_stale_uses; + if (stale_dns_args->GetInteger(kStaleDnsMaxStaleUses, &max_stale_uses)) + stale_dns_options.max_stale_uses = max_stale_uses; + bool allow_other_network; + if (stale_dns_args->GetBoolean(kStaleDnsAllowOtherNetwork, + &allow_other_network)) { + stale_dns_options.allow_other_network = allow_other_network; } - - int quic_idle_connection_timeout_seconds = 0; - if (quic_args->GetInteger(kQuicIdleConnectionTimeoutSeconds, - &quic_idle_connection_timeout_seconds)) { - context_builder->set_quic_idle_connection_timeout_seconds( - quic_idle_connection_timeout_seconds); - } - - std::string quic_host_whitelist; - if (quic_args->GetString(kQuicHostWhitelist, &quic_host_whitelist)) { - std::unordered_set<std::string> hosts; - for (const std::string& host : - base::SplitString(quic_host_whitelist, ",", base::TRIM_WHITESPACE, - base::SPLIT_WANT_ALL)) { - hosts.insert(host); - } - context_builder->set_quic_host_whitelist(hosts); - } - - bool quic_close_sessions_on_ip_change = false; - if (quic_args->GetBoolean(kQuicCloseSessionsOnIpChange, - &quic_close_sessions_on_ip_change)) { - context_builder->set_quic_close_sessions_on_ip_change( - quic_close_sessions_on_ip_change); - } - - bool quic_migrate_sessions_on_network_change = false; - if (quic_args->GetBoolean(kQuicMigrateSessionsOnNetworkChange, - &quic_migrate_sessions_on_network_change)) { - context_builder->set_quic_migrate_sessions_on_network_change( - quic_migrate_sessions_on_network_change); - } - - bool quic_prefer_aes = false; - if (quic_args->GetBoolean(kQuicPreferAes, &quic_prefer_aes)) { - context_builder->set_quic_prefer_aes(quic_prefer_aes); - } - - std::string quic_user_agent_id; - if (quic_args->GetString(kQuicUserAgentId, &quic_user_agent_id)) { - context_builder->set_quic_user_agent_id(quic_user_agent_id); - } - - bool quic_migrate_sessions_early = false; - if (quic_args->GetBoolean(kQuicMigrateSessionsEarly, - &quic_migrate_sessions_early)) { - context_builder->set_quic_migrate_sessions_early( - quic_migrate_sessions_early); - } - - bool quic_disable_bidirectional_streams = false; - if (quic_args->GetBoolean(kQuicDisableBidirectionalStreams, - &quic_disable_bidirectional_streams)) { - context_builder->set_quic_disable_bidirectional_streams( - quic_disable_bidirectional_streams); - } - - bool quic_race_cert_verification = false; - if (quic_args->GetBoolean(kQuicRaceCertVerification, - &quic_race_cert_verification)) { - context_builder->set_quic_race_cert_verification( - quic_race_cert_verification); - } - } else if (it.key() == kAsyncDnsFieldTrialName) { - const base::DictionaryValue* async_dns_args = nullptr; - if (!it.value().GetAsDictionary(&async_dns_args)) { - LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value() - << "\" is not a dictionary value"; - dict->Remove(it.key(), nullptr); - continue; - } - async_dns_args->GetBoolean(kAsyncDnsEnable, &async_dns_enable); - } else if (it.key() == kStaleDnsFieldTrialName) { - const base::DictionaryValue* stale_dns_args = nullptr; - if (!it.value().GetAsDictionary(&stale_dns_args)) { - LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value() - << "\" is not a dictionary value"; - dict->Remove(it.key(), nullptr); - continue; - } - if (stale_dns_args->GetBoolean(kStaleDnsEnable, &stale_dns_enable) && - stale_dns_enable) { - int delay; - if (stale_dns_args->GetInteger(kStaleDnsDelayMs, &delay)) - stale_dns_options.delay = base::TimeDelta::FromMilliseconds(delay); - int max_expired_time_ms; - if (stale_dns_args->GetInteger(kStaleDnsMaxExpiredTimeMs, - &max_expired_time_ms)) { - stale_dns_options.max_expired_time = - base::TimeDelta::FromMilliseconds(max_expired_time_ms); - } - int max_stale_uses; - if (stale_dns_args->GetInteger(kStaleDnsMaxStaleUses, &max_stale_uses)) - stale_dns_options.max_stale_uses = max_stale_uses; - bool allow_other_network; - if (stale_dns_args->GetBoolean(kStaleDnsAllowOtherNetwork, - &allow_other_network)) { - stale_dns_options.allow_other_network = allow_other_network; - } - } - } else if (it.key() == kHostResolverRulesFieldTrialName) { - const base::DictionaryValue* host_resolver_rules_args = nullptr; - if (!it.value().GetAsDictionary(&host_resolver_rules_args)) { - LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value() - << "\" is not a dictionary value"; - dict->Remove(it.key(), nullptr); - continue; - } - host_resolver_rules_enable = host_resolver_rules_args->GetString( - kHostResolverRules, &host_resolver_rules_string); - } else if (it.key() == kDisableIPv6) { - if (!it.value().GetAsBoolean(&disable_ipv6)) { - LOG(ERROR) << "\"" << it.key() << "\" config params \"" << it.value() - << "\" is not a bool"; - dict->Remove(it.key(), nullptr); - continue; - } - } else if (it.key() == kSSLKeyLogFile) { - std::string ssl_key_log_file_string; - if (it.value().GetAsString(&ssl_key_log_file_string)) { - DCHECK(file_task_runner); - base::FilePath ssl_key_log_file(ssl_key_log_file_string); - if (!ssl_key_log_file.empty() && file_task_runner) { - // SetSSLKeyLogFile is only safe to call before any SSLClientSockets - // are created. This should not be used if there are multiple - // CronetEngine. - // TODO(xunjieli): Expose this as a stable API after crbug.com/458365 - // is resolved. - net::SSLClientSocket::SetSSLKeyLogFile(ssl_key_log_file, - file_task_runner); - } - } - } else { - LOG(WARNING) << "Unrecognized Cronet experimental option \"" << it.key() - << "\" with params \"" << it.value(); - dict->Remove(it.key(), nullptr); } } + + const base::DictionaryValue* host_resolver_rules_args = nullptr; + if (dict->GetDictionary(kHostResolverRulesFieldTrialName, + &host_resolver_rules_args)) { + host_resolver_rules_enable = host_resolver_rules_args->GetString( + kHostResolverRules, &host_resolver_rules_string); + } + + dict->GetBoolean(kDisableIPv6, &disable_ipv6); + if (async_dns_enable || stale_dns_enable || host_resolver_rules_enable || disable_ipv6) { - CHECK(net_log) << "All DNS-related experiments require NetLog."; + if (net_log == nullptr) { + CHECK(false) << "All DNS-related experiments require NetLog."; + } std::unique_ptr<net::HostResolver> host_resolver; if (stale_dns_enable) { DCHECK(!disable_ipv6); @@ -325,7 +282,20 @@ } context_builder->set_host_resolver(std::move(host_resolver)); } - return dict; + + std::string ssl_key_log_file_string; + if (dict->GetString(kSSLKeyLogFile, &ssl_key_log_file_string)) { + DCHECK(file_task_runner); + base::FilePath ssl_key_log_file(ssl_key_log_file_string); + if (!ssl_key_log_file.empty() && file_task_runner) { + // SetSSLKeyLogFile is only safe to call before any SSLClientSockets are + // created. This should not be used if there are multiple CronetEngine. + // TODO(xunjieli): Expose this as a stable API after crbug.com/458365 is + // resolved. + net::SSLClientSocket::SetSSLKeyLogFile(ssl_key_log_file, + file_task_runner); + } + } } } // namespace @@ -417,8 +387,8 @@ if (enable_quic) context_builder->set_quic_user_agent_id(quic_user_agent_id); - effective_experimental_options = ParseAndSetExperimentalOptions( - experimental_options, context_builder, net_log, file_task_runner); + ParseAndSetExperimentalOptions(experimental_options, context_builder, net_log, + file_task_runner); std::unique_ptr<net::CertVerifier> cert_verifier; if (mock_cert_verifier) {
diff --git a/components/cronet/url_request_context_config.h b/components/cronet/url_request_context_config.h index 69e1b80..3ef5159a 100644 --- a/components/cronet/url_request_context_config.h +++ b/components/cronet/url_request_context_config.h
@@ -16,7 +16,6 @@ #include "net/cert/cert_verifier.h" namespace base { -class DictionaryValue; class SequencedTaskRunner; } // namespace base @@ -118,7 +117,7 @@ const std::string& cert_verifier_data); ~URLRequestContextConfig(); - // Configures |context_builder| based on |this|. + // Configure |context_builder| based on |this|. void ConfigureURLRequestContextBuilder( net::URLRequestContextBuilder* context_builder, net::NetLog* net_log, @@ -174,9 +173,6 @@ // The list of public key pins. ScopedVector<Pkp> pkp_list; - // Experimental options that are recognized by the config parser. - std::unique_ptr<base::DictionaryValue> effective_experimental_options; - private: DISALLOW_COPY_AND_ASSIGN(URLRequestContextConfig); };
diff --git a/components/cronet/url_request_context_config_unittest.cc b/components/cronet/url_request_context_config_unittest.cc index bbff700..30c9feea 100644 --- a/components/cronet/url_request_context_config_unittest.cc +++ b/components/cronet/url_request_context_config_unittest.cc
@@ -19,7 +19,7 @@ namespace cronet { -TEST(URLRequestContextConfigTest, TestExperimentalOptionParsing) { +TEST(URLRequestContextConfigTest, TestExperimentalOptionPassing) { URLRequestContextConfig config( // Enable QUIC. true, @@ -50,7 +50,6 @@ "\"race_cert_verification\":true," "\"connection_options\":\"TIME,TBBR,REJ\"}," "\"AsyncDNS\":{\"enable\":true}," - "\"UnknownOption\":{\"foo\":true}," "\"HostResolverRules\":{\"host_resolver_rules\":" "\"MAP * 127.0.0.1\"}," // See http://crbug.com/696569. @@ -76,7 +75,6 @@ net::URLRequestContextBuilder builder; net::NetLog net_log; config.ConfigureURLRequestContextBuilder(&builder, &net_log, nullptr); - EXPECT_FALSE(config.effective_experimental_options->HasKey("UnknownOption")); // Set a ProxyConfigService to avoid DCHECK failure when building. builder.set_proxy_config_service( base::MakeUnique<net::ProxyConfigServiceFixed>(
diff --git a/components/ntp_tiles/constants.cc b/components/ntp_tiles/constants.cc index 7e3d84f..0c54388e 100644 --- a/components/ntp_tiles/constants.cc +++ b/components/ntp_tiles/constants.cc
@@ -4,8 +4,13 @@ #include "components/ntp_tiles/constants.h" +#include "base/feature_list.h" + namespace ntp_tiles { const char kPopularSitesFieldTrialName[] = "NTPPopularSites"; +extern const base::Feature kPopularSitesBakedInContentFeature{ + "NTPPopularSitesBakedInContent", base::FEATURE_ENABLED_BY_DEFAULT}; + } // namespace ntp_tiles
diff --git a/components/ntp_tiles/constants.h b/components/ntp_tiles/constants.h index 8c50a99..4265108 100644 --- a/components/ntp_tiles/constants.h +++ b/components/ntp_tiles/constants.h
@@ -5,11 +5,20 @@ #ifndef COMPONENTS_NTP_TILES_CONSTANTS_H_ #define COMPONENTS_NTP_TILES_CONSTANTS_H_ +namespace base { +struct Feature; +} // namespace base + namespace ntp_tiles { // Name of the field trial to configure PopularSites. extern const char kPopularSitesFieldTrialName[]; +// This feature is enabled by default. Otherwise, users who need it would not +// get the right configuration timely enough. The configuration affects only +// Android or iOS users. +extern const base::Feature kPopularSitesBakedInContentFeature; + } // namespace ntp_tiles -#endif +#endif // COMPONENTS_NTP_TILES_CONSTANTS_H_
diff --git a/components/ntp_tiles/popular_sites_impl.cc b/components/ntp_tiles/popular_sites_impl.cc index 7777e37d..97867178 100644 --- a/components/ntp_tiles/popular_sites_impl.cc +++ b/components/ntp_tiles/popular_sites_impl.cc
@@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/command_line.h" +#include "base/feature_list.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/path_service.h" @@ -153,6 +154,9 @@ #if !defined(OS_ANDROID) && !defined(OS_IOS) return base::MakeUnique<base::ListValue>(); #else + if (!base::FeatureList::IsEnabled(kPopularSitesBakedInContentFeature)) { + return base::MakeUnique<base::ListValue>(); + } std::unique_ptr<base::ListValue> sites = base::ListValue::From(base::JSONReader::Read( ResourceBundle::GetSharedInstance().GetRawDataResource(
diff --git a/components/ntp_tiles/popular_sites_impl_unittest.cc b/components/ntp_tiles/popular_sites_impl_unittest.cc index d5502037..1deff6bd 100644 --- a/components/ntp_tiles/popular_sites_impl_unittest.cc +++ b/components/ntp_tiles/popular_sites_impl_unittest.cc
@@ -18,9 +18,11 @@ #include "base/memory/ptr_util.h" #include "base/optional.h" #include "base/run_loop.h" +#include "base/test/scoped_feature_list.h" #include "base/test/sequenced_worker_pool_owner.h" #include "base/threading/thread_task_runner_handle.h" #include "base/values.h" +#include "components/ntp_tiles/constants.h" #include "components/ntp_tiles/json_unsafe_parser.h" #include "components/ntp_tiles/pref_names.h" #include "components/ntp_tiles/switches.h" @@ -60,7 +62,7 @@ #if defined(OS_ANDROID) || defined(OS_IOS) return 8ul; #else - return 0; + return 0ul; #endif } @@ -83,18 +85,19 @@ {kFaviconUrl, "https://www.chromium.org/favicon.ico"}, }, worker_pool_owner_(2, "PopularSitesTest."), + prefs_(new sync_preferences::TestingPrefServiceSyncable()), url_fetcher_factory_(nullptr) { base::CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableNTPPopularSites); - PopularSitesImpl::RegisterProfilePrefs(prefs_.registry()); + PopularSitesImpl::RegisterProfilePrefs(prefs_->registry()); CHECK(scoped_cache_dir_.CreateUniqueTempDir()); cache_dir_ = scoped_cache_dir_.GetPath(); } void SetCountryAndVersion(const std::string& country, const std::string& version) { - prefs_.SetString(prefs::kPopularSitesOverrideCountry, country); - prefs_.SetString(prefs::kPopularSitesOverrideVersion, version); + prefs_->SetString(prefs::kPopularSitesOverrideCountry, country); + prefs_->SetString(prefs::kPopularSitesOverrideVersion, version); } void RespondWithJSON(const std::string& url, @@ -123,6 +126,11 @@ net::URLRequestStatus::SUCCESS); } + void ReregisterProfilePrefs() { + prefs_ = base::MakeUnique<sync_preferences::TestingPrefServiceSyncable>(); + PopularSitesImpl::RegisterProfilePrefs(prefs_->registry()); + } + // Returns an optional bool representing whether the completion callback was // called at all, and if yes which was the returned bool value. base::Optional<bool> FetchPopularSites(bool force_download, @@ -152,7 +160,7 @@ std::unique_ptr<PopularSites> CreatePopularSites( net::URLRequestContextGetter* context) { return base::MakeUnique<PopularSitesImpl>( - worker_pool_owner_.pool().get(), &prefs_, + worker_pool_owner_.pool().get(), prefs_.get(), /*template_url_service=*/nullptr, /*variations_service=*/nullptr, context, cache_dir_, base::Bind(JsonUnsafeParser::Parse)); @@ -166,7 +174,7 @@ base::SequencedWorkerPoolOwner worker_pool_owner_; base::ScopedTempDir scoped_cache_dir_; base::FilePath cache_dir_; - sync_preferences::TestingPrefServiceSyncable prefs_; + std::unique_ptr<sync_preferences::TestingPrefServiceSyncable> prefs_; net::FakeURLFetcherFactory url_fetcher_factory_; }; @@ -180,7 +188,20 @@ Eq(GetNumberOfDefaultPopularSitesForPlatform())); } -TEST_F(PopularSitesTest, Zasic) { +TEST_F(PopularSitesTest, IsEmptyOnConstructionIfDisabledByTrial) { + base::test::ScopedFeatureList override_features; + override_features.InitAndDisableFeature(kPopularSitesBakedInContentFeature); + ReregisterProfilePrefs(); + + scoped_refptr<net::TestURLRequestContextGetter> url_request_context( + new net::TestURLRequestContextGetter( + base::ThreadTaskRunnerHandle::Get())); + auto popular_sites = CreatePopularSites(url_request_context.get()); + + EXPECT_THAT(popular_sites->sites().size(), Eq(0ul)); +} + +TEST_F(PopularSitesTest, ShouldSucceedFetching) { SetCountryAndVersion("ZZ", "9"); RespondWithJSON( "https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json",
diff --git a/components/supervised_user_error_page/resources/supervised_user_block_interstitial.css b/components/supervised_user_error_page/resources/supervised_user_block_interstitial.css index 85aa6be..e84b3bc 100644 --- a/components/supervised_user_error_page/resources/supervised_user_block_interstitial.css +++ b/components/supervised_user_error_page/resources/supervised_user_block_interstitial.css
@@ -151,6 +151,7 @@ display: flex; font-size: 12px; line-height: normal; + margin-top: 14px; } .custodian-name {
diff --git a/components/tracing/common/process_metrics_memory_dump_provider.cc b/components/tracing/common/process_metrics_memory_dump_provider.cc index 147e8b33f..c296135 100644 --- a/components/tracing/common/process_metrics_memory_dump_provider.cc +++ b/components/tracing/common/process_metrics_memory_dump_provider.cc
@@ -213,6 +213,10 @@ // static uint64_t ProcessMetricsMemoryDumpProvider::rss_bytes_for_testing = 0; +// static +ProcessMetricsMemoryDumpProvider::FactoryFunction + ProcessMetricsMemoryDumpProvider::factory_for_testing = nullptr; + #if defined(OS_LINUX) || defined(OS_ANDROID) // static @@ -542,23 +546,31 @@ // static void ProcessMetricsMemoryDumpProvider::RegisterForProcess( base::ProcessId process) { - std::unique_ptr<ProcessMetricsMemoryDumpProvider> metrics_provider( - new ProcessMetricsMemoryDumpProvider(process)); - base::trace_event::MemoryDumpProvider::Options options; - options.target_pid = process; - options.is_fast_polling_supported = true; - base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( - metrics_provider.get(), "ProcessMemoryMetrics", nullptr, options); + std::unique_ptr<ProcessMetricsMemoryDumpProvider> owned_provider; + if (factory_for_testing) { + owned_provider = factory_for_testing(process); + } else { + owned_provider = std::unique_ptr<ProcessMetricsMemoryDumpProvider>( + new ProcessMetricsMemoryDumpProvider(process)); + } + + ProcessMetricsMemoryDumpProvider* provider = owned_provider.get(); bool did_insert = g_dump_providers_map.Get() - .insert(std::make_pair(process, std::move(metrics_provider))) + .insert(std::make_pair(process, std::move(owned_provider))) .second; if (!did_insert) { DLOG(ERROR) << "ProcessMetricsMemoryDumpProvider already registered for " << (process == base::kNullProcessId ? "current process" : "process id " + base::IntToString(process)); + return; } + base::trace_event::MemoryDumpProvider::Options options; + options.target_pid = process; + options.is_fast_polling_supported = true; + base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( + provider, "ProcessMemoryMetrics", nullptr, options); } // static
diff --git a/components/tracing/common/process_metrics_memory_dump_provider.h b/components/tracing/common/process_metrics_memory_dump_provider.h index 6534f18..730a9be 100644 --- a/components/tracing/common/process_metrics_memory_dump_provider.h +++ b/components/tracing/common/process_metrics_memory_dump_provider.h
@@ -37,7 +37,13 @@ void PollFastMemoryTotal(uint64_t* memory_total) override; void SuspendFastMemoryPolling() override; + protected: + ProcessMetricsMemoryDumpProvider(base::ProcessId process); + private: + using FactoryFunction = + std::unique_ptr<ProcessMetricsMemoryDumpProvider> (*)(base::ProcessId); + FRIEND_TEST_ALL_PREFIXES(ProcessMetricsMemoryDumpProviderTest, ParseProcSmaps); FRIEND_TEST_ALL_PREFIXES(ProcessMetricsMemoryDumpProviderTest, DumpRSS); @@ -51,16 +57,18 @@ #elif defined(OS_WIN) FRIEND_TEST_ALL_PREFIXES(ProcessMetricsMemoryDumpProviderTest, TestWinModuleReading); +#elif defined(OS_LINUX) || defined(OS_ANDROID) + FRIEND_TEST_ALL_PREFIXES(ProcessMetricsMemoryDumpProviderTest, + DoubleRegister); #endif - ProcessMetricsMemoryDumpProvider(base::ProcessId process); - bool DumpProcessTotals(const base::trace_event::MemoryDumpArgs& args, base::trace_event::ProcessMemoryDump* pmd); bool DumpProcessMemoryMaps(const base::trace_event::MemoryDumpArgs& args, base::trace_event::ProcessMemoryDump* pmd); static uint64_t rss_bytes_for_testing; + static FactoryFunction factory_for_testing; #if defined(OS_LINUX) || defined(OS_ANDROID) static FILE* proc_smaps_for_testing;
diff --git a/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc b/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc index 95a565d..7712434 100644 --- a/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc +++ b/components/tracing/common/process_metrics_memory_dump_provider_unittest.cc
@@ -7,10 +7,12 @@ #include <stdint.h> #include <memory> +#include <unordered_set> #include "base/files/file_util.h" #include "base/memory/ptr_util.h" #include "base/process/process_metrics.h" +#include "base/trace_event/memory_dump_manager.h" #include "base/trace_event/process_memory_dump.h" #include "base/trace_event/process_memory_maps.h" #include "base/trace_event/process_memory_totals.h" @@ -133,6 +135,25 @@ } // namespace #endif // defined(OS_LINUX) || defined(OS_ANDROID) +class MockMemoryDumpProvider : public ProcessMetricsMemoryDumpProvider { + public: + MockMemoryDumpProvider(base::ProcessId process); + ~MockMemoryDumpProvider() override; +}; + +std::unordered_set<MockMemoryDumpProvider*> g_live_mocks; +std::unordered_set<MockMemoryDumpProvider*> g_dead_mocks; + +MockMemoryDumpProvider::MockMemoryDumpProvider(base::ProcessId process) + : ProcessMetricsMemoryDumpProvider(process) { + g_live_mocks.insert(this); +} + +MockMemoryDumpProvider::~MockMemoryDumpProvider() { + g_live_mocks.erase(this); + g_dead_mocks.insert(this); +} + TEST(ProcessMetricsMemoryDumpProviderTest, DumpRSS) { const base::trace_event::MemoryDumpArgs high_detail_args = { base::trace_event::MemoryDumpLevelOfDetail::DETAILED}; @@ -244,6 +265,27 @@ EXPECT_EQ(4 * 1024UL, regions_2[0].byte_stats_private_dirty_resident); EXPECT_EQ(0 * 1024UL, regions_2[0].byte_stats_swapped); } + +TEST(ProcessMetricsMemoryDumpProviderTest, DoubleRegister) { + auto factory = [](base::ProcessId process) { + return std::unique_ptr<ProcessMetricsMemoryDumpProvider>( + new MockMemoryDumpProvider(process)); + }; + ProcessMetricsMemoryDumpProvider::factory_for_testing = factory; + ProcessMetricsMemoryDumpProvider::RegisterForProcess(1); + ProcessMetricsMemoryDumpProvider::RegisterForProcess(1); + ASSERT_EQ(1u, g_live_mocks.size()); + ASSERT_EQ(1u, g_dead_mocks.size()); + auto* manager = base::trace_event::MemoryDumpManager::GetInstance(); + MockMemoryDumpProvider* live_mock = *g_live_mocks.begin(); + EXPECT_TRUE(manager->IsDumpProviderRegisteredForTesting(live_mock)); + auto* dead_mock = *g_dead_mocks.begin(); + EXPECT_FALSE(manager->IsDumpProviderRegisteredForTesting(dead_mock)); + ProcessMetricsMemoryDumpProvider::UnregisterForProcess(1); + g_live_mocks.clear(); + g_dead_mocks.clear(); +} + #endif // defined(OS_LINUX) || defined(OS_ANDROID) TEST(ProcessMetricsMemoryDumpProviderTest, TestPollFastMemoryTotal) { @@ -392,5 +434,4 @@ } #endif // defined(OS_MACOSX) - } // namespace tracing
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc index c20e6bd5..0a714ef 100644 --- a/content/browser/frame_host/navigation_handle_impl.cc +++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -695,9 +695,9 @@ case NavigationThrottle::PROCEED: continue; + case NavigationThrottle::BLOCK_REQUEST: case NavigationThrottle::CANCEL: case NavigationThrottle::CANCEL_AND_IGNORE: - case NavigationThrottle::BLOCK_REQUEST: state_ = CANCELING; return result; @@ -727,6 +727,9 @@ case NavigationThrottle::PROCEED: continue; + case NavigationThrottle::BLOCK_REQUEST: + CHECK(IsBrowserSideNavigationEnabled()) + << "BLOCK_REQUEST must not be used on redirect without PlzNavigate"; case NavigationThrottle::CANCEL: case NavigationThrottle::CANCEL_AND_IGNORE: state_ = CANCELING; @@ -737,7 +740,6 @@ next_index_ = i + 1; return result; - case NavigationThrottle::BLOCK_REQUEST: case NavigationThrottle::BLOCK_RESPONSE: NOTREACHED(); }
diff --git a/content/browser/frame_host/navigation_handle_impl_browsertest.cc b/content/browser/frame_host/navigation_handle_impl_browsertest.cc index 44a1581..ff2536a 100644 --- a/content/browser/frame_host/navigation_handle_impl_browsertest.cc +++ b/content/browser/frame_host/navigation_handle_impl_browsertest.cc
@@ -7,6 +7,7 @@ #include "content/browser/web_contents/web_contents_impl.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" +#include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/request_context_type.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" @@ -902,6 +903,21 @@ EXPECT_FALSE(NavigateToURL(shell(), kUrl)); EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code()); } + + // Using BLOCK_REQUEST on redirect is available only with PlzNavigate. + if (IsBrowserSideNavigationEnabled()) { + // Set up a NavigationThrottle that will block the navigation in + // WillRedirectRequest. + TestNavigationThrottleInstaller installer( + shell()->web_contents(), NavigationThrottle::PROCEED, + NavigationThrottle::BLOCK_REQUEST, NavigationThrottle::PROCEED); + NavigationHandleObserver observer(shell()->web_contents(), kRedirectingUrl); + + // Try to navigate to the url. The navigation should be canceled and the + // NavigationHandle should have the right error code. + EXPECT_FALSE(NavigateToURL(shell(), kRedirectingUrl)); + EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code()); + } } // Specialized test that verifies the NavigationHandle gets the HTTPS upgraded @@ -1036,4 +1052,70 @@ } } +// Record and list the navigations that are started and finished. +class NavigationLogger : public WebContentsObserver { + public: + NavigationLogger(WebContents* web_contents) + : WebContentsObserver(web_contents) {} + + void DidStartNavigation(NavigationHandle* navigation_handle) override { + started_navigation_urls_.push_back(navigation_handle->GetURL()); + } + + void DidFinishNavigation(NavigationHandle* navigation_handle) override { + finished_navigation_urls_.push_back(navigation_handle->GetURL()); + } + + const std::vector<GURL>& started_navigation_urls() const { + return started_navigation_urls_; + } + const std::vector<GURL>& finished_navigation_urls() const { + return finished_navigation_urls_; + } + + private: + std::vector<GURL> started_navigation_urls_; + std::vector<GURL> finished_navigation_urls_; +}; + +// There was a bug without PlzNavigate that happened when a navigation was +// blocked after a redirect. Blink didn't know about the redirect and tried +// to commit an error page to the pre-redirect URL. The result was that the +// NavigationHandle was not found on the browser-side and a new NavigationHandle +// created for committing the error page. This test makes sure that only one +// NavigationHandle is used for committing the error page. +// See https://crbug.com/695421 +IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest, BlockedOnRedirect) { + // Returning BLOCK_REQUEST is not supported yet without PlzNavigate. It will + // call a CHECK(false). + // TODO(arthursonzogni) Provide support for BLOCK_REQUEST without PlzNavigate + // once https://crbug.com/695421 is fixed. + if (!IsBrowserSideNavigationEnabled()) + return; + + const GURL kUrl = embedded_test_server()->GetURL("/title1.html"); + const GURL kRedirectingUrl = + embedded_test_server()->GetURL("/server-redirect?" + kUrl.spec()); + + // Set up a NavigationThrottle that will block the navigation in + // WillRedirectRequest. + TestNavigationThrottleInstaller installer( + shell()->web_contents(), NavigationThrottle::PROCEED, + NavigationThrottle::BLOCK_REQUEST, NavigationThrottle::PROCEED); + NavigationHandleObserver observer(shell()->web_contents(), kRedirectingUrl); + NavigationLogger logger(shell()->web_contents()); + + // Try to navigate to the url. The navigation should be canceled and the + // NavigationHandle should have the right error code. + EXPECT_FALSE(NavigateToURL(shell(), kRedirectingUrl)); + // EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, observer.net_error_code()); + + // Only one navigation is expected to happen. + std::vector<GURL> started_navigation = {kRedirectingUrl}; + EXPECT_EQ(started_navigation, logger.started_navigation_urls()); + + std::vector<GURL> finished_navigation = {kUrl}; + EXPECT_EQ(finished_navigation, logger.finished_navigation_urls()); +} + } // namespace content
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc index e08de57..a1b6e73d 100644 --- a/content/browser/frame_host/navigation_request.cc +++ b/content/browser/frame_host/navigation_request.cc
@@ -735,6 +735,14 @@ return; } + if (result == NavigationThrottle::BLOCK_REQUEST) { + OnRequestFailed(false, net::ERR_BLOCKED_BY_CLIENT); + + // DO NOT ADD CODE after this. The previous call to OnRequestFailed has + // destroyed the NavigationRequest. + return; + } + loader_->FollowRedirect(); }
diff --git a/content/browser/memory/memory_coordinator_impl.cc b/content/browser/memory/memory_coordinator_impl.cc index 0e2cfa9..acbb7620 100644 --- a/content/browser/memory/memory_coordinator_impl.cc +++ b/content/browser/memory/memory_coordinator_impl.cc
@@ -155,6 +155,10 @@ kDefaultMinimumTransitionPeriodSeconds)) { DCHECK(memory_monitor_.get()); base::MemoryCoordinatorProxy::SetMemoryCoordinator(this); + + // Force the "memory_coordinator" category to show up in the trace viewer. + base::trace_event::TraceLog::GetCategoryGroupEnabled( + TRACE_DISABLED_BY_DEFAULT("memory_coordinator")); } MemoryCoordinatorImpl::~MemoryCoordinatorImpl() { @@ -280,7 +284,7 @@ MemoryCondition prev_condition = memory_condition_; memory_condition_ = next_condition; - TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("memory-infra"), + TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("memory_coordinator"), "MemoryCoordinatorImpl::UpdateConditionIfNeeded", "prev", MemoryConditionToString(prev_condition), "next", MemoryConditionToString(next_condition));
diff --git a/content/child/memory/child_memory_coordinator_impl.cc b/content/child/memory/child_memory_coordinator_impl.cc index 6616a22..3bc4fda1 100644 --- a/content/child/memory/child_memory_coordinator_impl.cc +++ b/content/child/memory/child_memory_coordinator_impl.cc
@@ -64,14 +64,14 @@ } void ChildMemoryCoordinatorImpl::PurgeMemory() { - TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("memory-infra"), + TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("memory_coordinator"), "ChildMemoryCoordinatorImpl::PurgeMemory"); base::MemoryCoordinatorClientRegistry::GetInstance()->PurgeMemory(); } void ChildMemoryCoordinatorImpl::OnStateChange(mojom::MemoryState state) { current_state_ = ToBaseMemoryState(state); - TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("memory-infra"), + TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("memory_coordinator"), "ChildMemoryCoordinatorImpl::OnStateChange", "state", MemoryStateToString(current_state_)); base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(current_state_);
diff --git a/content/public/browser/navigation_throttle.h b/content/public/browser/navigation_throttle.h index ba49aa7..109a2fc4 100644 --- a/content/public/browser/navigation_throttle.h +++ b/content/public/browser/navigation_throttle.h
@@ -34,7 +34,8 @@ CANCEL_AND_IGNORE, // Blocks a navigation due to rules asserted before the request is made. - // This can only be returned from WillStartRequest. This will result in an + // This can only be returned from WillStartRequest and also from + // WillRedirectRequest when PlzNavigate is enabled. This will result in an // error page for net::ERR_BLOCKED_BY_CLIENT being loaded in the frame that // is navigated. BLOCK_REQUEST,
diff --git a/ios/web_view/BUILD.gn b/ios/web_view/BUILD.gn index f96dc15..c2310ec 100644 --- a/ios/web_view/BUILD.gn +++ b/ios/web_view/BUILD.gn
@@ -31,7 +31,6 @@ "public/cwv_ui_delegate.h", "public/cwv_web_view.h", "public/cwv_web_view_configuration.h", - "public/cwv_website_data_store.h", ] deps = [
diff --git a/ios/web_view/internal/BUILD.gn b/ios/web_view/internal/BUILD.gn index 3031f76..1622a36 100644 --- a/ios/web_view/internal/BUILD.gn +++ b/ios/web_view/internal/BUILD.gn
@@ -14,8 +14,7 @@ "cwv_html_element_internal.h", "cwv_web_view.mm", "cwv_web_view_configuration.mm", - "cwv_website_data_store.mm", - "cwv_website_data_store_internal.h", + "cwv_web_view_configuration_internal.h", "pref_names.cc", "pref_names.h", "web_view_browser_state.h",
diff --git a/ios/web_view/internal/cwv.mm b/ios/web_view/internal/cwv.mm index e24120c..e692bb4 100644 --- a/ios/web_view/internal/cwv.mm +++ b/ios/web_view/internal/cwv.mm
@@ -16,7 +16,6 @@ #import "ios/web_view/public/cwv_delegate.h" #import "ios/web_view/public/cwv_web_view.h" #import "ios/web_view/public/cwv_web_view_configuration.h" -#import "ios/web_view/public/cwv_website_data_store.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -50,9 +49,7 @@ + (CWVWebView*)webViewWithFrame:(CGRect)frame { CWVWebViewConfiguration* configuration = - [[CWVWebViewConfiguration alloc] init]; - configuration.websiteDataStore = [CWVWebsiteDataStore defaultDataStore]; - + [CWVWebViewConfiguration defaultConfiguration]; return [[CWVWebView alloc] initWithFrame:frame configuration:configuration]; }
diff --git a/ios/web_view/internal/cwv_web_view.mm b/ios/web_view/internal/cwv_web_view.mm index ff7c2d1e..6df0d2a 100644 --- a/ios/web_view/internal/cwv_web_view.mm +++ b/ios/web_view/internal/cwv_web_view.mm
@@ -20,7 +20,7 @@ #import "ios/web/public/web_state/web_state_delegate_bridge.h" #import "ios/web/public/web_state/web_state_observer_bridge.h" #import "ios/web_view/internal/cwv_html_element_internal.h" -#import "ios/web_view/internal/cwv_website_data_store_internal.h" +#import "ios/web_view/internal/cwv_web_view_configuration_internal.h" #import "ios/web_view/internal/translate/web_view_translate_client.h" #include "ios/web_view/internal/web_view_browser_state.h" #import "ios/web_view/internal/web_view_java_script_dialog_presenter.h" @@ -28,7 +28,6 @@ #import "ios/web_view/public/cwv_navigation_delegate.h" #import "ios/web_view/public/cwv_ui_delegate.h" #import "ios/web_view/public/cwv_web_view_configuration.h" -#import "ios/web_view/public/cwv_website_data_store.h" #import "net/base/mac/url_conversions.h" #include "ui/base/page_transition_types.h" #include "url/gurl.h" @@ -66,7 +65,7 @@ _configuration = [configuration copy]; web::WebState::CreateParams webStateCreateParams( - [configuration.websiteDataStore browserState]); + configuration.browserState); _webState = web::WebState::Create(webStateCreateParams); _webState->SetWebUsageEnabled(true);
diff --git a/ios/web_view/internal/cwv_web_view_configuration.mm b/ios/web_view/internal/cwv_web_view_configuration.mm index 01568c5..f29c002 100644 --- a/ios/web_view/internal/cwv_web_view_configuration.mm +++ b/ios/web_view/internal/cwv_web_view_configuration.mm
@@ -3,39 +3,70 @@ // found in the LICENSE file. #import "ios/web_view/public/cwv_web_view_configuration.h" +#import "ios/web_view/internal/cwv_web_view_configuration_internal.h" -#import "ios/web_view/public/cwv_website_data_store.h" +#import "ios/web_view/internal/web_view_browser_state.h" +#import "ios/web_view/internal/web_view_web_client.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." #endif @interface CWVWebViewConfiguration () -// Initialize configuration with specified data store. -- (instancetype)initWithDataStore:(CWVWebsiteDataStore*)dataStore; +// Initialize configuration with the specified browser state. +- (instancetype)initWithBrowserState: + (ios_web_view::WebViewBrowserState*)browserState; @end -@implementation CWVWebViewConfiguration - -@synthesize websiteDataStore = _websiteDataStore; - -- (instancetype)init { - return [self initWithDataStore:[CWVWebsiteDataStore defaultDataStore]]; +@implementation CWVWebViewConfiguration { + // TODO(crbug.com/690182): CWVWebViewConfiguration should own _browserState. + ios_web_view::WebViewBrowserState* _browserState; } -- (instancetype)initWithDataStore:(CWVWebsiteDataStore*)dataStore { ++ (instancetype)defaultConfiguration { + static dispatch_once_t once; + static CWVWebViewConfiguration* configuration; + dispatch_once(&once, ^{ + ios_web_view::WebViewWebClient* client = + static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient()); + configuration = [[self alloc] initWithBrowserState:client->browser_state()]; + }); + return configuration; +} + ++ (instancetype)incognitoConfiguration { + static dispatch_once_t once; + static CWVWebViewConfiguration* configuration; + dispatch_once(&once, ^{ + ios_web_view::WebViewWebClient* client = + static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient()); + configuration = [[self alloc] + initWithBrowserState:client->off_the_record_browser_state()]; + }); + return configuration; +} + +- (instancetype)initWithBrowserState: + (ios_web_view::WebViewBrowserState*)browserState { self = [super init]; if (self) { - _websiteDataStore = dataStore; + _browserState = browserState; } return self; } +- (BOOL)isPersistent { + return !_browserState->IsOffTheRecord(); +} + +- (ios_web_view::WebViewBrowserState*)browserState { + return _browserState; +} + // NSCopying - (id)copyWithZone:(NSZone*)zone { - return - [[[self class] allocWithZone:zone] initWithDataStore:_websiteDataStore]; + return [[[self class] allocWithZone:zone] initWithBrowserState:_browserState]; } @end
diff --git a/ios/web_view/internal/cwv_web_view_configuration_internal.h b/ios/web_view/internal/cwv_web_view_configuration_internal.h new file mode 100644 index 0000000..d7942d6 --- /dev/null +++ b/ios/web_view/internal/cwv_web_view_configuration_internal.h
@@ -0,0 +1,22 @@ +// 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. + +#ifndef IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_CONFIGURATION_INTERNAL_H_ +#define IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_CONFIGURATION_INTERNAL_H_ + +#import "ios/web_view/public/cwv_web_view_configuration.h" + +namespace ios_web_view { +class WebViewBrowserState; +} // namespace ios_web_view + +@interface CWVWebViewConfiguration () + +// The browser state associated with this configuration. +@property(nonatomic, readonly, nonnull) + ios_web_view::WebViewBrowserState* browserState; + +@end + +#endif // IOS_WEB_VIEW_INTERNAL_CWV_WEB_VIEW_CONFIGURATION_INTERNAL_H_
diff --git a/ios/web_view/internal/cwv_website_data_store.mm b/ios/web_view/internal/cwv_website_data_store.mm deleted file mode 100644 index da503472..0000000 --- a/ios/web_view/internal/cwv_website_data_store.mm +++ /dev/null
@@ -1,54 +0,0 @@ -// 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. - -#import "ios/web_view/internal/cwv_website_data_store_internal.h" - -#include <memory.h> - -#include "ios/web_view/internal/web_view_browser_state.h" -#import "ios/web_view/internal/web_view_web_client.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -@implementation CWVWebsiteDataStore { - // TODO(crbug.com/690182): CWVWebsiteDataStore should own _browserState. - ios_web_view::WebViewBrowserState* _browserState; -} - -- (BOOL)isPersistent { - return !_browserState->IsOffTheRecord(); -} - -- (ios_web_view::WebViewBrowserState*)browserState { - return _browserState; -} - -- (void)setBrowserState: - (ios_web_view::WebViewBrowserState* _Nonnull)browserState { - _browserState = browserState; -} - -+ (instancetype)defaultDataStore { - CWVWebsiteDataStore* dataStore = [[CWVWebsiteDataStore alloc] init]; - - ios_web_view::WebViewWebClient* client = - static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient()); - [dataStore setBrowserState:client->browser_state()]; - - return dataStore; -} - -+ (instancetype)nonPersistentDataStore { - CWVWebsiteDataStore* dataStore = [[CWVWebsiteDataStore alloc] init]; - - ios_web_view::WebViewWebClient* client = - static_cast<ios_web_view::WebViewWebClient*>(web::GetWebClient()); - [dataStore setBrowserState:client->off_the_record_browser_state()]; - - return dataStore; -} - -@end
diff --git a/ios/web_view/internal/cwv_website_data_store_internal.h b/ios/web_view/internal/cwv_website_data_store_internal.h deleted file mode 100644 index 31035e1..0000000 --- a/ios/web_view/internal/cwv_website_data_store_internal.h +++ /dev/null
@@ -1,22 +0,0 @@ -// 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. - -#ifndef IOS_WEB_VIEW_INTERNAL_CWV_WEBSITE_DATA_STORE_INTERNAL_H_ -#define IOS_WEB_VIEW_INTERNAL_CWV_WEBSITE_DATA_STORE_INTERNAL_H_ - -#import "ios/web_view/public/cwv_website_data_store.h" - -namespace ios_web_view { -class WebViewBrowserState; -} // namespace ios_web_view - -@interface CWVWebsiteDataStore () - -// The browser state associated with this website data store. -@property(nonatomic, readonly, nonnull) - ios_web_view::WebViewBrowserState* browserState; - -@end - -#endif // IOS_WEB_VIEW_INTERNAL_CWV_WEBSITE_DATA_STORE_INTERNAL_H_
diff --git a/ios/web_view/public/BUILD.gn b/ios/web_view/public/BUILD.gn index 85c95e9..3bf4fb9c 100644 --- a/ios/web_view/public/BUILD.gn +++ b/ios/web_view/public/BUILD.gn
@@ -18,7 +18,6 @@ "cwv_ui_delegate.h", "cwv_web_view.h", "cwv_web_view_configuration.h", - "cwv_website_data_store.h", ] libs = [ "UIKit.framework" ]
diff --git a/ios/web_view/public/cwv_web_view_configuration.h b/ios/web_view/public/cwv_web_view_configuration.h index aa4b96f..a2a7489 100644 --- a/ios/web_view/public/cwv_web_view_configuration.h +++ b/ios/web_view/public/cwv_web_view_configuration.h
@@ -12,9 +12,17 @@ // Configuration used for creation of a CWVWebView. @interface CWVWebViewConfiguration : NSObject<NSCopying> -// Data store defining persistance of website data. Default is -// [CWVWebsiteDataStore defaultDataStore]. -@property(nonatomic, strong, nonnull) CWVWebsiteDataStore* websiteDataStore; +// Configuration with persistent data store which stores all data on disk. ++ (instancetype)defaultConfiguration; + +// Configuration with ephemeral data store that neven stores data on disk. ++ (instancetype)incognitoConfiguration; + +- (instancetype)init NS_UNAVAILABLE; + +// YES if it is a configuration with persistent data store which stores all data +// on disk. +@property(readonly, getter=isPersistent) BOOL persistent; @end
diff --git a/ios/web_view/public/cwv_website_data_store.h b/ios/web_view/public/cwv_website_data_store.h deleted file mode 100644 index c6359b5..0000000 --- a/ios/web_view/public/cwv_website_data_store.h +++ /dev/null
@@ -1,24 +0,0 @@ -// 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. - -#ifndef IOS_WEB_VIEW_PUBLIC_CWV_WEBSITE_DATA_STORE_H_ -#define IOS_WEB_VIEW_PUBLIC_CWV_WEBSITE_DATA_STORE_H_ - -#import <Foundation/Foundation.h> - -// Controls persistence of website data such as cookies, caches, and local -// storage. -@interface CWVWebsiteDataStore : NSObject - -// Whether or not this data store persists data to disk. -@property(readonly, getter=isPersistent) BOOL persistent; - -// Persistent data store which stores all data on disk. -+ (instancetype)defaultDataStore; -// Ephemeral data store that never stores data on disk. -+ (instancetype)nonPersistentDataStore; - -@end - -#endif // IOS_WEB_VIEW_PUBLIC_CWV_WEBSITE_DATA_STORE_H_
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 61a76cc4..279c5f13 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2832,3 +2832,7 @@ # Sheriff failures 2017-03-13 crbug.com/701140 inspector-protocol/css/css-get-rule-list.html [ Failure Pass ] + +# Sheriff failures 2017-03-16 +crbug.com/701718 [ Win7 ] fast/table/border-collapsing/001-vertical.html [ Failure Pass ] +crbug.com/701718 [ Win7 ] fast/table/border-collapsing/001.html [ Failure Pass ]
diff --git a/third_party/WebKit/LayoutTests/editing/deleting/display-table.html b/third_party/WebKit/LayoutTests/editing/deleting/display-table.html index 3b038779..b845437 100644 --- a/third_party/WebKit/LayoutTests/editing/deleting/display-table.html +++ b/third_party/WebKit/LayoutTests/editing/deleting/display-table.html
@@ -26,9 +26,12 @@ } test(function () { - runTest(0, "forwardDelete", "ABCD"); - runTest(1, "Delete", "ABCD"); -}, "This test verifies that we are able to successfully delete the first and the last characters of a contenteditable div with display: table."); + runTest(0, "forwardDelete", "BCD"); +}, "This test verifies that ForwardDelete command is able to successfully delete the first character of a contenteditable div with display: table."); + +test(function () { + runTest(1, "Delete", "ABC"); +}, "This test verifies that Delete command is able to successfully delete the last character of a contenteditable div with display: table."); </script> </body> </html>
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp index fa50898..82fd778 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp
@@ -33,21 +33,13 @@ #include "bindings/core/v8/ScriptController.h" #include "bindings/core/v8/ScriptSourceCode.h" -#include "bindings/core/v8/ScriptValue.h" #include "bindings/core/v8/V8Binding.h" -#include "bindings/core/v8/V8Event.h" #include "bindings/core/v8/V8GCController.h" -#include "bindings/core/v8/V8HTMLElement.h" -#include "bindings/core/v8/V8PerContextData.h" #include "bindings/core/v8/V8ScriptRunner.h" -#include "bindings/core/v8/V8Window.h" #include "bindings/core/v8/WindowProxy.h" #include "core/dom/Document.h" -#include "core/dom/Node.h" #include "core/dom/ScriptableDocumentParser.h" -#include "core/events/Event.h" -#include "core/events/EventListener.h" -#include "core/frame/LocalDOMWindow.h" +#include "core/frame/LocalFrame.h" #include "core/frame/LocalFrameClient.h" #include "core/frame/Settings.h" #include "core/frame/UseCounter.h" @@ -67,20 +59,16 @@ #include "platform/UserGestureIndicator.h" #include "platform/instrumentation/tracing/TraceEvent.h" #include "platform/weborigin/SecurityOrigin.h" -#include "public/platform/Platform.h" #include "wtf/CurrentTime.h" #include "wtf/StdLibExtras.h" #include "wtf/StringExtras.h" #include "wtf/text/CString.h" #include "wtf/text/StringBuilder.h" -#include "wtf/text/TextPosition.h" namespace blink { -ScriptController::ScriptController(LocalFrame* frame) - : m_windowProxyManager(LocalWindowProxyManager::create(*frame)) {} - DEFINE_TRACE(ScriptController) { + visitor->trace(m_frame); visitor->trace(m_windowProxyManager); } @@ -153,12 +141,6 @@ return result; } -LocalWindowProxy* ScriptController::windowProxy(DOMWrapperWorld& world) { - LocalWindowProxy* windowProxy = m_windowProxyManager->windowProxy(world); - windowProxy->initializeIfNeeded(); - return windowProxy; -} - bool ScriptController::shouldBypassMainWorldCSP() { v8::HandleScope handleScope(isolate()); v8::Local<v8::Context> context = isolate()->GetCurrentContext(); @@ -180,7 +162,8 @@ void ScriptController::enableEval() { v8::HandleScope handleScope(isolate()); v8::Local<v8::Context> v8Context = - m_windowProxyManager->mainWorldProxy()->contextIfInitialized(); + m_windowProxyManager->mainWorldProxyMaybeUninitialized() + ->contextIfInitialized(); if (v8Context.IsEmpty()) return; v8Context->AllowCodeGenerationFromStrings(true); @@ -189,7 +172,8 @@ void ScriptController::disableEval(const String& errorMessage) { v8::HandleScope handleScope(isolate()); v8::Local<v8::Context> v8Context = - m_windowProxyManager->mainWorldProxy()->contextIfInitialized(); + m_windowProxyManager->mainWorldProxyMaybeUninitialized() + ->contextIfInitialized(); if (v8Context.IsEmpty()) return; v8Context->AllowCodeGenerationFromStrings(false); @@ -237,7 +221,7 @@ } void ScriptController::updateDocument() { - m_windowProxyManager->mainWorldProxy()->updateDocument(); + m_windowProxyManager->mainWorldProxyMaybeUninitialized()->updateDocument(); } bool ScriptController::executeScriptIfJavaScriptURL(const KURL& url,
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptController.h b/third_party/WebKit/Source/bindings/core/v8/ScriptController.h index 5428ae47..3469048 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptController.h +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptController.h
@@ -35,7 +35,6 @@ #include "bindings/core/v8/WindowProxyManager.h" #include "core/CoreExport.h" #include "core/dom/ExecutionContext.h" -#include "core/frame/LocalFrame.h" #include "platform/heap/Handle.h" #include "platform/loader/fetch/AccessControlStatus.h" #include "platform/loader/fetch/CrossOriginAccessControl.h" @@ -51,6 +50,7 @@ class Element; class FrameViewBase; class KURL; +class LocalFrame; class ScriptSourceCode; class SecurityOrigin; @@ -67,15 +67,18 @@ DoNotExecuteScriptWhenScriptsDisabled }; - static ScriptController* create(LocalFrame* frame) { - return new ScriptController(frame); + static ScriptController* create(LocalFrame& frame, + LocalWindowProxyManager& windowProxyManager) { + return new ScriptController(frame, windowProxyManager); } DECLARE_TRACE(); // This returns an initialized window proxy. (If the window proxy is not // yet initialized, it's implicitly initialized at the first access.) - LocalWindowProxy* windowProxy(DOMWrapperWorld&); + LocalWindowProxy* windowProxy(DOMWrapperWorld& world) { + return m_windowProxyManager->windowProxy(world); + } // Evaluate JavaScript in the main world. void executeScriptInMainWorld( @@ -131,22 +134,20 @@ static void registerExtensionIfNeeded(v8::Extension*); static V8Extensions& registeredExtensions(); - v8::Isolate* isolate() const { return m_windowProxyManager->isolate(); } - - LocalWindowProxyManager* getWindowProxyManager() const { - return m_windowProxyManager.get(); - } - private: - explicit ScriptController(LocalFrame*); + ScriptController(LocalFrame& frame, + LocalWindowProxyManager& windowProxyManager) + : m_frame(&frame), m_windowProxyManager(&windowProxyManager) {} - LocalFrame* frame() const { return m_windowProxyManager->frame(); } + LocalFrame* frame() const { return m_frame; } + v8::Isolate* isolate() const { return m_windowProxyManager->isolate(); } v8::Local<v8::Value> evaluateScriptInMainWorld(const ScriptSourceCode&, AccessControlStatus, ExecuteScriptPolicy); - Member<LocalWindowProxyManager> m_windowProxyManager; + const Member<LocalFrame> m_frame; + const Member<LocalWindowProxyManager> m_windowProxyManager; }; } // namespace blink
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp b/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp index 33e3c667..3d01b570 100644 --- a/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp
@@ -951,8 +951,8 @@ } v8::Isolate* toIsolate(LocalFrame* frame) { - ASSERT(frame); - return frame->script().isolate(); + DCHECK(frame); + return frame->getWindowProxyManager()->isolate(); } v8::Local<v8::Value> freezeV8Object(v8::Local<v8::Value> value,
diff --git a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp index a96d286..8097cdb8 100644 --- a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp
@@ -8,28 +8,65 @@ namespace blink { -namespace { - -WindowProxy* createWindowProxyForFrame(v8::Isolate* isolate, - Frame& frame, - - RefPtr<DOMWrapperWorld> world) { - if (frame.isLocalFrame()) { - return LocalWindowProxy::create(isolate, toLocalFrame(frame), - std::move(world)); - } - return RemoteWindowProxy::create(isolate, toRemoteFrame(frame), - std::move(world)); -} -} - -DEFINE_TRACE(WindowProxyManagerBase) { +DEFINE_TRACE(WindowProxyManager) { visitor->trace(m_frame); visitor->trace(m_windowProxy); visitor->trace(m_isolatedWorlds); } -WindowProxy* WindowProxyManagerBase::windowProxy(DOMWrapperWorld& world) { +void WindowProxyManager::clearForClose() { + m_windowProxy->clearForClose(); + for (auto& entry : m_isolatedWorlds) + entry.value->clearForClose(); +} + +void WindowProxyManager::clearForNavigation() { + m_windowProxy->clearForNavigation(); + for (auto& entry : m_isolatedWorlds) + entry.value->clearForNavigation(); +} + +void WindowProxyManager::releaseGlobals(GlobalsVector& globals) { + globals.reserveInitialCapacity(1 + m_isolatedWorlds.size()); + globals.emplace_back(&m_windowProxy->world(), m_windowProxy->releaseGlobal()); + for (auto& entry : m_isolatedWorlds) { + globals.emplace_back( + &entry.value->world(), + windowProxyMaybeUninitialized(entry.value->world())->releaseGlobal()); + } +} + +void WindowProxyManager::setGlobals(const GlobalsVector& globals) { + for (const auto& entry : globals) + windowProxyMaybeUninitialized(*entry.first)->setGlobal(entry.second); +} + +WindowProxyManager::WindowProxyManager(Frame& frame, FrameType frameType) + : m_isolate(v8::Isolate::GetCurrent()), + m_frame(&frame), + m_frameType(frameType), + m_windowProxy(createWindowProxy(DOMWrapperWorld::mainWorld())) {} + +WindowProxy* WindowProxyManager::createWindowProxy(DOMWrapperWorld& world) { + switch (m_frameType) { + case FrameType::Local: + // Directly use static_cast instead of toLocalFrame because + // WindowProxyManager gets instantiated during a construction of + // LocalFrame and at that time virtual member functions are not yet + // available (we cannot use LocalFrame::isLocalFrame). Ditto for + // RemoteFrame. + return LocalWindowProxy::create( + m_isolate, *static_cast<LocalFrame*>(m_frame.get()), &world); + case FrameType::Remote: + return RemoteWindowProxy::create( + m_isolate, *static_cast<RemoteFrame*>(m_frame.get()), &world); + } + NOTREACHED(); + return nullptr; +} + +WindowProxy* WindowProxyManager::windowProxyMaybeUninitialized( + DOMWrapperWorld& world) { WindowProxy* windowProxy = nullptr; if (world.isMainWorld()) { windowProxy = m_windowProxy.get(); @@ -38,51 +75,19 @@ if (iter != m_isolatedWorlds.end()) { windowProxy = iter->value.get(); } else { - windowProxy = createWindowProxyForFrame(m_isolate, *m_frame, &world); + windowProxy = createWindowProxy(world); m_isolatedWorlds.set(world.worldId(), windowProxy); } } return windowProxy; } -void WindowProxyManagerBase::clearForClose() { - m_windowProxy->clearForClose(); - for (auto& entry : m_isolatedWorlds) - entry.value->clearForClose(); -} - -void WindowProxyManagerBase::clearForNavigation() { - m_windowProxy->clearForNavigation(); - for (auto& entry : m_isolatedWorlds) - entry.value->clearForNavigation(); -} - -void WindowProxyManagerBase::releaseGlobals(GlobalsVector& globals) { - globals.reserveInitialCapacity(1 + m_isolatedWorlds.size()); - globals.emplace_back(&m_windowProxy->world(), m_windowProxy->releaseGlobal()); - for (auto& entry : m_isolatedWorlds) { - globals.emplace_back(&entry.value->world(), - windowProxy(entry.value->world())->releaseGlobal()); - } -} - -void WindowProxyManagerBase::setGlobals(const GlobalsVector& globals) { - for (const auto& entry : globals) - windowProxy(*entry.first)->setGlobal(entry.second); -} - -WindowProxyManagerBase::WindowProxyManagerBase(Frame& frame) - : m_isolate(v8::Isolate::GetCurrent()), - m_frame(&frame), - m_windowProxy(createWindowProxyForFrame(m_isolate, - frame, - &DOMWrapperWorld::mainWorld())) {} - void LocalWindowProxyManager::updateSecurityOrigin( SecurityOrigin* securityOrigin) { - static_cast<LocalWindowProxy*>(mainWorldProxy()) + static_cast<LocalWindowProxy*>(m_windowProxy.get()) ->updateSecurityOrigin(securityOrigin); - for (auto& entry : isolatedWorlds()) { + + for (auto& entry : m_isolatedWorlds) { auto* isolatedWindowProxy = static_cast<LocalWindowProxy*>(entry.value.get()); SecurityOrigin* isolatedSecurityOrigin =
diff --git a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h index 35f4427..5659222 100644 --- a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h +++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
@@ -19,9 +19,8 @@ class DOMWrapperWorld; class SecurityOrigin; -class ScriptController; -class WindowProxyManagerBase : public GarbageCollected<WindowProxyManagerBase> { +class WindowProxyManager : public GarbageCollected<WindowProxyManager> { public: DECLARE_TRACE(); @@ -38,32 +37,44 @@ void CORE_EXPORT releaseGlobals(GlobalsVector&); void CORE_EXPORT setGlobals(const GlobalsVector&); + WindowProxy* mainWorldProxy() { + m_windowProxy->initializeIfNeeded(); + return m_windowProxy; + } + + WindowProxy* windowProxy(DOMWrapperWorld& world) { + WindowProxy* windowProxy = windowProxyMaybeUninitialized(world); + windowProxy->initializeIfNeeded(); + return windowProxy; + } + protected: using IsolatedWorldMap = HeapHashMap<int, Member<WindowProxy>>; + enum class FrameType { Local, Remote }; - explicit WindowProxyManagerBase(Frame&); - - Frame* frame() const { return m_frame; } - WindowProxy* mainWorldProxy() const { return m_windowProxy.get(); } - WindowProxy* windowProxy(DOMWrapperWorld&); - - IsolatedWorldMap& isolatedWorlds() { return m_isolatedWorlds; } + WindowProxyManager(Frame&, FrameType); private: + // Creates an uninitialized WindowProxy. + WindowProxy* createWindowProxy(DOMWrapperWorld&); + WindowProxy* windowProxyMaybeUninitialized(DOMWrapperWorld&); + v8::Isolate* const m_isolate; const Member<Frame> m_frame; + const FrameType m_frameType; + + protected: const Member<WindowProxy> m_windowProxy; IsolatedWorldMap m_isolatedWorlds; }; template <typename FrameType, typename ProxyType> -class WindowProxyManagerImplHelper : public WindowProxyManagerBase { +class WindowProxyManagerImplHelper : public WindowProxyManager { private: - using Base = WindowProxyManagerBase; + using Base = WindowProxyManager; public: - FrameType* frame() const { return static_cast<FrameType*>(Base::frame()); } - ProxyType* mainWorldProxy() const { + ProxyType* mainWorldProxy() { return static_cast<ProxyType*>(Base::mainWorldProxy()); } ProxyType* windowProxy(DOMWrapperWorld& world) { @@ -71,8 +82,8 @@ } protected: - explicit WindowProxyManagerImplHelper(Frame& frame) - : WindowProxyManagerBase(frame) {} + WindowProxyManagerImplHelper(Frame& frame, FrameType frameType) + : WindowProxyManager(frame, frameType) {} }; class LocalWindowProxyManager @@ -82,16 +93,20 @@ return new LocalWindowProxyManager(frame); } + // TODO(yukishiino): Remove this method. + LocalWindowProxy* mainWorldProxyMaybeUninitialized() { + return static_cast<LocalWindowProxy*>(m_windowProxy.get()); + } + // Sets the given security origin to the main world's context. Also updates // the security origin of the context for each isolated world. void updateSecurityOrigin(SecurityOrigin*); private: - // TODO(dcheng): Merge LocalWindowProxyManager and ScriptController? - friend class ScriptController; - explicit LocalWindowProxyManager(LocalFrame& frame) - : WindowProxyManagerImplHelper<LocalFrame, LocalWindowProxy>(frame) {} + : WindowProxyManagerImplHelper<LocalFrame, LocalWindowProxy>( + frame, + FrameType::Local) {} }; class RemoteWindowProxyManager @@ -103,7 +118,9 @@ private: explicit RemoteWindowProxyManager(RemoteFrame& frame) - : WindowProxyManagerImplHelper<RemoteFrame, RemoteWindowProxy>(frame) {} + : WindowProxyManagerImplHelper<RemoteFrame, RemoteWindowProxy>( + frame, + FrameType::Remote) {} }; } // namespace blink
diff --git a/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py b/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py index 477de5c..9c44520 100644 --- a/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py +++ b/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py
@@ -72,6 +72,10 @@ def find_base_type(current_type): if current_type.is_array_or_sequence_type: return find_base_type(current_type.element_type) + if current_type.is_record_type: + # IdlRecordType.key_type is always a string type, so we only need + # to looking into value_type. + return find_base_type(current_type.value_type) if current_type.is_nullable: return find_base_type(current_type.inner_type) return current_type
diff --git a/third_party/WebKit/Source/bindings/scripts/idl_types.py b/third_party/WebKit/Source/bindings/scripts/idl_types.py index 104e020..9ba59ed 100644 --- a/third_party/WebKit/Source/bindings/scripts/idl_types.py +++ b/third_party/WebKit/Source/bindings/scripts/idl_types.py
@@ -345,7 +345,7 @@ def single_matching_member_type(self, predicate): matching_types = filter(predicate, self.flattened_member_types) if len(matching_types) > 1: - raise "%s is ambigious." % self.name + raise ValueError('%s is ambiguous.' % self.name) return matching_types[0] if matching_types else None @property
diff --git a/third_party/WebKit/Source/bindings/scripts/v8_types.py b/third_party/WebKit/Source/bindings/scripts/v8_types.py index dcabd8a..6e1cb2c 100644 --- a/third_party/WebKit/Source/bindings/scripts/v8_types.py +++ b/third_party/WebKit/Source/bindings/scripts/v8_types.py
@@ -580,7 +580,7 @@ if 'FlexibleArrayBufferView' in extended_attributes: if base_idl_type not in TYPED_ARRAY_TYPES.union(set(['ArrayBufferView'])): - raise "Unrecognized base type for extended attribute 'FlexibleArrayBufferView': %s" % (idl_type.base_type) + raise ValueError("Unrecognized base type for extended attribute 'FlexibleArrayBufferView': %s" % (idl_type.base_type)) base_idl_type = 'FlexibleArrayBufferView' if idl_type.is_integer_type: @@ -689,7 +689,7 @@ } elif 'FlexibleArrayBufferView' in extended_attributes: if idl_type.base_type not in TYPED_ARRAY_TYPES.union(set(['ArrayBufferView'])): - raise "Unrecognized base type for extended attribute 'FlexibleArrayBufferView': %s" % (idl_type.base_type) + raise ValueError("Unrecognized base type for extended attribute 'FlexibleArrayBufferView': %s" % (idl_type.base_type)) set_expression = cpp_value else: assign_expression = cpp_value
diff --git a/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl b/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl index ef58931..73de7576f 100644 --- a/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl +++ b/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl
@@ -39,5 +39,6 @@ [PrefixGet] object prefixGetMember; record<ByteString, byte> recordMember; record<USVString, TestObject> garbageCollectedRecordMember; + record<ByteString, (long or boolean)> unionInRecordMember; (Float or BooleanType) unionWithTypedefs; };
diff --git a/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp b/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp index e8357a8..6983701 100644 --- a/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp +++ b/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp
@@ -395,6 +395,17 @@ void TestDictionary::setUint8ArrayMember(DOMUint8Array* value) { m_uint8ArrayMember = value; } +bool TestDictionary::hasUnionInRecordMember() const { + return m_hasUnionInRecordMember; +} +const HeapVector<std::pair<String, LongOrBoolean>>& TestDictionary::unionInRecordMember() const { + DCHECK(m_hasUnionInRecordMember); + return m_unionInRecordMember; +} +void TestDictionary::setUnionInRecordMember(const HeapVector<std::pair<String, LongOrBoolean>>& value) { + m_unionInRecordMember = value; + m_hasUnionInRecordMember = true; +} bool TestDictionary::hasUnionWithTypedefs() const { return !m_unionWithTypedefs.isNull(); } @@ -433,6 +444,7 @@ visitor->trace(m_testInterfaceSequenceMember); visitor->trace(m_testObjectSequenceMember); visitor->trace(m_uint8ArrayMember); + visitor->trace(m_unionInRecordMember); visitor->trace(m_unionWithTypedefs); IDLDictionaryBase::trace(visitor); }
diff --git a/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h b/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h index e88e977..727d6c48 100644 --- a/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h +++ b/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h
@@ -16,6 +16,7 @@ #include "bindings/core/v8/DoubleOrString.h" #include "bindings/core/v8/FloatOrBoolean.h" #include "bindings/core/v8/IDLDictionaryBase.h" +#include "bindings/core/v8/LongOrBoolean.h" #include "bindings/core/v8/ScriptValue.h" #include "bindings/core/v8/TestInterface2OrUint8Array.h" #include "bindings/tests/idls/core/TestInterface2.h" @@ -184,6 +185,10 @@ DOMUint8Array* uint8ArrayMember() const; void setUint8ArrayMember(DOMUint8Array*); + bool hasUnionInRecordMember() const; + const HeapVector<std::pair<String, LongOrBoolean>>& unionInRecordMember() const; + void setUnionInRecordMember(const HeapVector<std::pair<String, LongOrBoolean>>&); + bool hasUnionWithTypedefs() const; const FloatOrBoolean& unionWithTypedefs() const; void setUnionWithTypedefs(const FloatOrBoolean&); @@ -246,6 +251,8 @@ bool m_hasTestObjectSequenceMember = false; HeapVector<Member<TestObject>> m_testObjectSequenceMember; Member<DOMUint8Array> m_uint8ArrayMember; + bool m_hasUnionInRecordMember = false; + HeapVector<std::pair<String, LongOrBoolean>> m_unionInRecordMember; FloatOrBoolean m_unionWithTypedefs; bool m_hasUnrestrictedDoubleMember = false; double m_unrestrictedDoubleMember;
diff --git a/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp b/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp index 20a79e9..d3a465a 100644 --- a/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp +++ b/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp
@@ -592,6 +592,20 @@ impl.setUint8ArrayMember(uint8ArrayMember); } + v8::Local<v8::Value> unionInRecordMemberValue; + if (!v8Object->Get(isolate->GetCurrentContext(), v8AtomicString(isolate, "unionInRecordMember")).ToLocal(&unionInRecordMemberValue)) { + exceptionState.rethrowV8Exception(block.Exception()); + return; + } + if (unionInRecordMemberValue.IsEmpty() || unionInRecordMemberValue->IsUndefined()) { + // Do nothing. + } else { + HeapVector<std::pair<String, LongOrBoolean>> unionInRecordMember = NativeValueTraits<IDLRecord<IDLByteString, LongOrBoolean>>::nativeValue(isolate, unionInRecordMemberValue, exceptionState); + if (exceptionState.hadException()) + return; + impl.setUnionInRecordMember(unionInRecordMember); + } + v8::Local<v8::Value> unionWithTypedefsValue; if (!v8Object->Get(isolate->GetCurrentContext(), v8AtomicString(isolate, "unionWithTypedefs")).ToLocal(&unionWithTypedefsValue)) { exceptionState.rethrowV8Exception(block.Exception()); @@ -854,6 +868,11 @@ return false; } + if (impl.hasUnionInRecordMember()) { + if (!v8CallBoolean(dictionary->CreateDataProperty(isolate->GetCurrentContext(), v8AtomicString(isolate, "unionInRecordMember"), ToV8(impl.unionInRecordMember(), creationContext, isolate)))) + return false; + } + if (impl.hasUnionWithTypedefs()) { if (!v8CallBoolean(dictionary->CreateDataProperty(isolate->GetCurrentContext(), v8AtomicString(isolate, "unionWithTypedefs"), ToV8(impl.unionWithTypedefs(), creationContext, isolate)))) return false;
diff --git a/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp b/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp index 8e498b52..f2a05a3 100644 --- a/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp
@@ -134,6 +134,15 @@ direction)) return found; + // Slotted nodes are already handled in traverseSiblingsForV1HostChild() + // above, here is for fallback contents. + Element* parent = node.parentElement(); + if (parent && isHTMLSlotElement(parent)) { + HTMLSlotElement& slot = toHTMLSlotElement(*parent); + if (slot.supportsDistribution() && slot.assignedNodes().isEmpty()) + return traverseSiblings(slot, direction); + } + if (!node.isInV0ShadowTree()) return nullptr;
diff --git a/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp b/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp index d87fd3ab..6b8c652 100644 --- a/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp +++ b/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversalTest.cpp
@@ -594,4 +594,104 @@ EXPECT_EQ(parent, FlatTreeTraversal::parent(*slot)); } +TEST_F(FlatTreeTraversalTest, v1FallbackContent) { + const char* mainHTML = "<div id='d1'></div>"; + const char* shadowHTML = + "<div id='before'></div>" + "<slot><p>fallback content</p></slot>" + "<div id='after'></div>"; + + setupDocumentTree(mainHTML); + + Element* body = document().body(); + Element* d1 = body->querySelector("#d1"); + + attachOpenShadowRoot(*d1, shadowHTML); + ShadowRoot* shadowRoot = d1->openShadowRoot(); + Element* before = shadowRoot->querySelector("#before"); + Element* after = shadowRoot->querySelector("#after"); + Element* fallbackContent = shadowRoot->querySelector("p"); + + EXPECT_EQ(before, FlatTreeTraversal::firstChild(*d1)); + EXPECT_EQ(after, FlatTreeTraversal::lastChild(*d1)); + EXPECT_EQ(d1, FlatTreeTraversal::parent(*fallbackContent)); + + EXPECT_EQ(fallbackContent, FlatTreeTraversal::nextSibling(*before)); + EXPECT_EQ(after, FlatTreeTraversal::nextSibling(*fallbackContent)); + EXPECT_EQ(nullptr, FlatTreeTraversal::nextSibling(*after)); + + EXPECT_EQ(fallbackContent, FlatTreeTraversal::previousSibling(*after)); + EXPECT_EQ(before, FlatTreeTraversal::previousSibling(*fallbackContent)); + EXPECT_EQ(nullptr, FlatTreeTraversal::previousSibling(*before)); +} + +TEST_F(FlatTreeTraversalTest, v1FallbackContentSkippedInTraversal) { + const char* mainHTML = "<div id='d1'><span></span></div>"; + const char* shadowHTML = + "<div id='before'></div>" + "<slot><p>fallback content</p></slot>" + "<div id='after'></div>"; + + setupDocumentTree(mainHTML); + + Element* body = document().body(); + Element* d1 = body->querySelector("#d1"); + Element* span = body->querySelector("span"); + + attachOpenShadowRoot(*d1, shadowHTML); + ShadowRoot* shadowRoot = d1->openShadowRoot(); + Element* before = shadowRoot->querySelector("#before"); + Element* after = shadowRoot->querySelector("#after"); + Element* fallbackContent = shadowRoot->querySelector("p"); + + EXPECT_EQ(before, FlatTreeTraversal::firstChild(*d1)); + EXPECT_EQ(after, FlatTreeTraversal::lastChild(*d1)); + EXPECT_EQ(d1, FlatTreeTraversal::parent(*span)); + + EXPECT_EQ(span, FlatTreeTraversal::nextSibling(*before)); + EXPECT_EQ(after, FlatTreeTraversal::nextSibling(*span)); + EXPECT_EQ(nullptr, FlatTreeTraversal::nextSibling(*after)); + + EXPECT_EQ(span, FlatTreeTraversal::previousSibling(*after)); + EXPECT_EQ(before, FlatTreeTraversal::previousSibling(*span)); + EXPECT_EQ(nullptr, FlatTreeTraversal::previousSibling(*before)); + + EXPECT_EQ(nullptr, FlatTreeTraversal::parent(*fallbackContent)); + EXPECT_EQ(nullptr, FlatTreeTraversal::nextSibling(*fallbackContent)); + EXPECT_EQ(nullptr, FlatTreeTraversal::previousSibling(*fallbackContent)); +} + +TEST_F(FlatTreeTraversalTest, v1AllFallbackContent) { + const char* mainHTML = "<div id='d1'></div>"; + const char* shadowHTML = + "<slot name='a'><p id='x'>fallback content X</p></slot>" + "<slot name='b'><p id='y'>fallback content Y</p></slot>" + "<slot name='c'><p id='z'>fallback content Z</p></slot>"; + + setupDocumentTree(mainHTML); + + Element* body = document().body(); + Element* d1 = body->querySelector("#d1"); + + attachOpenShadowRoot(*d1, shadowHTML); + ShadowRoot* shadowRoot = d1->openShadowRoot(); + Element* fallbackX = shadowRoot->querySelector("#x"); + Element* fallbackY = shadowRoot->querySelector("#y"); + Element* fallbackZ = shadowRoot->querySelector("#z"); + + EXPECT_EQ(fallbackX, FlatTreeTraversal::firstChild(*d1)); + EXPECT_EQ(fallbackZ, FlatTreeTraversal::lastChild(*d1)); + EXPECT_EQ(d1, FlatTreeTraversal::parent(*fallbackX)); + EXPECT_EQ(d1, FlatTreeTraversal::parent(*fallbackY)); + EXPECT_EQ(d1, FlatTreeTraversal::parent(*fallbackZ)); + + EXPECT_EQ(fallbackY, FlatTreeTraversal::nextSibling(*fallbackX)); + EXPECT_EQ(fallbackZ, FlatTreeTraversal::nextSibling(*fallbackY)); + EXPECT_EQ(nullptr, FlatTreeTraversal::nextSibling(*fallbackZ)); + + EXPECT_EQ(fallbackY, FlatTreeTraversal::previousSibling(*fallbackZ)); + EXPECT_EQ(fallbackX, FlatTreeTraversal::previousSibling(*fallbackY)); + EXPECT_EQ(nullptr, FlatTreeTraversal::previousSibling(*fallbackX)); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp index 53f009d..736d889 100644 --- a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp +++ b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
@@ -1507,11 +1507,7 @@ } bool isDisplayInsideTable(const Node* node) { - if (!node || !node->isElementNode()) - return false; - - LayoutObject* layoutObject = node->layoutObject(); - return (layoutObject && layoutObject->isTable()); + return node && node->layoutObject() && isHTMLTableElement(node); } bool isTableCell(const Node* node) {
diff --git a/third_party/WebKit/Source/core/editing/EditingUtilities.h b/third_party/WebKit/Source/core/editing/EditingUtilities.h index 6d5a149..b4505d04 100644 --- a/third_party/WebKit/Source/core/editing/EditingUtilities.h +++ b/third_party/WebKit/Source/core/editing/EditingUtilities.h
@@ -174,6 +174,8 @@ bool isTabHTMLSpanElement(const Node*); bool isTabHTMLSpanElementTextNode(const Node*); bool isMailHTMLBlockquoteElement(const Node*); +// Returns true if the specified node is visible <table>. We don't want to add +// invalid nodes to <table> elements. bool isDisplayInsideTable(const Node*); bool isInline(const Node*); bool isTableCell(const Node*);
diff --git a/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp index 3b826f4..46272509 100644 --- a/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp +++ b/third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
@@ -147,6 +147,13 @@ return 0; } +bool isRenderedAsTable(const Node* node) { + if (!node || !node->isElementNode()) + return false; + LayoutObject* layoutObject = node->layoutObject(); + return layoutObject && layoutObject->isTable(); +} + } // namespace template <typename Strategy> @@ -1037,8 +1044,7 @@ // container) or as we hit it (if it's atomic). template <typename Strategy> bool TextIteratorAlgorithm<Strategy>::shouldRepresentNodeOffsetZero() { - if (emitsCharactersBetweenAllVisiblePositions() && - isDisplayInsideTable(m_node)) + if (emitsCharactersBetweenAllVisiblePositions() && isRenderedAsTable(m_node)) return true; // Leave element positioned flush with start of a paragraph @@ -1107,7 +1113,7 @@ template <typename Strategy> bool TextIteratorAlgorithm<Strategy>::shouldEmitSpaceBeforeAndAfterNode( Node* node) { - return isDisplayInsideTable(node) && + return isRenderedAsTable(node) && (node->layoutObject()->isInline() || emitsCharactersBetweenAllVisiblePositions()); }
diff --git a/third_party/WebKit/Source/core/frame/Frame.cpp b/third_party/WebKit/Source/core/frame/Frame.cpp index c2858eb..134ddc6c 100644 --- a/third_party/WebKit/Source/core/frame/Frame.cpp +++ b/third_party/WebKit/Source/core/frame/Frame.cpp
@@ -30,6 +30,7 @@ #include "core/frame/Frame.h" +#include "bindings/core/v8/WindowProxyManager.h" #include "core/dom/DocumentType.h" #include "core/events/Event.h" #include "core/frame/FrameHost.h" @@ -64,6 +65,7 @@ visitor->trace(m_treeNode); visitor->trace(m_host); visitor->trace(m_owner); + visitor->trace(m_windowProxyManager); visitor->trace(m_domWindow); visitor->trace(m_client); } @@ -387,6 +389,10 @@ return nullptr; } +WindowProxy* Frame::windowProxy(DOMWrapperWorld& world) { + return m_windowProxyManager->windowProxy(world); +} + void Frame::didChangeVisibilityState() { HeapVector<Member<Frame>> childFrames; for (Frame* child = tree().firstChild(); child; @@ -412,11 +418,15 @@ return featurePolicy->IsFeatureEnabled(feature); } -Frame::Frame(FrameClient* client, FrameHost* host, FrameOwner* owner) +Frame::Frame(FrameClient* client, + FrameHost* host, + FrameOwner* owner, + WindowProxyManager* windowProxyManager) : m_treeNode(this), m_host(host), m_owner(owner), m_client(client), + m_windowProxyManager(windowProxyManager), m_isLoading(false) { InstanceCounters::incrementCounter(InstanceCounters::FrameCounter);
diff --git a/third_party/WebKit/Source/core/frame/Frame.h b/third_party/WebKit/Source/core/frame/Frame.h index 3b5501c..ee53c61 100644 --- a/third_party/WebKit/Source/core/frame/Frame.h +++ b/third_party/WebKit/Source/core/frame/Frame.h
@@ -54,7 +54,7 @@ class SecurityContext; class Settings; class WindowProxy; -class WindowProxyManagerBase; +class WindowProxyManager; struct FrameLoadRequest; enum class FrameDetachType { Remove, Swap }; @@ -74,8 +74,6 @@ virtual bool isLocalFrame() const = 0; virtual bool isRemoteFrame() const = 0; - virtual WindowProxy* windowProxy(DOMWrapperWorld&) = 0; - virtual void navigate(Document& originDocument, const KURL&, bool replaceCurrentItem, @@ -141,7 +139,10 @@ void setIsLoading(bool isLoading) { m_isLoading = isLoading; } bool isLoading() const { return m_isLoading; } - virtual WindowProxyManagerBase* getWindowProxyManager() const = 0; + WindowProxyManager* getWindowProxyManager() const { + return m_windowProxyManager; + } + WindowProxy* windowProxy(DOMWrapperWorld&); virtual void didChangeVisibilityState(); @@ -155,7 +156,7 @@ bool isFeatureEnabled(WebFeaturePolicyFeature) const; protected: - Frame(FrameClient*, FrameHost*, FrameOwner*); + Frame(FrameClient*, FrameHost*, FrameOwner*, WindowProxyManager*); mutable FrameTree m_treeNode; @@ -170,6 +171,7 @@ bool canNavigateWithoutFramebusting(const Frame&, String& errorReason); Member<FrameClient> m_client; + const Member<WindowProxyManager> m_windowProxyManager; bool m_isLoading; };
diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/third_party/WebKit/Source/core/frame/LocalFrame.cpp index b107838..d8be255b 100644 --- a/third_party/WebKit/Source/core/frame/LocalFrame.cpp +++ b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
@@ -361,10 +361,6 @@ Supplementable<LocalFrame>::trace(visitor); } -WindowProxy* LocalFrame::windowProxy(DOMWrapperWorld& world) { - return m_script->windowProxy(world); -} - void LocalFrame::navigate(Document& originDocument, const KURL& url, bool replaceCurrentItem, @@ -493,10 +489,6 @@ ConsoleMessage::create(JSMessageSource, WarningMessageLevel, message)); } -WindowProxyManagerBase* LocalFrame::getWindowProxyManager() const { - return m_script->getWindowProxyManager(); -} - bool LocalFrame::shouldClose() { // TODO(dcheng): This should be fixed to dispatch beforeunload events to // both local and remote frames. @@ -855,12 +847,14 @@ FrameOwner* owner, InterfaceProvider* interfaceProvider, InterfaceRegistry* interfaceRegistry) - : Frame(client, host, owner), + : Frame(client, host, owner, LocalWindowProxyManager::create(*this)), m_frameScheduler(page()->chromeClient().createFrameScheduler( client->frameBlameContext())), m_loader(this), m_navigationScheduler(NavigationScheduler::create(this)), - m_script(ScriptController::create(this)), + m_script(ScriptController::create( + *this, + *static_cast<LocalWindowProxyManager*>(getWindowProxyManager()))), m_editor(Editor::create(*this)), m_spellChecker(SpellChecker::create(*this)), m_selection(FrameSelection::create(*this)),
diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.h b/third_party/WebKit/Source/core/frame/LocalFrame.h index da489be..985d1e95 100644 --- a/third_party/WebKit/Source/core/frame/LocalFrame.h +++ b/third_party/WebKit/Source/core/frame/LocalFrame.h
@@ -107,7 +107,6 @@ // Frame overrides: ~LocalFrame() override; DECLARE_VIRTUAL_TRACE(); - WindowProxy* windowProxy(DOMWrapperWorld&) override; void navigate(Document& originDocument, const KURL&, bool replaceCurrentItem, @@ -232,8 +231,6 @@ InterfaceProvider*, InterfaceRegistry*); - // Internal Frame helper overrides: - WindowProxyManagerBase* getWindowProxyManager() const override; // Intentionally private to prevent redundant checks when the type is // already LocalFrame. bool isLocalFrame() const override { return true; }
diff --git a/third_party/WebKit/Source/core/frame/RemoteFrame.cpp b/third_party/WebKit/Source/core/frame/RemoteFrame.cpp index d04e312..7eaddcd2 100644 --- a/third_party/WebKit/Source/core/frame/RemoteFrame.cpp +++ b/third_party/WebKit/Source/core/frame/RemoteFrame.cpp
@@ -28,9 +28,8 @@ inline RemoteFrame::RemoteFrame(RemoteFrameClient* client, FrameHost* host, FrameOwner* owner) - : Frame(client, host, owner), - m_securityContext(RemoteSecurityContext::create()), - m_windowProxyManager(RemoteWindowProxyManager::create(*this)) { + : Frame(client, host, owner, RemoteWindowProxyManager::create(*this)), + m_securityContext(RemoteSecurityContext::create()) { m_domWindow = RemoteDOMWindow::create(*this); } @@ -47,17 +46,9 @@ DEFINE_TRACE(RemoteFrame) { visitor->trace(m_view); visitor->trace(m_securityContext); - visitor->trace(m_windowProxyManager); Frame::trace(visitor); } -WindowProxy* RemoteFrame::windowProxy(DOMWrapperWorld& world) { - WindowProxy* windowProxy = m_windowProxyManager->windowProxy(world); - ASSERT(windowProxy); - windowProxy->initializeIfNeeded(); - return windowProxy; -} - void RemoteFrame::navigate(Document& originDocument, const KURL& url, bool replaceCurrentItem, @@ -101,7 +92,7 @@ if (m_view) m_view->dispose(); client()->willBeDetached(); - m_windowProxyManager->clearForClose(); + getWindowProxyManager()->clearForClose(); setView(nullptr); // ... the RemoteDOMWindow will need to be informed of detachment, // as otherwise it will keep a strong reference back to this RemoteFrame. @@ -172,10 +163,6 @@ client()->advanceFocus(type, source); } -WindowProxyManagerBase* RemoteFrame::getWindowProxyManager() const { - return m_windowProxyManager.get(); -} - void RemoteFrame::detachChildren() { using FrameVector = HeapVector<Member<Frame>>; FrameVector childrenToDetach;
diff --git a/third_party/WebKit/Source/core/frame/RemoteFrame.h b/third_party/WebKit/Source/core/frame/RemoteFrame.h index 4c108d0..27ec118 100644 --- a/third_party/WebKit/Source/core/frame/RemoteFrame.h +++ b/third_party/WebKit/Source/core/frame/RemoteFrame.h
@@ -16,7 +16,6 @@ class LocalFrame; class RemoteFrameClient; class RemoteFrameView; -class RemoteWindowProxyManager; class WebLayer; struct FrameLoadRequest; @@ -28,7 +27,6 @@ // Frame overrides: DECLARE_VIRTUAL_TRACE(); - WindowProxy* windowProxy(DOMWrapperWorld&) override; void navigate(Document& originDocument, const KURL&, bool replaceCurrentItem, @@ -61,9 +59,6 @@ private: RemoteFrame(RemoteFrameClient*, FrameHost*, FrameOwner*); - // Internal Frame helper overrides: - WindowProxyManagerBase* getWindowProxyManager() const override; - // Intentionally private to prevent redundant checks when the type is // already RemoteFrame. bool isLocalFrame() const override { return false; } @@ -73,7 +68,6 @@ Member<RemoteFrameView> m_view; Member<RemoteSecurityContext> m_securityContext; - Member<RemoteWindowProxyManager> m_windowProxyManager; WebLayer* m_webLayer = nullptr; };
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp index 1145a798d..e2a0d5b 100644 --- a/third_party/WebKit/Source/core/input/EventHandler.cpp +++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -726,7 +726,7 @@ mouseEvent.button == WebPointerProperties::Button::Left) { DCHECK_EQ(WebInputEvent::MouseDown, mouseEvent.type()); HitTestResult result = mev.hitTestResult(); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); m_frame->chromeClient().onMouseDown(result.innerNode()); } @@ -756,7 +756,7 @@ if (FrameView* frameView = m_frame->view()) frameView->mouseMovedInContentArea(); - hoveredNode.setToShadowHostIfInUserAgentShadowRoot(); + hoveredNode.setToShadowHostIfInRestrictedShadowRoot(); page->chromeClient().mouseDidMoveOverElement(*m_frame, hoveredNode); return result;
diff --git a/third_party/WebKit/Source/core/input/GestureManager.cpp b/third_party/WebKit/Source/core/input/GestureManager.cpp index 92a5227..4b4617d 100644 --- a/third_party/WebKit/Source/core/input/GestureManager.cpp +++ b/third_party/WebKit/Source/core/input/GestureManager.cpp
@@ -230,7 +230,7 @@ if (currentHitTest.innerNode()) { DCHECK(gestureEvent.type() == WebInputEvent::GestureTap); HitTestResult result = currentHitTest; - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); m_frame->chromeClient().onMouseDown(result.innerNode()); }
diff --git a/third_party/WebKit/Source/core/layout/HitTestResult.cpp b/third_party/WebKit/Source/core/layout/HitTestResult.cpp index 54e7376..f317553 100644 --- a/third_party/WebKit/Source/core/layout/HitTestResult.cpp +++ b/third_party/WebKit/Source/core/layout/HitTestResult.cpp
@@ -169,7 +169,7 @@ return m_innerNode ? m_innerNode->layoutObject() : 0; } -void HitTestResult::setToShadowHostIfInUserAgentShadowRoot() { +void HitTestResult::setToShadowHostIfInRestrictedShadowRoot() { Node* node = innerNode(); if (!node) return; @@ -177,10 +177,14 @@ ShadowRoot* containingShadowRoot = node->containingShadowRoot(); Element* shadowHost = nullptr; + // Consider a closed shadow tree of SVG's <use> element as a special + // case so that a toolip title in the shadow tree works. while (containingShadowRoot && - containingShadowRoot->type() == ShadowRootType::UserAgent) { + (containingShadowRoot->type() == ShadowRootType::UserAgent || + isSVGUseElement(containingShadowRoot->host()))) { shadowHost = &containingShadowRoot->host(); containingShadowRoot = shadowHost->containingShadowRoot(); + setInnerNode(node->ownerShadowHost()); } if (shadowHost)
diff --git a/third_party/WebKit/Source/core/layout/HitTestResult.h b/third_party/WebKit/Source/core/layout/HitTestResult.h index afda9c27..fd1cef67 100644 --- a/third_party/WebKit/Source/core/layout/HitTestResult.h +++ b/third_party/WebKit/Source/core/layout/HitTestResult.h
@@ -131,7 +131,7 @@ PositionWithAffinity position() const; LayoutObject* layoutObject() const; - void setToShadowHostIfInUserAgentShadowRoot(); + void setToShadowHostIfInRestrictedShadowRoot(); const HitTestLocation& hitTestLocation() const { return m_hitTestLocation; } const HitTestRequest& hitTestRequest() const { return m_hitTestRequest; }
diff --git a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp index 34ab1db..6c203a1 100644 --- a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp +++ b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
@@ -19,11 +19,6 @@ // Meaningful Paint. const int kBlankCharactersThreshold = 200; -// The page is n-quiet if there are no more than n active network requests for -// this duration of time. -const double kNetwork2QuietWindowSeconds = 3; -const double kNetwork0QuietWindowSeconds = 0.5; - } // namespace FirstMeaningfulPaintDetector& FirstMeaningfulPaintDetector::from(
diff --git a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h index c04d9da..d9733eb 100644 --- a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h +++ b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h
@@ -53,6 +53,11 @@ private: friend class FirstMeaningfulPaintDetectorTest; + // The page is n-quiet if there are no more than n active network requests for + // this duration of time. + static constexpr double kNetwork2QuietWindowSeconds = 3; + static constexpr double kNetwork0QuietWindowSeconds = 0.5; + Document* document(); int activeConnections(); void setNetworkQuietTimers(int activeConnections);
diff --git a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp index 63527ab..6d0136cd 100644 --- a/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp +++ b/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
@@ -6,7 +6,7 @@ #include "core/paint/PaintTiming.h" #include "core/testing/DummyPageHolder.h" -#include "platform/scheduler/test/fake_web_task_runner.h" +#include "platform/testing/TestingPlatformSupport.h" #include "testing/gtest/include/gtest/gtest.h" #include "wtf/text/StringBuilder.h" @@ -15,16 +15,13 @@ class FirstMeaningfulPaintDetectorTest : public testing::Test { protected: void SetUp() override { + m_platform->advanceClockSeconds(1); m_dummyPageHolder = DummyPageHolder::create(IntSize(800, 600)); - s_timeElapsed = 0.0; - m_originalTimeFunction = setTimeFunctionsForTesting(returnMockTime); - m_taskRunner = adoptRef(new scheduler::FakeWebTaskRunner); - detector().m_network0QuietTimer.moveToNewTaskRunner(m_taskRunner); - detector().m_network2QuietTimer.moveToNewTaskRunner(m_taskRunner); } - void TearDown() override { - setTimeFunctionsForTesting(m_originalTimeFunction); + double advanceClockAndGetTime() { + m_platform->advanceClockSeconds(1); + return monotonicallyIncreasingTime(); } Document& document() { return m_dummyPageHolder->document(); } @@ -34,6 +31,7 @@ } void simulateLayoutAndPaint(int newElements) { + m_platform->advanceClockSeconds(0.001); StringBuilder builder; for (int i = 0; i < newElements; i++) builder.append("<span>a</span>"); @@ -59,22 +57,7 @@ } void setActiveConnections(int connections) { - double time0 = 0.0; - double time2 = 0.0; - m_taskRunner->setTime(returnMockTime()); - if (isNetwork0QuietTimerActive()) - time0 = detector().m_network0QuietTimer.nextFireInterval(); - if (isNetwork2QuietTimerActive()) - time2 = detector().m_network2QuietTimer.nextFireInterval(); - detector().setNetworkQuietTimers(connections); - - m_0QuietTimerRestarted = - isNetwork0QuietTimerActive() && - detector().m_network0QuietTimer.nextFireInterval() != time0; - m_2QuietTimerRestarted = - isNetwork2QuietTimerActive() && - detector().m_network2QuietTimer.nextFireInterval() != time2; } bool isNetwork0QuietTimerActive() { @@ -85,25 +68,22 @@ return detector().m_network2QuietTimer.isActive(); } - bool isNetwork0QuietTimerRestarted() { return m_0QuietTimerRestarted; } - bool isNetwork2QuietTimerRestarted() { return m_2QuietTimerRestarted; } + bool hadNetwork0Quiet() { return detector().m_network0QuietReached; } + bool hadNetwork2Quiet() { return detector().m_network2QuietReached; } + + protected: + ScopedTestingPlatformSupport<TestingPlatformSupportWithMockScheduler> + m_platform; + + static constexpr double kNetwork0QuietWindowSeconds = + FirstMeaningfulPaintDetector::kNetwork0QuietWindowSeconds; + static constexpr double kNetwork2QuietWindowSeconds = + FirstMeaningfulPaintDetector::kNetwork2QuietWindowSeconds; private: - static double returnMockTime() { - s_timeElapsed += 1.0; - return s_timeElapsed; - } - std::unique_ptr<DummyPageHolder> m_dummyPageHolder; - RefPtr<scheduler::FakeWebTaskRunner> m_taskRunner; - TimeFunction m_originalTimeFunction; - bool m_0QuietTimerRestarted; - bool m_2QuietTimerRestarted; - static double s_timeElapsed; }; -double FirstMeaningfulPaintDetectorTest::s_timeElapsed; - TEST_F(FirstMeaningfulPaintDetectorTest, NoFirstPaint) { simulateLayoutAndPaint(1); simulateNetworkStable(); @@ -113,7 +93,7 @@ TEST_F(FirstMeaningfulPaintDetectorTest, OneLayout) { paintTiming().markFirstContentfulPaint(); simulateLayoutAndPaint(1); - double afterPaint = monotonicallyIncreasingTime(); + double afterPaint = advanceClockAndGetTime(); EXPECT_EQ(paintTiming().firstMeaningfulPaint(), 0.0); simulateNetworkStable(); EXPECT_GT(paintTiming().firstMeaningfulPaint(), paintTiming().firstPaint()); @@ -123,9 +103,9 @@ TEST_F(FirstMeaningfulPaintDetectorTest, TwoLayoutsSignificantSecond) { paintTiming().markFirstContentfulPaint(); simulateLayoutAndPaint(1); - double afterLayout1 = monotonicallyIncreasingTime(); + double afterLayout1 = advanceClockAndGetTime(); simulateLayoutAndPaint(10); - double afterLayout2 = monotonicallyIncreasingTime(); + double afterLayout2 = advanceClockAndGetTime(); simulateNetworkStable(); EXPECT_GT(paintTiming().firstMeaningfulPaint(), afterLayout1); EXPECT_LT(paintTiming().firstMeaningfulPaint(), afterLayout2); @@ -134,7 +114,7 @@ TEST_F(FirstMeaningfulPaintDetectorTest, TwoLayoutsSignificantFirst) { paintTiming().markFirstContentfulPaint(); simulateLayoutAndPaint(10); - double afterLayout1 = monotonicallyIncreasingTime(); + double afterLayout1 = advanceClockAndGetTime(); simulateLayoutAndPaint(1); simulateNetworkStable(); EXPECT_GT(paintTiming().firstMeaningfulPaint(), paintTiming().firstPaint()); @@ -145,7 +125,7 @@ paintTiming().markFirstContentfulPaint(); EXPECT_EQ(paintTiming().firstMeaningfulPaintCandidate(), 0.0); simulateLayoutAndPaint(1); - double afterPaint = monotonicallyIncreasingTime(); + double afterPaint = advanceClockAndGetTime(); // The first candidate gets ignored. EXPECT_EQ(paintTiming().firstMeaningfulPaintCandidate(), 0.0); simulateLayoutAndPaint(10); @@ -161,7 +141,7 @@ OnlyOneFirstMeaningfulPaintCandidateBeforeNetworkStable) { paintTiming().markFirstContentfulPaint(); EXPECT_EQ(paintTiming().firstMeaningfulPaintCandidate(), 0.0); - double beforePaint = monotonicallyIncreasingTime(); + double beforePaint = advanceClockAndGetTime(); simulateLayoutAndPaint(1); // The first candidate is initially ignored. EXPECT_EQ(paintTiming().firstMeaningfulPaintCandidate(), 0.0); @@ -199,7 +179,7 @@ paintTiming().markFirstContentfulPaint(); simulateLayoutAndPaint(1); - double afterFirstPaint = monotonicallyIncreasingTime(); + double afterFirstPaint = advanceClockAndGetTime(); simulateNetwork2Quiet(); simulateLayoutAndPaint(10); @@ -214,11 +194,11 @@ paintTiming().markFirstContentfulPaint(); simulateLayoutAndPaint(1); - double afterFirstPaint = monotonicallyIncreasingTime(); + double afterFirstPaint = advanceClockAndGetTime(); simulateNetwork0Quiet(); simulateLayoutAndPaint(10); - double afterSecondPaint = monotonicallyIncreasingTime(); + double afterSecondPaint = advanceClockAndGetTime(); simulateNetwork2Quiet(); // The second paint is FirstMeaningfulPaint. @@ -226,33 +206,45 @@ EXPECT_LT(paintTiming().firstMeaningfulPaint(), afterSecondPaint); } -TEST_F(FirstMeaningfulPaintDetectorTest, NetworkQuietTimers) { - setActiveConnections(3); - EXPECT_FALSE(isNetwork0QuietTimerActive()); - EXPECT_FALSE(isNetwork2QuietTimerActive()); - - setActiveConnections(2); - EXPECT_FALSE(isNetwork0QuietTimerActive()); - EXPECT_TRUE(isNetwork2QuietTimerActive()); +TEST_F(FirstMeaningfulPaintDetectorTest, Network0QuietTimer) { + paintTiming().markFirstContentfulPaint(); setActiveConnections(1); EXPECT_FALSE(isNetwork0QuietTimerActive()); - EXPECT_TRUE(isNetwork2QuietTimerActive()); - EXPECT_FALSE(isNetwork2QuietTimerRestarted()); + + setActiveConnections(0); + m_platform->runForPeriodSeconds(kNetwork0QuietWindowSeconds - 0.1); + EXPECT_TRUE(isNetwork0QuietTimerActive()); + EXPECT_FALSE(hadNetwork0Quiet()); + + setActiveConnections(0); // This should reset the 0-quiet timer. + m_platform->runForPeriodSeconds(kNetwork0QuietWindowSeconds - 0.1); + EXPECT_TRUE(isNetwork0QuietTimerActive()); + EXPECT_FALSE(hadNetwork0Quiet()); + + m_platform->runForPeriodSeconds(0.1001); + EXPECT_TRUE(hadNetwork0Quiet()); +} + +TEST_F(FirstMeaningfulPaintDetectorTest, Network2QuietTimer) { + paintTiming().markFirstContentfulPaint(); + + setActiveConnections(3); + EXPECT_FALSE(isNetwork2QuietTimerActive()); setActiveConnections(2); - EXPECT_TRUE(isNetwork2QuietTimerRestarted()); - - setActiveConnections(0); - EXPECT_TRUE(isNetwork0QuietTimerActive()); + m_platform->runForPeriodSeconds(kNetwork2QuietWindowSeconds - 0.1); EXPECT_TRUE(isNetwork2QuietTimerActive()); - EXPECT_FALSE(isNetwork2QuietTimerRestarted()); + EXPECT_FALSE(hadNetwork2Quiet()); - setActiveConnections(0); - EXPECT_TRUE(isNetwork0QuietTimerActive()); - EXPECT_TRUE(isNetwork0QuietTimerRestarted()); + setActiveConnections(2); // This should reset the 2-quiet timer. + m_platform->runForPeriodSeconds(kNetwork2QuietWindowSeconds - 0.1); EXPECT_TRUE(isNetwork2QuietTimerActive()); - EXPECT_FALSE(isNetwork2QuietTimerRestarted()); + EXPECT_FALSE(hadNetwork2Quiet()); + + setActiveConnections(1); // This should not reset the 2-quiet timer. + m_platform->runForPeriodSeconds(0.1001); + EXPECT_TRUE(hadNetwork2Quiet()); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/svg/SVGElement.cpp b/third_party/WebKit/Source/core/svg/SVGElement.cpp index 313e7bb..821591b 100644 --- a/third_party/WebKit/Source/core/svg/SVGElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGElement.cpp
@@ -650,8 +650,7 @@ SVGUseElement* SVGElement::correspondingUseElement() const { if (ShadowRoot* root = containingShadowRoot()) { - if (isSVGUseElement(root->host()) && - (root->type() == ShadowRootType::UserAgent)) + if (isSVGUseElement(root->host())) return &toSVGUseElement(root->host()); } return nullptr;
diff --git a/third_party/WebKit/Source/core/svg/SVGUseElement.cpp b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp index 95b2c3f..0c0d7c0 100644 --- a/third_party/WebKit/Source/core/svg/SVGUseElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
@@ -32,6 +32,7 @@ #include "core/dom/IdTargetObserver.h" #include "core/dom/StyleChangeReason.h" #include "core/dom/TaskRunnerHelper.h" +#include "core/dom/shadow/ElementShadow.h" #include "core/dom/shadow/ShadowRoot.h" #include "core/events/Event.h" #include "core/layout/svg/LayoutSVGTransformableContainer.h" @@ -80,7 +81,7 @@ SVGUseElement* SVGUseElement::create(Document& document) { // Always build a user agent #shadow-root for SVGUseElement. SVGUseElement* use = new SVGUseElement(document); - use->ensureUserAgentShadowRoot(); + use->ensureShadow().addShadowRoot(*use, ShadowRootType::Closed); return use; } @@ -313,7 +314,7 @@ if (inUseShadowTree()) return; // FIXME: We should try to optimize this, to at least allow partial reclones. - userAgentShadowRoot()->removeChildren(OmitSubtreeModifiedEvent); + useShadowRoot().removeChildren(OmitSubtreeModifiedEvent); clearResourceReference(); cancelShadowTreeRecreation(); if (!isConnected()) @@ -420,8 +421,8 @@ DCHECK(!m_targetElementInstance); DCHECK(!m_needsShadowTreeRecreation); - // <use> creates a "user agent" shadow root. Do not build the shadow/instance - // tree for <use> elements living in a user agent shadow tree because they + // <use> creates a closed shadow root. Do not build the shadow/instance + // tree for <use> elements living in a closed tree because they // will get expanded in a second pass -- see expandUseElementsInShadowTree(). if (inUseShadowTree()) return; @@ -435,8 +436,8 @@ // <symbol> yet. Element* instanceRoot = createInstanceTree(target); m_targetElementInstance = toSVGElement(instanceRoot); - ShadowRoot* shadowTreeRootElement = userAgentShadowRoot(); - shadowTreeRootElement->appendChild(instanceRoot); + ShadowRoot& shadowRoot = useShadowRoot(); + shadowRoot.appendChild(instanceRoot); addReferencesToFirstDegreeNestedUseElements(target); @@ -451,16 +452,15 @@ // Expand all <use> elements in the shadow tree. // Expand means: replace the actual <use> element by what it references. if (!expandUseElementsInShadowTree()) { - shadowTreeRootElement->removeChildren(OmitSubtreeModifiedEvent); + shadowRoot.removeChildren(OmitSubtreeModifiedEvent); clearResourceReference(); return; } // If the instance root was a <use>, it could have been replaced now, so // reset |m_targetElementInstance|. - m_targetElementInstance = - toSVGElementOrDie(shadowTreeRootElement->firstChild()); - DCHECK_EQ(m_targetElementInstance->parentNode(), shadowTreeRootElement); + m_targetElementInstance = toSVGElementOrDie(shadowRoot.firstChild()); + DCHECK_EQ(m_targetElementInstance->parentNode(), shadowRoot); // Update relative length information. updateRelativeLengthsInformation(); @@ -497,7 +497,7 @@ SVGGraphicsElement* SVGUseElement::visibleTargetGraphicsElementForClipping() const { - Node* n = userAgentShadowRoot()->firstChild(); + Node* n = useShadowRoot().firstChild(); if (!n || !n->isSVGElement()) return nullptr; @@ -585,8 +585,8 @@ // a <symbol> contains <use> tags, we'd miss them. So once we're done with // setting up the actual shadow tree (after the special case modification for // svg/symbol) we have to walk it completely and expand all <use> elements. - ShadowRoot* shadowRoot = userAgentShadowRoot(); - for (SVGUseElement* use = Traversal<SVGUseElement>::firstWithin(*shadowRoot); + ShadowRoot& shadowRoot = useShadowRoot(); + for (SVGUseElement* use = Traversal<SVGUseElement>::firstWithin(shadowRoot); use;) { DCHECK(!use->resourceIsStillLoading()); @@ -617,7 +617,7 @@ // Replace <use> with referenced content. use->parentNode()->replaceChild(cloneParent, use); - use = Traversal<SVGUseElement>::next(*replacingElement, shadowRoot); + use = Traversal<SVGUseElement>::next(*replacingElement, &shadowRoot); } return true; } @@ -718,7 +718,7 @@ bool SVGUseElement::instanceTreeIsLoading() const { for (const SVGUseElement& useElement : - Traversal<SVGUseElement>::descendantsOf(*userAgentShadowRoot())) { + Traversal<SVGUseElement>::descendantsOf(useShadowRoot())) { if (useElement.resourceIsStillLoading()) return true; }
diff --git a/third_party/WebKit/Source/core/svg/SVGUseElement.h b/third_party/WebKit/Source/core/svg/SVGUseElement.h index 0b67aa3..477e98b 100644 --- a/third_party/WebKit/Source/core/svg/SVGUseElement.h +++ b/third_party/WebKit/Source/core/svg/SVGUseElement.h
@@ -89,6 +89,11 @@ bool selfHasRelativeLengths() const override; + ShadowRoot& useShadowRoot() const { + CHECK(closedShadowRoot()); + return *closedShadowRoot(); + } + // Instance tree handling Element* resolveTargetElement(); void buildShadowAndInstanceTree(SVGElement& target);
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc b/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc index 3219540..9f6d225 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.cc
@@ -36,12 +36,12 @@ CPUTimeBudgetPool::CPUTimeBudgetPool( const char* name, - TaskQueueThrottler* task_queue_throttler, + BudgetPoolController* budget_pool_controller, base::TimeTicks now, base::Optional<base::TimeDelta> max_budget_level, base::Optional<base::TimeDelta> max_throttling_duration) : name_(name), - task_queue_throttler_(task_queue_throttler), + budget_pool_controller_(budget_pool_controller), max_budget_level_(max_budget_level), max_throttling_duration_(max_throttling_duration), last_checkpoint_(now), @@ -58,39 +58,23 @@ } void CPUTimeBudgetPool::AddQueue(base::TimeTicks now, TaskQueue* queue) { - std::pair<TaskQueueThrottler::TaskQueueMap::iterator, bool> insert_result = - task_queue_throttler_->queue_details_.insert( - std::make_pair(queue, TaskQueueThrottler::Metadata())); - TaskQueueThrottler::Metadata& metadata = insert_result.first->second; - DCHECK(!metadata.time_budget_pool); - metadata.time_budget_pool = this; - + budget_pool_controller_->AddQueueToBudgetPool(queue, this); associated_task_queues_.insert(queue); - if (!is_enabled_ || !task_queue_throttler_->IsThrottled(queue)) + if (!is_enabled_ || !budget_pool_controller_->IsThrottled(queue)) return; - queue->InsertFence(TaskQueue::InsertFencePosition::BEGINNING_OF_TIME); - - task_queue_throttler_->MaybeSchedulePumpQueue(FROM_HERE, now, queue, - GetNextAllowedRunTime()); + budget_pool_controller_->BlockQueue(now, queue); } void CPUTimeBudgetPool::RemoveQueue(base::TimeTicks now, TaskQueue* queue) { - auto find_it = task_queue_throttler_->queue_details_.find(queue); - DCHECK(find_it != task_queue_throttler_->queue_details_.end() && - find_it->second.time_budget_pool == this); - find_it->second.time_budget_pool = nullptr; - bool is_throttled = task_queue_throttler_->IsThrottled(queue); - - task_queue_throttler_->MaybeDeleteQueueMetadata(find_it); + budget_pool_controller_->RemoveQueueFromBudgetPool(queue, this); associated_task_queues_.erase(queue); - if (!is_enabled_ || !is_throttled) + if (!is_enabled_ || !budget_pool_controller_->IsThrottled(queue)) return; - task_queue_throttler_->MaybeSchedulePumpQueue(FROM_HERE, now, queue, - base::nullopt); + budget_pool_controller_->UnblockQueue(now, queue); } void CPUTimeBudgetPool::EnableThrottling(LazyNow* lazy_now) { @@ -98,8 +82,7 @@ return; is_enabled_ = true; - TRACE_EVENT0(task_queue_throttler_->tracing_category_, - "CPUTimeBudgetPool_EnableThrottling"); + TRACE_EVENT0("renderer.scheduler", "CPUTimeBudgetPool_EnableThrottling"); BlockThrottledQueues(lazy_now->Now()); } @@ -109,15 +92,13 @@ return; is_enabled_ = false; - TRACE_EVENT0(task_queue_throttler_->tracing_category_, - "CPUTimeBudgetPool_DisableThrottling"); + TRACE_EVENT0("renderer.scheduler", "CPUTimeBudgetPool_DisableThrottling"); for (TaskQueue* queue : associated_task_queues_) { - if (!task_queue_throttler_->IsThrottled(queue)) + if (!budget_pool_controller_->IsThrottled(queue)) continue; - task_queue_throttler_->MaybeSchedulePumpQueue(FROM_HERE, lazy_now->Now(), - queue, base::nullopt); + budget_pool_controller_->UnblockQueue(lazy_now->Now(), queue); } // TODO(altimin): We need to disable TimeBudgetQueues here or they will @@ -143,7 +124,7 @@ void CPUTimeBudgetPool::Close() { DCHECK_EQ(0u, associated_task_queues_.size()); - task_queue_throttler_->time_budget_pools_.erase(this); + budget_pool_controller_->UnregisterBudgetPool(this); } bool CPUTimeBudgetPool::HasEnoughBudgetToRun(base::TimeTicks now) { @@ -212,14 +193,8 @@ } void CPUTimeBudgetPool::BlockThrottledQueues(base::TimeTicks now) { - for (TaskQueue* queue : associated_task_queues_) { - if (!task_queue_throttler_->IsThrottled(queue)) - continue; - - queue->InsertFence(TaskQueue::InsertFencePosition::BEGINNING_OF_TIME); - task_queue_throttler_->MaybeSchedulePumpQueue(FROM_HERE, now, queue, - base::nullopt); - } + for (TaskQueue* queue : associated_task_queues_) + budget_pool_controller_->BlockQueue(now, queue); } void CPUTimeBudgetPool::EnforceBudgetLevelRestrictions() {
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h b/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h index e602b93a..1ece45aa 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h +++ b/third_party/WebKit/Source/platform/scheduler/renderer/budget_pool.h
@@ -24,7 +24,7 @@ namespace scheduler { class TaskQueue; -class TaskQueueThrottler; +class BudgetPoolController; // BudgetPool represents a group of task queues which share a limit // on a resource. This limit applies when task queues are already throttled @@ -61,6 +61,9 @@ virtual void RecordTaskRunTime(base::TimeTicks start_time, base::TimeTicks end_time) = 0; + // Block all associated queues and schedule them to run when appropriate. + virtual void BlockThrottledQueues(base::TimeTicks now) = 0; + // All queues should be removed before calling Close(). virtual void Close() = 0; @@ -79,6 +82,12 @@ // on total cpu time. class BLINK_PLATFORM_EXPORT CPUTimeBudgetPool : public BudgetPool { public: + CPUTimeBudgetPool(const char* name, + BudgetPoolController* budget_pool_controller, + base::TimeTicks now, + base::Optional<base::TimeDelta> max_budget_level, + base::Optional<base::TimeDelta> max_throttling_duration); + ~CPUTimeBudgetPool(); // Throttle task queues from this time budget pool if tasks are running @@ -105,6 +114,7 @@ bool IsThrottlingEnabled() const override; void RecordTaskRunTime(base::TimeTicks start_time, base::TimeTicks end_time) override; + void BlockThrottledQueues(base::TimeTicks now) override; void Close() override; bool HasEnoughBudgetToRun(base::TimeTicks now) override; base::TimeTicks GetNextAllowedRunTime() override; @@ -112,23 +122,12 @@ base::TimeTicks now) const override; private: - friend class TaskQueueThrottler; - FRIEND_TEST_ALL_PREFIXES(TaskQueueThrottlerTest, CPUTimeBudgetPool); - CPUTimeBudgetPool(const char* name, - TaskQueueThrottler* task_queue_throttler, - base::TimeTicks now, - base::Optional<base::TimeDelta> max_budget_level, - base::Optional<base::TimeDelta> max_throttling_duration); - // Advances |last_checkpoint_| to |now| if needed and recalculates // budget level. void Advance(base::TimeTicks now); - // Disable all associated throttled queues. - void BlockThrottledQueues(base::TimeTicks now); - // Increase |current_budget_level_| to satisfy max throttling duration // condition if necessary. // Decrease |current_budget_level_| to satisfy max budget level @@ -137,7 +136,7 @@ const char* name_; // NOT OWNED - TaskQueueThrottler* task_queue_throttler_; + BudgetPoolController* budget_pool_controller_; // Max budget level which we can accrue. // Tasks will be allowed to run for this time before being throttled
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc index 8a6a96e..4b49ef2 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc
@@ -176,8 +176,8 @@ if (find_it == queue_details_.end()) return; - if (find_it->second.time_budget_pool) - find_it->second.time_budget_pool->RemoveQueue(lazy_now.Now(), task_queue); + if (find_it->second.budget_pool) + find_it->second.budget_pool->RemoveQueue(lazy_now.Now(), task_queue); queue_details_.erase(find_it); } @@ -315,7 +315,7 @@ CPUTimeBudgetPool* time_budget_pool = new CPUTimeBudgetPool(name, this, tick_clock_->NowTicks(), max_budget_level, max_throttling_duration); - time_budget_pools_[time_budget_pool] = base::WrapUnique(time_budget_pool); + budget_pools_[time_budget_pool] = base::WrapUnique(time_budget_pool); return time_budget_pool; } @@ -325,13 +325,21 @@ if (!IsThrottled(task_queue)) return; - CPUTimeBudgetPool* time_budget_pool = GetTimeBudgetPoolForQueue(task_queue); - if (!time_budget_pool) + BudgetPool* budget_pool = GetBudgetPoolForQueue(task_queue); + if (!budget_pool) return; - time_budget_pool->RecordTaskRunTime(start_time, end_time); - if (!time_budget_pool->HasEnoughBudgetToRun(end_time)) - time_budget_pool->BlockThrottledQueues(end_time); + budget_pool->RecordTaskRunTime(start_time, end_time); + if (!budget_pool->HasEnoughBudgetToRun(end_time)) + budget_pool->BlockThrottledQueues(end_time); +} + +void TaskQueueThrottler::BlockQueue(base::TimeTicks now, TaskQueue* queue) { + if (!IsThrottled(queue)) + return; + + queue->InsertFence(TaskQueue::InsertFencePosition::BEGINNING_OF_TIME); + SchedulePumpQueue(FROM_HERE, now, queue); } void TaskQueueThrottler::AsValueInto(base::trace_event::TracedValue* state, @@ -345,7 +353,7 @@ state->SetBoolean("allow_throttling", allow_throttling_); state->BeginDictionary("time_budget_pools"); - for (const auto& map_entry : time_budget_pools_) { + for (const auto& map_entry : budget_pools_) { BudgetPool* pool = map_entry.first; pool->AsValueInto(state, now); } @@ -363,38 +371,72 @@ state->EndDictionary(); } -CPUTimeBudgetPool* TaskQueueThrottler::GetTimeBudgetPoolForQueue( +void TaskQueueThrottler::AddQueueToBudgetPool(TaskQueue* queue, + BudgetPool* budget_pool) { + std::pair<TaskQueueMap::iterator, bool> insert_result = + queue_details_.insert(std::make_pair(queue, Metadata())); + + Metadata& metadata = insert_result.first->second; + + DCHECK(!metadata.budget_pool); + metadata.budget_pool = budget_pool; +} + +void TaskQueueThrottler::RemoveQueueFromBudgetPool(TaskQueue* queue, + BudgetPool* budget_pool) { + auto find_it = queue_details_.find(queue); + DCHECK(find_it != queue_details_.end() && + find_it->second.budget_pool == budget_pool); + + find_it->second.budget_pool = nullptr; + + MaybeDeleteQueueMetadata(find_it); +} + +void TaskQueueThrottler::UnregisterBudgetPool(BudgetPool* budget_pool) { + budget_pools_.erase(budget_pool); +} + +void TaskQueueThrottler::UnblockQueue(base::TimeTicks now, TaskQueue* queue) { + SchedulePumpQueue(FROM_HERE, now, queue); +} + +void TaskQueueThrottler::SchedulePumpQueue( + const tracked_objects::Location& from_here, + base::TimeTicks now, TaskQueue* queue) { + if (!IsThrottled(queue)) + return; + + LazyNow lazy_now(now); + base::Optional<base::TimeTicks> next_desired_run_time = + NextTaskRunTime(&lazy_now, queue); + if (!next_desired_run_time) + return; + + base::Optional<base::TimeTicks> next_run_time = + Max(next_desired_run_time, GetNextAllowedRunTime(now, queue)); + + MaybeSchedulePumpThrottledTasks(from_here, now, next_run_time.value()); +} + +BudgetPool* TaskQueueThrottler::GetBudgetPoolForQueue(TaskQueue* queue) { auto find_it = queue_details_.find(queue); if (find_it == queue_details_.end()) return nullptr; - return find_it->second.time_budget_pool; -} - -void TaskQueueThrottler::MaybeSchedulePumpQueue( - const tracked_objects::Location& from_here, - base::TimeTicks now, - TaskQueue* queue, - base::Optional<base::TimeTicks> next_possible_run_time) { - LazyNow lazy_now(now); - base::Optional<base::TimeTicks> next_run_time = - Max(NextTaskRunTime(&lazy_now, queue), next_possible_run_time); - - if (next_run_time) { - MaybeSchedulePumpThrottledTasks(from_here, now, next_run_time.value()); - } + return find_it->second.budget_pool; } base::TimeTicks TaskQueueThrottler::GetNextAllowedRunTime(base::TimeTicks now, TaskQueue* queue) { - CPUTimeBudgetPool* time_budget_pool = GetTimeBudgetPoolForQueue(queue); - if (!time_budget_pool) + BudgetPool* budget_pool = GetBudgetPoolForQueue(queue); + if (!budget_pool) return now; - return std::max(now, time_budget_pool->GetNextAllowedRunTime()); + return std::max(now, budget_pool->GetNextAllowedRunTime()); } void TaskQueueThrottler::MaybeDeleteQueueMetadata(TaskQueueMap::iterator it) { - if (it->second.throttling_ref_count == 0 && !it->second.time_budget_pool) + if (it->second.throttling_ref_count == 0 && !it->second.budget_pool) queue_details_.erase(it); } @@ -439,8 +481,7 @@ // to enforce task alignment. queue->InsertFence(TaskQueue::InsertFencePosition::BEGINNING_OF_TIME); queue->SetTimeDomain(time_domain_.get()); - MaybeSchedulePumpQueue(FROM_HERE, lazy_now.Now(), queue, - GetNextAllowedRunTime(lazy_now.Now(), queue)); + SchedulePumpQueue(FROM_HERE, lazy_now.Now(), queue); } TRACE_EVENT0(tracing_category_, "TaskQueueThrottler_EnableThrottling");
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h index 6e5014c..1cbab9d 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h +++ b/third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h
@@ -14,6 +14,7 @@ #include "base/threading/thread_checker.h" #include "platform/scheduler/base/cancelable_closure_holder.h" #include "platform/scheduler/base/time_domain.h" +#include "platform/scheduler/renderer/budget_pool.h" #include "public/platform/WebViewScheduler.h" namespace base { @@ -30,6 +31,33 @@ class ThrottledTimeDomain; class CPUTimeBudgetPool; +// Interface for BudgetPool to interact with TaskQueueThrottler. +class BLINK_PLATFORM_EXPORT BudgetPoolController { + public: + virtual ~BudgetPoolController() {} + + // To be used by BudgetPool only, use BudgetPool::{Add,Remove}Queue + // methods instead. + virtual void AddQueueToBudgetPool(TaskQueue* queue, + BudgetPool* budget_pool) = 0; + virtual void RemoveQueueFromBudgetPool(TaskQueue* queue, + BudgetPool* budget_pool) = 0; + + // Deletes the budget pool. + virtual void UnregisterBudgetPool(BudgetPool* budget_pool) = 0; + + // Insert a fence to prevent tasks from running and schedule a wakeup at + // an appropriate time. + virtual void BlockQueue(base::TimeTicks now, TaskQueue* queue) = 0; + + // Schedule a call to unblock queue at an appropriate moment. + virtual void UnblockQueue(base::TimeTicks now, TaskQueue* queue) = 0; + + // Returns true if the |queue| is throttled (i.e. added to TaskQueueThrottler + // and throttling is not disabled). + virtual bool IsThrottled(TaskQueue* queue) const = 0; +}; + // The job of the TaskQueueThrottler is to control when tasks posted on // throttled queues get run. The TaskQueueThrottler: // - runs throttled tasks once per second, @@ -52,7 +80,8 @@ // See IncreaseThrottleRefCount & DecreaseThrottleRefCount. // // This class is main-thread only. -class BLINK_PLATFORM_EXPORT TaskQueueThrottler : public TimeDomain::Observer { +class BLINK_PLATFORM_EXPORT TaskQueueThrottler : public TimeDomain::Observer, + public BudgetPoolController { public: // TODO(altimin): Do not pass tracing category as const char*, // hard-code string instead. @@ -65,6 +94,15 @@ void OnTimeDomainHasImmediateWork(TaskQueue*) override; void OnTimeDomainHasDelayedWork(TaskQueue*) override; + // BudgetPoolController implementation: + void AddQueueToBudgetPool(TaskQueue* queue, BudgetPool* budget_pool) override; + void RemoveQueueFromBudgetPool(TaskQueue* queue, + BudgetPool* budget_pool) override; + void UnregisterBudgetPool(BudgetPool* budget_pool) override; + void BlockQueue(base::TimeTicks now, TaskQueue* queue) override; + void UnblockQueue(base::TimeTicks now, TaskQueue* queue) override; + bool IsThrottled(TaskQueue* queue) const override; + // Increments the throttled refcount and causes |task_queue| to be throttled // if its not already throttled. void IncreaseThrottleRefCount(TaskQueue* task_queue); @@ -77,9 +115,6 @@ // Removes |task_queue| from |queue_details| and from appropriate budget pool. void UnregisterTaskQueue(TaskQueue* task_queue); - // Returns true if the |task_queue| is throttled. - bool IsThrottled(TaskQueue* task_queue) const; - // Disable throttling for all queues, this setting takes precedence over // all other throttling settings. Designed to be used when a global event // disabling throttling happens (e.g. audio is playing). @@ -108,17 +143,13 @@ void AsValueInto(base::trace_event::TracedValue* state, base::TimeTicks now) const; - private: - friend class BudgetPool; - friend class CPUTimeBudgetPool; - struct Metadata { - Metadata() : throttling_ref_count(0), time_budget_pool(nullptr) {} + Metadata() : throttling_ref_count(0), budget_pool(nullptr) {} size_t throttling_ref_count; - CPUTimeBudgetPool* time_budget_pool; + BudgetPool* budget_pool; }; using TaskQueueMap = std::unordered_map<TaskQueue*, Metadata>; @@ -132,14 +163,7 @@ base::TimeTicks now, base::TimeTicks runtime); - CPUTimeBudgetPool* GetTimeBudgetPoolForQueue(TaskQueue* queue); - - // Schedule pumping because of given task queue. - void MaybeSchedulePumpQueue( - const tracked_objects::Location& from_here, - base::TimeTicks now, - TaskQueue* queue, - base::Optional<base::TimeTicks> next_possible_run_time); + BudgetPool* GetBudgetPoolForQueue(TaskQueue* queue); // Return next possible time when queue is allowed to run in accordance // with throttling policy. @@ -147,6 +171,11 @@ void MaybeDeleteQueueMetadata(TaskQueueMap::iterator it); + // Schedule a call PumpThrottledTasks at an appropriate moment for this queue. + void SchedulePumpQueue(const tracked_objects::Location& from_here, + base::TimeTicks now, + TaskQueue* queue); + TaskQueueMap queue_details_; base::Callback<void(TaskQueue*)> forward_immediate_work_callback_; scoped_refptr<TaskQueue> task_runner_; @@ -159,8 +188,7 @@ base::Optional<base::TimeTicks> pending_pump_throttled_tasks_runtime_; bool allow_throttling_; - std::unordered_map<BudgetPool*, std::unique_ptr<BudgetPool>> - time_budget_pools_; + std::unordered_map<BudgetPool*, std::unique_ptr<BudgetPool>> budget_pools_; base::WeakPtrFactory<TaskQueueThrottler> weak_factory_;
diff --git a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp index 1cf04868..cff003f 100644 --- a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp +++ b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
@@ -156,7 +156,7 @@ HitTestResult r = m_webView->page()->contextMenuController().hitTestResult(); - r.setToShadowHostIfInUserAgentShadowRoot(); + r.setToShadowHostIfInRestrictedShadowRoot(); LocalFrame* selectedFrame = r.innerNodeFrame();
diff --git a/third_party/WebKit/Source/web/PageWidgetDelegate.cpp b/third_party/WebKit/Source/web/PageWidgetDelegate.cpp index 27fcf54..9f5e391f 100644 --- a/third_party/WebKit/Source/web/PageWidgetDelegate.cpp +++ b/third_party/WebKit/Source/web/PageWidgetDelegate.cpp
@@ -128,7 +128,7 @@ flooredIntPoint(mouseEvent.positionInRootFrame()))); HitTestResult result = root->eventHandler().hitTestResultAtPoint( docPoint, HitTestRequest::ReadOnly | HitTestRequest::Active); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); if (result.innerNodeFrame()) { Document* document = result.innerNodeFrame()->document(); if (document) {
diff --git a/third_party/WebKit/Source/web/WebFrame.cpp b/third_party/WebKit/Source/web/WebFrame.cpp index 099be8f5..d10fef69 100644 --- a/third_party/WebKit/Source/web/WebFrame.cpp +++ b/third_party/WebKit/Source/web/WebFrame.cpp
@@ -74,7 +74,7 @@ FrameOwner* owner = oldFrame->owner(); v8::HandleScope handleScope(v8::Isolate::GetCurrent()); - WindowProxyManagerBase::GlobalsVector globals; + WindowProxyManager::GlobalsVector globals; oldFrame->getWindowProxyManager()->clearForNavigation(); oldFrame->getWindowProxyManager()->releaseGlobals(globals);
diff --git a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp index 631c930..21987cb 100644 --- a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp +++ b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
@@ -744,7 +744,7 @@ point = m_localRoot->frameView()->rootFrameToContents(point); HitTestResult result( m_localRoot->frame()->eventHandler().hitTestResultAtPoint(point)); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); Node* hitNode = result.innerNode(); if (!result.scrollbar() && hitNode && hitNode->layoutObject() && @@ -1115,7 +1115,7 @@ HitTestResult result = m_localRoot->frame()->eventHandler().hitTestResultAtPoint( docPoint, HitTestRequest::ReadOnly | HitTestRequest::Active); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); return result; }
diff --git a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp index 34a3e9c3..14900c1 100644 --- a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp +++ b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
@@ -1927,7 +1927,7 @@ IntPoint docPoint(frame()->view()->rootFrameToContents(rootFramePoint)); HitTestResult result = frame()->eventHandler().hitTestResultAtPoint( docPoint, HitTestRequest::ReadOnly | HitTestRequest::Active); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); return result; }
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp index 0d1daa1..4777ed0 100644 --- a/third_party/WebKit/Source/web/WebViewImpl.cpp +++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -506,7 +506,7 @@ HitTestResult result( m_page->deprecatedLocalMainFrame()->eventHandler().hitTestResultAtPoint( point)); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); Node* hitNode = result.innerNodeOrImageMapImage(); if (!result.scrollbar() && hitNode && hitNode->layoutObject() && @@ -1264,7 +1264,7 @@ HitTestResult result = mainFrameImpl()->frame()->eventHandler().hitTestResultAtPoint(point, hitType); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); Node* node = result.innerNodeOrImageMapImage(); if (!node) @@ -3776,7 +3776,7 @@ HitTestResult result = m_page->deprecatedLocalMainFrame()->eventHandler().hitTestResultAtPoint( docPoint, HitTestRequest::ReadOnly | HitTestRequest::Active); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); return result; } @@ -3807,7 +3807,7 @@ scaledEvent, HitTestRequest::ReadOnly | HitTestRequest::Active) .hitTestResult(); - result.setToShadowHostIfInUserAgentShadowRoot(); + result.setToShadowHostIfInRestrictedShadowRoot(); return result; } @@ -4064,7 +4064,7 @@ // Need a local copy of the hit test as // setToShadowHostIfInUserAgentShadowRoot() will modify it. HitTestResult touchHit = targetedEvent.hitTestResult(); - touchHit.setToShadowHostIfInUserAgentShadowRoot(); + touchHit.setToShadowHostIfInRestrictedShadowRoot(); if (touchHit.isContentEditable()) return false;