diff --git a/build/android/gyp/lint.py b/build/android/gyp/lint.py index c48154d4..ea9055cf 100755 --- a/build/android/gyp/lint.py +++ b/build/android/gyp/lint.py
@@ -113,8 +113,7 @@ else: # This is a zip file with generated resources (e. g. strings from GRD). # Extract it to temporary folder. - resource_dir = _NewTempSubdir(_RebasePath(resource_source), - append_digit=False) + resource_dir = _NewTempSubdir(resource_source, append_digit=False) resource_dirs.append(resource_dir) build_utils.ExtractAll(resource_source, path=resource_dir)
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java index 57117f0..a6b95d0 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
@@ -140,8 +140,15 @@ ChromeActivity activity = mTargetActivity.get(); if (activity == null) return; getInstance(activity).mDonSucceeded = true; - ((ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE)) - .moveTaskToFront(activity.getTaskId(), 0); + if (sInstance.mPaused) { + ((ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE)) + .moveTaskToFront(activity.getTaskId(), 0); + } else { + // If a WebVR app calls requestPresent in response to the displayactivate event + // after the DON flow completes, the DON flow is skipped, meaning our app won't be + // paused when daydream fires our BroadcastReceiver, so onResume won't be called. + sInstance.handleDonFlowSuccess(); + } } /** @@ -610,12 +617,7 @@ } if (mDonSucceeded) { - mDonSucceeded = false; - // If we fail to enter VR when we should have entered VR, return to the home screen. - if (!enterVrAfterDon()) { - maybeSetPresentResult(false); - mVrDaydreamApi.launchVrHomescreen(); - } + handleDonFlowSuccess(); } else if (mRestoreOrientation != null) { // This means the user backed out of the DON flow, and we won't be entering VR. maybeSetPresentResult(false); @@ -623,6 +625,15 @@ } } + private void handleDonFlowSuccess() { + mDonSucceeded = false; + // If we fail to enter VR when we should have entered VR, return to the home screen. + if (!enterVrAfterDon()) { + maybeSetPresentResult(false); + mVrDaydreamApi.launchVrHomescreen(); + } + } + private void pauseVr() { mPaused = true; if (mVrSupportLevel == VR_NOT_AVAILABLE) return;
diff --git a/chrome/browser/captive_portal/captive_portal_tab_helper.cc b/chrome/browser/captive_portal/captive_portal_tab_helper.cc index 33d6433a..f4261dc 100644 --- a/chrome/browser/captive_portal/captive_portal_tab_helper.cc +++ b/chrome/browser/captive_portal/captive_portal_tab_helper.cc
@@ -58,23 +58,26 @@ void CaptivePortalTabHelper::DidStartNavigation( content::NavigationHandle* navigation_handle) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (!navigation_handle->IsInMainFrame()) + if (!navigation_handle->IsInMainFrame() || + navigation_handle->IsSameDocument()) { return; + } // TODO(clamy): Remove this when we understand the root cause behind // crbug.com/704892. if (navigation_handle == navigation_handle_) base::debug::DumpWithoutCrashing(); + bool was_tracking_navigation = !!navigation_handle_; + navigation_handle_ = navigation_handle; + // Always track the latest navigation. If a navigation was already tracked, // and it committed (either the navigation proper or an error page), it is // safe to start tracking the new navigation. Otherwise simulate an abort // before reporting the start of the new navigation. - if (navigation_handle_ && !navigation_handle_->HasCommitted()) { + if (was_tracking_navigation) tab_reloader_->OnAbort(); - } - navigation_handle_ = navigation_handle; tab_reloader_->OnLoadStart( navigation_handle->GetURL().SchemeIsCryptographic()); } @@ -92,36 +95,36 @@ void CaptivePortalTabHelper::DidFinishNavigation( content::NavigationHandle* navigation_handle) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (!navigation_handle->IsInMainFrame()) { - if (navigation_handle == navigation_handle_) - // TODO(clamy): Remove this when we understand the root cause behind - // crbug.com/704892. - base::debug::DumpWithoutCrashing(); + // Exclude subframe navigations. + if (!navigation_handle->IsInMainFrame()) + return; + + // Exclude same-document navigations and aborted navigations that were not + // being tracked. + if (navigation_handle_ != navigation_handle && + (!navigation_handle->HasCommitted() || + navigation_handle->IsSameDocument())) { return; } - if (navigation_handle_ != navigation_handle) { - // Another navigation is being tracked, so there is no need to update the - // TabReloader. - if (!navigation_handle->HasCommitted()) - return; - // An untracked navigation just committed. Simulate its start before - // informing the TabReloader of its commit. - DidStartNavigation(navigation_handle); - } + bool need_to_simulate_start = navigation_handle_ != navigation_handle; + bool need_to_simulate_previous_abort = + need_to_simulate_start && !!navigation_handle_; + navigation_handle_ = nullptr; - // TODO(clamy): Remove this when we understand the root cause behind - // crbug.com/704892. - if (navigation_handle != navigation_handle_) - base::debug::DumpWithoutCrashing(); + if (need_to_simulate_previous_abort) + tab_reloader_->OnAbort(); + + if (need_to_simulate_start) { + tab_reloader_->OnLoadStart( + navigation_handle->GetURL().SchemeIsCryptographic()); + } if (navigation_handle->HasCommitted()) { tab_reloader_->OnLoadCommitted(navigation_handle->GetNetErrorCode()); } else { tab_reloader_->OnAbort(); } - - navigation_handle_ = nullptr; } void CaptivePortalTabHelper::DidStopLoading() {
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 49452e8..41f485b7 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc
@@ -1119,21 +1119,15 @@ renderer_helper_->OnExtensionLoaded(*extension); - // Tell subsystems that use the EXTENSION_LOADED notification about the new - // extension. + // Tell subsystems that use the ExtensionRegistryObserver::OnExtensionLoaded + // about the new extension. // // NOTE: It is important that this happen after notifying the renderers about // the new extensions so that if we navigate to an extension URL in - // ExtensionRegistryObserver::OnLoaded or - // NOTIFICATION_EXTENSION_LOADED_DEPRECATED, the - // renderer is guaranteed to know about it. + // ExtensionRegistryObserver::OnExtensionLoaded the renderer is guaranteed to + // know about it. registry_->TriggerOnLoaded(extension); - content::NotificationService::current()->Notify( - extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED, - content::Source<Profile>(profile_), - content::Details<const Extension>(extension)); - // TODO(kalman): Convert ExtensionSpecialStoragePolicy to a // BrowserContextKeyedService and use ExtensionRegistryObserver. profile_->GetExtensionSpecialStoragePolicy()->
diff --git a/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc index ef5d5c4..cc36f635 100644 --- a/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc +++ b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
@@ -443,27 +443,6 @@ safe_browsing_->OnResourceRequest(request); const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request); - -// The lowering of request priority causes issues with scheduling, since -// content::ResourceScheduler uses it to delay and throttle requests. This is -// disabled only on Android, as the prerenders are not likely to compete with -// page loads there. -// See https://crbug.com/652746 for details. -// TODO(lizeb,droger): Fix the issue on all platforms. -#if !defined(OS_ANDROID) - bool is_prerendering = - info->GetVisibilityState() == blink::kWebPageVisibilityStatePrerender; - if (is_prerendering) { - // Requests with the IGNORE_LIMITS flag set (i.e., sync XHRs) - // should remain at MAXIMUM_PRIORITY. - if (request->load_flags() & net::LOAD_IGNORE_LIMITS) { - DCHECK_EQ(request->priority(), net::MAXIMUM_PRIORITY); - } else { - request->SetPriority(net::IDLE); - } - } -#endif // OS_ANDROID - ProfileIOData* io_data = ProfileIOData::FromResourceContext( resource_context);
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc index e12a573..fdac9ca 100644 --- a/chrome/browser/prerender/prerender_browsertest.cc +++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -143,7 +143,7 @@ using net::NetworkChangeNotifier; using prerender::test_utils::RequestCounter; using prerender::test_utils::CreateCountingInterceptorOnIO; -using prerender::test_utils::CreateHangingFirstRequestInterceptorOnIO; +using prerender::test_utils::CreateHangingFirstRequestInterceptor; using prerender::test_utils::CreateMockInterceptorOnIO; using prerender::test_utils::TestPrerender; using prerender::test_utils::TestPrerenderContents; @@ -166,6 +166,8 @@ namespace { +const char kPrefetchJpeg[] = "/prerender/image.jpeg"; + class FaviconUpdateWatcher : public favicon::FaviconDriverObserver { public: explicit FaviconUpdateWatcher(content::WebContents* web_contents) @@ -539,6 +541,17 @@ dest_url, false /* started_in_foreground */); } +// Helper function, to allow passing a UI closure to +// CreateHangingFirstRequestInterceptor() instead of a IO callback. +base::Callback<void(net::URLRequest*)> GetIOCallbackFromUIClosure( + base::Closure ui_closure) { + auto lambda = [](base::Closure closure, net::URLRequest*) { + content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, + closure); + }; + return base::Bind(lambda, ui_closure); +} + } // namespace class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest { @@ -1302,10 +1315,9 @@ base::FilePath file(GetTestPath("prerender_page.html")); base::RunLoop prerender_start_loop; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, kNoCommitUrl, file, - prerender_start_loop.QuitClosure())); + CreateHangingFirstRequestInterceptor( + kNoCommitUrl, file, + GetIOCallbackFromUIClosure(prerender_start_loop.QuitClosure())); DisableJavascriptCalls(); PrerenderTestURL(kNoCommitUrl, FINAL_STATUS_NAVIGATION_UNCOMMITTED, @@ -1330,10 +1342,9 @@ base::FilePath file(GetTestPath("prerender_page.html")); base::RunLoop prerender_start_loop; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, kNoCommitUrl, file, - prerender_start_loop.QuitClosure())); + CreateHangingFirstRequestInterceptor( + kNoCommitUrl, file, + GetIOCallbackFromUIClosure(prerender_start_loop.QuitClosure())); DisableJavascriptCalls(); PrerenderTestURL(CreateClientRedirect(kNoCommitUrl.spec()), FINAL_STATUS_APP_TERMINATING, 1); @@ -2109,7 +2120,7 @@ // Checks that prerendering a JPG works correctly. IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderImageJpeg) { DisableJavascriptCalls(); - PrerenderTestURL("/prerender/image.jpeg", FINAL_STATUS_USED, 1); + PrerenderTestURL(kPrefetchJpeg, FINAL_STATUS_USED, 1); NavigateToDestURL(); } @@ -2181,7 +2192,7 @@ https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_MISMATCHED_NAME); https_server.ServeFilesFromSourceDirectory("chrome/test/data"); ASSERT_TRUE(https_server.Start()); - GURL https_url = https_server.GetURL("/prerender/image.jpeg"); + GURL https_url = https_server.GetURL(kPrefetchJpeg); base::StringPairs replacement_text; replacement_text.push_back( std::make_pair("REPLACE_WITH_IMAGE_URL", https_url.spec())); @@ -2293,7 +2304,7 @@ https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_OK, ssl_config); https_server.ServeFilesFromSourceDirectory("chrome/test/data"); ASSERT_TRUE(https_server.Start()); - GURL https_url = https_server.GetURL("/prerender/image.jpeg"); + GURL https_url = https_server.GetURL(kPrefetchJpeg); base::StringPairs replacement_text; replacement_text.push_back( std::make_pair("REPLACE_WITH_IMAGE_URL", https_url.spec())); @@ -2367,7 +2378,7 @@ // Ensures that we do not prerender pages which have a malware subresource. IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderSafeBrowsingSubresource) { - GURL image_url = embedded_test_server()->GetURL("/prerender/image.jpeg"); + GURL image_url = embedded_test_server()->GetURL(kPrefetchJpeg); GetFakeSafeBrowsingDatabaseManager()->SetThreatTypeForUrl( image_url, safe_browsing::SB_THREAT_TYPE_URL_MALWARE); base::StringPairs replacement_text; @@ -2491,11 +2502,8 @@ const GURL hang_url("http://unload-url.test"); base::FilePath empty_file = ui_test_utils::GetTestFilePath( base::FilePath(), base::FilePath(FILE_PATH_LITERAL("empty.html"))); - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, - hang_url, empty_file, - base::Closure())); + CreateHangingFirstRequestInterceptor( + hang_url, empty_file, base::Callback<void(net::URLRequest*)>()); set_loader_path("/prerender/prerender_loader_with_unload.html"); PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1); @@ -3291,6 +3299,122 @@ EXPECT_EQ(0, done_counter.count()); } +// Checks that the requests from a prerender are IDLE priority before the swap +// (except on Android), but normal priority after the swap. +IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, ResourcePriority) { + GURL before_swap_url = embedded_test_server()->GetURL(kPrefetchJpeg); + GURL after_swap_url = embedded_test_server()->GetURL("/prerender/image.png"); + GURL main_page_url = + GetURLWithReplacement("/prerender/prerender_with_image.html", + "REPLACE_WITH_IMAGE_URL", kPrefetchJpeg); + + // Setup request interceptors for subresources. + auto get_priority_lambda = [](net::RequestPriority* out_priority, + net::URLRequest* request) { + *out_priority = request->priority(); + }; + RequestCounter before_swap_counter; + net::RequestPriority before_swap_priority = net::THROTTLED; + InterceptRequestAndCount( + before_swap_url, &before_swap_counter, + base::Bind(get_priority_lambda, base::Unretained(&before_swap_priority))); + RequestCounter after_swap_counter; + net::RequestPriority after_swap_priority = net::THROTTLED; + InterceptRequestAndCount( + after_swap_url, &after_swap_counter, + base::Bind(get_priority_lambda, base::Unretained(&after_swap_priority))); + + // Start the prerender. + PrerenderTestURL(main_page_url, FINAL_STATUS_USED, 1); + + // Check priority before swap. + before_swap_counter.WaitForCount(1); +#if defined(OS_ANDROID) + EXPECT_GT(before_swap_priority, net::IDLE); +#else + EXPECT_EQ(net::IDLE, before_swap_priority); +#endif + + // Swap. + NavigateToDestURL(); + + // Check priority after swap. + GetActiveWebContents()->GetMainFrame()->ExecuteJavaScriptForTests( + base::ASCIIToUTF16( + "var img=new Image(); img.src='/prerender/image.png'")); + after_swap_counter.WaitForCount(1); + EXPECT_NE(net::IDLE, after_swap_priority); +} + +// Checks that a request started before the swap gets its original priority back +// after the swap. +IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, ResourcePriorityOverlappingSwap) { + GURL image_url = embedded_test_server()->GetURL(kPrefetchJpeg); + GURL main_page_url = + GetURLWithReplacement("/prerender/prerender_with_image.html", + "REPLACE_WITH_IMAGE_URL", kPrefetchJpeg); + + // Setup request interceptors for subresources. + net::URLRequest* url_request = nullptr; + net::RequestPriority priority = net::THROTTLED; + base::RunLoop wait_loop; + auto io_lambda = [](net::URLRequest** out_request, + net::RequestPriority* out_priority, base::Closure closure, + net::URLRequest* request) { + if (out_request) + *out_request = request; + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind( + [](net::RequestPriority priority, + net::RequestPriority* out_priority, base::Closure closure) { + *out_priority = priority; + closure.Run(); + }, + request->priority(), base::Unretained(out_priority), closure)); + }; + + CreateHangingFirstRequestInterceptor( + image_url, base::FilePath(), + base::Bind(io_lambda, base::Unretained(&url_request), + base::Unretained(&priority), wait_loop.QuitClosure())); + + // The prerender will hang on the image resource, can't run the usual checks. + DisableLoadEventCheck(); + DisableJavascriptCalls(); + // Start the prerender. + PrerenderTestURL(main_page_url, FINAL_STATUS_USED, 0); + +// Check priority before swap. +#if defined(OS_ANDROID) + if (priority <= net::IDLE) + wait_loop.Run(); + EXPECT_GT(priority, net::IDLE); +#else + if (priority != net::IDLE) + wait_loop.Run(); + EXPECT_EQ(net::IDLE, priority); +#endif + + // Swap. Cannot use NavigateToDestURL, because it waits for the load to + // complete, but the resource is still hung. + current_browser()->OpenURL(content::OpenURLParams( + dest_url(), Referrer(), WindowOpenDisposition::CURRENT_TAB, + ui::PAGE_TRANSITION_TYPED, false)); + + // Check priority after swap. The test may timeout in case of failure. + priority = net::THROTTLED; + do { + base::RunLoop loop; + content::BrowserThread::PostTask( + content::BrowserThread::IO, FROM_HERE, + base::Bind(io_lambda, nullptr, base::Unretained(&priority), + loop.QuitClosure(), base::Unretained(url_request))); + loop.Run(); + } while (priority <= net::IDLE); + EXPECT_GT(priority, net::IDLE); +} + IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, FirstContentfulPaintTimingSimple) { GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting(); base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks(); @@ -3324,10 +3448,9 @@ GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html"); base::RunLoop hanging_request_waiter; - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, - url, GetTestPath("prerender_page.html"), - hanging_request_waiter.QuitClosure())); + CreateHangingFirstRequestInterceptor( + url, GetTestPath("prerender_page.html"), + GetIOCallbackFromUIClosure(hanging_request_waiter.QuitClosure())); // As this load will be canceled, it is not waited for, and hence no // javascript is executed. DisableJavascriptCalls(); @@ -3414,10 +3537,9 @@ base::FilePath(FILE_PATH_LITERAL("prerender/prerender_page.html"))); base::RunLoop prerender_start_loop; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, url, url_file, - prerender_start_loop.QuitClosure())); + CreateHangingFirstRequestInterceptor( + url, url_file, + GetIOCallbackFromUIClosure(prerender_start_loop.QuitClosure())); // As this load is uncommitted, it is not waited for, and hence no // javascript is executed. DisableJavascriptCalls(); @@ -3537,10 +3659,9 @@ base::FilePath(FILE_PATH_LITERAL("prerender/prerender_page.html"))); base::RunLoop prerender_start_loop; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&CreateHangingFirstRequestInterceptorOnIO, url, url_file, - prerender_start_loop.QuitClosure())); + CreateHangingFirstRequestInterceptor( + url, url_file, + GetIOCallbackFromUIClosure(prerender_start_loop.QuitClosure())); // As this load is uncommitted, it is not waited for, and hence no // javascript is executed. DisableJavascriptCalls();
diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc index 8fa33154..f533a2b 100644 --- a/chrome/browser/prerender/prerender_contents.cc +++ b/chrome/browser/prerender/prerender_contents.cc
@@ -70,7 +70,12 @@ }; void ResumeThrottles( - std::vector<base::WeakPtr<PrerenderResourceThrottle> > throttles) { + std::vector<base::WeakPtr<PrerenderResourceThrottle>> throttles, + std::vector<base::WeakPtr<PrerenderResourceThrottle>> idle_resources) { + for (auto resource : idle_resources) { + if (resource) + resource->ResetResourcePriority(); + } for (size_t i = 0; i < throttles.size(); i++) { if (throttles[i]) throttles[i]->ResumeHandler(); @@ -757,10 +762,10 @@ NotifyPrerenderStop(); BrowserThread::PostTask( - BrowserThread::IO, - FROM_HERE, - base::Bind(&ResumeThrottles, resource_throttles_)); + BrowserThread::IO, FROM_HERE, + base::Bind(&ResumeThrottles, resource_throttles_, idle_resources_)); resource_throttles_.clear(); + idle_resources_.clear(); } void PrerenderContents::CancelPrerenderForPrinting() { @@ -778,6 +783,11 @@ resource_throttles_.push_back(throttle); } +void PrerenderContents::AddIdleResource( + const base::WeakPtr<PrerenderResourceThrottle>& throttle) { + idle_resources_.push_back(throttle); +} + void PrerenderContents::AddNetworkBytes(int64_t bytes) { network_bytes_ += bytes; for (Observer& observer : observer_list_)
diff --git a/chrome/browser/prerender/prerender_contents.h b/chrome/browser/prerender/prerender_contents.h index 082fa62d..3648fefd 100644 --- a/chrome/browser/prerender/prerender_contents.h +++ b/chrome/browser/prerender/prerender_contents.h
@@ -235,6 +235,12 @@ void AddResourceThrottle( const base::WeakPtr<PrerenderResourceThrottle>& throttle); + // Called when a PrerenderResourceThrottle changes a resource priority to + // net::IDLE. The resources are reset back to their original priorities when + // the prerender contents is swapped in. + void AddIdleResource( + const base::WeakPtr<PrerenderResourceThrottle>& throttle); + // Increments the number of bytes fetched over the network for this prerender. void AddNetworkBytes(int64_t bytes); @@ -372,6 +378,8 @@ // Resources that are throttled, pending a prerender use. Can only access a // throttle on the IO thread. std::vector<base::WeakPtr<PrerenderResourceThrottle> > resource_throttles_; + // Resources for which the priority was lowered to net::IDLE. + std::vector<base::WeakPtr<PrerenderResourceThrottle>> idle_resources_; // A running tally of the number of bytes this prerender has caused to be // transferred over the network for resources. Updated with AddNetworkBytes.
diff --git a/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc b/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc index ecd1846..34ac7f9 100644 --- a/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc +++ b/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
@@ -352,10 +352,8 @@ base::FilePath first_path = ui_test_utils::GetTestFilePath( base::FilePath(), base::FilePath().AppendASCII(kPrefetchPage)); - content::BrowserThread::PostTask( - content::BrowserThread::IO, FROM_HERE, - base::Bind(&test_utils::CreateHangingFirstRequestInterceptorOnIO, - first_url, first_path, base::Closure())); + test_utils::CreateHangingFirstRequestInterceptor( + first_url, first_path, base::Callback<void(net::URLRequest*)>()); // Start the first prefetch directly instead of via PrefetchFromFile for the // first prefetch to avoid the wait on prerender stop.
diff --git a/chrome/browser/prerender/prerender_resource_throttle.cc b/chrome/browser/prerender/prerender_resource_throttle.cc index afd908f..b6ed41fd 100644 --- a/chrome/browser/prerender/prerender_resource_throttle.cc +++ b/chrome/browser/prerender/prerender_resource_throttle.cc
@@ -12,6 +12,7 @@ #include "chrome/browser/prerender/prerender_manager.h" #include "chrome/browser/prerender/prerender_util.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/resource_dispatcher_host.h" #include "content/public/browser/web_contents.h" #include "net/base/load_flags.h" #include "net/http/http_response_headers.h" @@ -85,7 +86,29 @@ PrerenderResourceThrottle::PrerenderResourceThrottle(net::URLRequest* request) : request_(request), load_flags_(net::LOAD_NORMAL), - prerender_throttle_info_(new PrerenderThrottleInfo()) {} + prerender_throttle_info_(new PrerenderThrottleInfo()) { +// Priorities for prerendering requests are lowered, to avoid competing with +// other page loads, except on Android where this is less likely to be a +// problem. In some cases, this may negatively impact the performance of +// prerendering, see https://crbug.com/652746 for details. +#if !defined(OS_ANDROID) + // Requests with the IGNORE_LIMITS flag set (i.e., sync XHRs) + // should remain at MAXIMUM_PRIORITY. + if (request_->load_flags() & net::LOAD_IGNORE_LIMITS) { + DCHECK_EQ(request_->priority(), net::MAXIMUM_PRIORITY); + } else if (request_->priority() != net::IDLE) { + original_request_priority_ = request_->priority(); + // In practice, the resource scheduler does not know about the request yet, + // and it falls back to calling request_->SetPriority(), so it would be + // possible to do just that here. It is cleaner and more robust to go + // through the resource dispatcher host though. + if (content::ResourceDispatcherHost::Get()) { + content::ResourceDispatcherHost::Get()->ReprioritizeRequest(request_, + net::IDLE); + } + } +#endif // OS_ANDROID +} PrerenderResourceThrottle::~PrerenderResourceThrottle() {} @@ -94,6 +117,7 @@ const content::ResourceRequestInfo* info = content::ResourceRequestInfo::ForRequest(request_); *defer = true; + BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, base::Bind(&PrerenderResourceThrottle::WillStartRequestOnUI, AsWeakPtr(), @@ -149,6 +173,16 @@ Resume(); } +void PrerenderResourceThrottle::ResetResourcePriority() { + if (!original_request_priority_) + return; + + if (content::ResourceDispatcherHost::Get()) { + content::ResourceDispatcherHost::Get()->ReprioritizeRequest( + request_, original_request_priority_.value()); + } +} + // static void PrerenderResourceThrottle::WillStartRequestOnUI( const base::WeakPtr<PrerenderResourceThrottle>& throttle, @@ -202,9 +236,16 @@ // Delay icon fetching until the contents are getting swapped in // to conserve network usage in mobile devices. prerender_contents->AddResourceThrottle(throttle); + + // No need to call AddIdleResource() on Android. return; #endif } + +#if !defined(OS_ANDROID) + if (!cancel) + prerender_contents->AddIdleResource(throttle); +#endif } BrowserThread::PostTask(
diff --git a/chrome/browser/prerender/prerender_resource_throttle.h b/chrome/browser/prerender/prerender_resource_throttle.h index a9f853e..0302ec5 100644 --- a/chrome/browser/prerender/prerender_resource_throttle.h +++ b/chrome/browser/prerender/prerender_resource_throttle.h
@@ -10,10 +10,12 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" +#include "base/optional.h" #include "chrome/common/prerender_types.h" #include "content/public/browser/resource_request_info.h" #include "content/public/browser/resource_throttle.h" #include "content/public/common/resource_type.h" +#include "net/base/request_priority.h" class GURL; @@ -50,6 +52,9 @@ // May only be called if currently throttling the resource. void ResumeHandler(); + // Resets the resource priority back to its original value. + void ResetResourcePriority(); + static void OverridePrerenderContentsForTesting(PrerenderContents* contents); private: @@ -90,6 +95,11 @@ net::URLRequest* request_; int load_flags_; // Load flags to be OR'ed with the existing request flags. + // The throttle changes most request priorities to IDLE during prerendering. + // The priority is reset back to the original priority when prerendering is + // finished. + base::Optional<net::RequestPriority> original_request_priority_; + scoped_refptr<PrerenderThrottleInfo> prerender_throttle_info_; DISALLOW_COPY_AND_ASSIGN(PrerenderResourceThrottle);
diff --git a/chrome/browser/prerender/prerender_test_utils.cc b/chrome/browser/prerender/prerender_test_utils.cc index 9ee543a..15f46ca 100644 --- a/chrome/browser/prerender/prerender_test_utils.cc +++ b/chrome/browser/prerender/prerender_test_utils.cc
@@ -177,12 +177,10 @@ class HangingFirstRequestInterceptor : public net::URLRequestInterceptor { public: - HangingFirstRequestInterceptor(const base::FilePath& file, - base::Closure callback) - : file_(file), - callback_(callback), - first_run_(true) { - } + HangingFirstRequestInterceptor( + const base::FilePath& file, + base::Callback<void(net::URLRequest*)> callback) + : file_(file), callback_(callback), first_run_(true) {} ~HangingFirstRequestInterceptor() override {} net::URLRequestJob* MaybeInterceptRequest( @@ -190,10 +188,8 @@ net::NetworkDelegate* network_delegate) const override { if (first_run_) { first_run_ = false; - if (!callback_.is_null()) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, callback_); - } + if (!callback_.is_null()) + callback_.Run(request); return new HangingURLRequestJob(request, network_delegate); } return new net::URLRequestMockHTTPJob( @@ -206,7 +202,7 @@ private: base::FilePath file_; - base::Closure callback_; + base::Callback<void(net::URLRequest*)> callback_; mutable bool first_run_; }; @@ -251,6 +247,17 @@ void FinishedProcessingCheck() override { NOTREACHED(); } }; +void CreateHangingFirstRequestInterceptorOnIO( + const GURL& url, + const base::FilePath& file, + base::Callback<void(net::URLRequest*)> callback_io) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + std::unique_ptr<net::URLRequestInterceptor> interceptor( + new HangingFirstRequestInterceptor(file, callback_io)); + net::URLRequestFilter::GetInstance()->AddUrlInterceptor( + url, std::move(interceptor)); +} + } // namespace RequestCounter::RequestCounter() : count_(0), expected_count_(-1) {} @@ -813,13 +820,15 @@ file, content::BrowserThread::GetBlockingPool())); } -void CreateHangingFirstRequestInterceptorOnIO( - const GURL& url, const base::FilePath& file, base::Closure callback) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - std::unique_ptr<net::URLRequestInterceptor> interceptor( - new HangingFirstRequestInterceptor(file, callback)); - net::URLRequestFilter::GetInstance()->AddUrlInterceptor( - url, std::move(interceptor)); +void CreateHangingFirstRequestInterceptor( + const GURL& url, + const base::FilePath& file, + base::Callback<void(net::URLRequest*)> callback_io) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + content::BrowserThread::PostTask( + content::BrowserThread::IO, FROM_HERE, + base::Bind(&CreateHangingFirstRequestInterceptorOnIO, url, file, + callback_io)); } } // namespace test_utils
diff --git a/chrome/browser/prerender/prerender_test_utils.h b/chrome/browser/prerender/prerender_test_utils.h index cdc266b..55d889b 100644 --- a/chrome/browser/prerender/prerender_test_utils.h +++ b/chrome/browser/prerender/prerender_test_utils.h
@@ -433,10 +433,12 @@ void CreateMockInterceptorOnIO(const GURL& url, const base::FilePath& file); // Makes |url| never respond on the first load, and then with the contents of -// |file| afterwards. When the first load has been scheduled, runs |callback| on -// the UI thread. -void CreateHangingFirstRequestInterceptorOnIO( - const GURL& url, const base::FilePath& file, base::Closure callback); +// |file| afterwards. When the first load has been scheduled, runs |callback_io| +// on the IO thread. +void CreateHangingFirstRequestInterceptor( + const GURL& url, + const base::FilePath& file, + base::Callback<void(net::URLRequest*)> callback_io); } // namespace test_utils
diff --git a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm index 5f158f2..13549db 100644 --- a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm +++ b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
@@ -505,12 +505,6 @@ // (URLDropTarget protocol) - (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)sender { - // Make ourself the first responder, which will select the text to indicate - // that our contents would be replaced by a drop. - // TODO(viettrungluu): crbug.com/30809 -- this is a hack since it steals focus - // and doesn't return it. - [[self window] makeFirstResponder:self]; - bool canDropAtLocation = [[self cell] canDropAtLocationInWindow:[sender draggingLocation] ofView:self];
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index 90ed1ce..ccd64e4 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -498,6 +498,12 @@ http_header_interceptor_map_[http_header] = interceptor_info; } +void ResourceDispatcherHostImpl::ReprioritizeRequest( + net::URLRequest* request, + net::RequestPriority priority) { + scheduler_->ReprioritizeRequest(request, priority); +} + void ResourceDispatcherHostImpl::Shutdown() { DCHECK(main_thread_task_runner_->BelongsToCurrentThread()); io_thread_task_runner_->PostTask(
diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h index cbb6e11..20679dd9 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.h +++ b/content/browser/loader/resource_dispatcher_host_impl.h
@@ -108,6 +108,8 @@ void RegisterInterceptor(const std::string& http_header, const std::string& starts_with, const InterceptorCallback& interceptor) override; + void ReprioritizeRequest(net::URLRequest* request, + net::RequestPriority priority) override; // Puts the resource dispatcher host in an inactive state (unable to begin // new requests). Cancels all pending requests.
diff --git a/content/browser/loader/resource_scheduler.cc b/content/browser/loader/resource_scheduler.cc index e1dcb0a..d653c724 100644 --- a/content/browser/loader/resource_scheduler.cc +++ b/content/browser/loader/resource_scheduler.cc
@@ -237,8 +237,9 @@ } static ScheduledResourceRequest* ForRequest(net::URLRequest* request) { - return static_cast<UnownedPointer*>(request->GetUserData(kUserDataKey)) - ->get(); + UnownedPointer* pointer = + static_cast<UnownedPointer*>(request->GetUserData(kUserDataKey)); + return pointer ? pointer->get() : nullptr; } // Starts the request. If |start_mode| is START_ASYNC, the request will not @@ -1080,6 +1081,17 @@ new_priority_params); } +void ResourceScheduler::ReprioritizeRequest(net::URLRequest* request, + net::RequestPriority new_priority) { + int current_intra_priority = 0; + auto* existing_request = ScheduledResourceRequest::ForRequest(request); + if (existing_request) { + current_intra_priority = + existing_request->get_request_priority_params().intra_priority; + } + ReprioritizeRequest(request, new_priority, current_intra_priority); +} + ResourceScheduler::ClientId ResourceScheduler::MakeClientId( int child_id, int route_id) { return (static_cast<ResourceScheduler::ClientId>(child_id) << 32) | route_id;
diff --git a/content/browser/loader/resource_scheduler.h b/content/browser/loader/resource_scheduler.h index bcb3213a3..71014ce 100644 --- a/content/browser/loader/resource_scheduler.h +++ b/content/browser/loader/resource_scheduler.h
@@ -97,11 +97,16 @@ // Returns true if at least one client is currently loading. bool HasLoadingClients() const; - // Update the priority for |request|. Modifies request->priority(), and may + // Updates the priority for |request|. Modifies request->priority(), and may // start the request loading if it wasn't already started. + // If the scheduler does not know about the request, |new_priority| is set but + // |intra_priority_value| is ignored. void ReprioritizeRequest(net::URLRequest* request, net::RequestPriority new_priority, int intra_priority_value); + // Same as above, but keeps the existing intra priority value. + void ReprioritizeRequest(net::URLRequest* request, + net::RequestPriority new_priority); private: // Returns the maximum number of delayable requests to all be in-flight at
diff --git a/content/public/browser/resource_dispatcher_host.h b/content/public/browser/resource_dispatcher_host.h index 4838b48..6f2f26f 100644 --- a/content/public/browser/resource_dispatcher_host.h +++ b/content/public/browser/resource_dispatcher_host.h
@@ -12,6 +12,7 @@ #include "base/callback_forward.h" #include "content/common/content_export.h" +#include "net/base/request_priority.h" namespace net { class URLRequest; @@ -84,6 +85,11 @@ const std::string& starts_with, const InterceptorCallback& interceptor) = 0; + // Updates the priority for |request|. Modifies request->priority(), and may + // start the request loading if it wasn't already started. + virtual void ReprioritizeRequest(net::URLRequest* request, + net::RequestPriority priority) = 0; + protected: virtual ~ResourceDispatcherHost() {} };
diff --git a/extensions/browser/notification_types.h b/extensions/browser/notification_types.h index 5c63f8a..ff00ecd 100644 --- a/extensions/browser/notification_types.h +++ b/extensions/browser/notification_types.h
@@ -34,12 +34,6 @@ // DEPRECATED: Use ExtensionSystem::Get(browser_context)->ready().Post(). NOTIFICATION_EXTENSIONS_READY_DEPRECATED, - // Sent when a new extension is loaded. The details are an Extension, and - // the source is a BrowserContext*. - // - // DEPRECATED: Use ExtensionRegistry::AddObserver instead. - NOTIFICATION_EXTENSION_LOADED_DEPRECATED, - // An error occured while attempting to load an extension. The details are a // string with details about why the load failed. NOTIFICATION_EXTENSION_LOAD_ERROR,
diff --git a/extensions/shell/browser/shell_extension_system.cc b/extensions/shell/browser/shell_extension_system.cc index b1497c1..992ab8f2 100644 --- a/extensions/shell/browser/shell_extension_system.cc +++ b/extensions/shell/browser/shell_extension_system.cc
@@ -82,11 +82,6 @@ RendererStartupHelperFactory::GetForBrowserContext(browser_context_) ->OnExtensionLoaded(*extension); - content::NotificationService::current()->Notify( - NOTIFICATION_EXTENSION_LOADED_DEPRECATED, - content::Source<BrowserContext>(browser_context_), - content::Details<const Extension>(extension.get())); - return extension.get(); }
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm index 19dbe62a..6a83d8f 100644 --- a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm +++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
@@ -234,6 +234,27 @@ completion:completion]; } +- (void)fetchFaviconImageForSuggestion:(ContentSuggestionIdentifier*)suggestion + completion:(void (^)(UIImage*))completion { + if (!completion) + return; + + void (^imageCallback)(const gfx::Image&) = ^(const gfx::Image& image) { + if (!image.IsEmpty()) { + completion([image.ToUIImage() copy]); + } + }; + + ntp_snippets::ContentSuggestion::ID identifier = + ntp_snippets::ContentSuggestion::ID( + [[self categoryWrapperForSectionInfo:suggestion.sectionInfo] + category], + suggestion.IDInSection); + self.contentService->FetchSuggestionFavicon( + identifier, /* minimum_size_in_pixel = */ 1, kDefaultFaviconSize, + base::BindBlockArc(imageCallback)); +} + #pragma mark - ContentSuggestionsServiceObserver - (void)contentSuggestionsService:
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm index 00daaae7..a775e46 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
@@ -476,6 +476,18 @@ weakItem.attributes = attributes; }]; + __weak ContentSuggestionsCollectionUpdater* weakSelf = self; + [self.dataSource + fetchFaviconImageForSuggestion:articleItem.suggestionIdentifier + completion:^void(UIImage* favicon) { + if (!weakItem || !weakSelf) + return; + + weakItem.attributes = + [FaviconAttributes attributesWithImage:favicon]; + [weakSelf reconfigure:weakItem]; + }]; + return articleItem; }
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h index fc7bfe1..671b7f1 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
@@ -40,6 +40,14 @@ (void (^_Nonnull)(FaviconAttributes* _Nonnull)) completion; +// Fetches favicon image associated with this |suggestion| in history. If it is +// not present in the history, tries to download it. Calls the completion block +// if an image has been found. +// This can only be used for public URL. +- (void) +fetchFaviconImageForSuggestion:(nonnull ContentSuggestionIdentifier*)suggestion + completion:(void (^_Nonnull)(UIImage* _Nonnull))completion; + // Fetches additional content. All the |knownSuggestions| must come from the // same |sectionInfo|. If the fetch was completed, the given |callback| is // called with the new content.
diff --git a/ios/clean/chrome/app/steps/BUILD.gn b/ios/clean/chrome/app/steps/BUILD.gn index e87ff249..d5cc421 100644 --- a/ios/clean/chrome/app/steps/BUILD.gn +++ b/ios/clean/chrome/app/steps/BUILD.gn
@@ -27,6 +27,7 @@ "//ios/chrome/browser/browser_state:browser_state_impl", "//ios/chrome/browser/content_settings", "//ios/chrome/browser/web:web_internal", + "//ios/chrome/browser/web_state_list", "//ios/clean/chrome/app:application_state", "//ios/clean/chrome/browser/ui/root", "//ios/net",
diff --git a/ios/clean/chrome/app/steps/root_coordinator+application_step.mm b/ios/clean/chrome/app/steps/root_coordinator+application_step.mm index 73da4e99..a0d3af9d 100644 --- a/ios/clean/chrome/app/steps/root_coordinator+application_step.mm +++ b/ios/clean/chrome/app/steps/root_coordinator+application_step.mm
@@ -5,10 +5,14 @@ #import "ios/clean/chrome/app/steps/root_coordinator+application_step.h" #import "base/supports_user_data.h" +#include "ios/chrome/browser/browser_state/chrome_browser_state.h" +#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/clean/chrome/app/application_state.h" #import "ios/shared/chrome/browser/ui/browser_list/browser.h" #import "ios/shared/chrome/browser/ui/browser_list/browser_list.h" #import "ios/shared/chrome/browser/ui/coordinators/browser_coordinator+internal.h" +#import "ios/web/public/navigation_manager.h" +#include "ios/web/public/web_state/web_state.h" #if !defined(__has_feature) || !__has_feature(objc_arc) #error "This file requires ARC support." @@ -43,6 +47,18 @@ - (void)runInState:(ApplicationState*)state { self.browser = BrowserList::FromBrowserState(state.browserState)->CreateNewBrowser(); + + // PLACEHOLDER: Create some open, empty web states. + WebStateList& webStateList = self.browser->web_state_list(); + for (int i = 0; i < 7; i++) { + web::WebState::CreateParams webStateCreateParams( + self.browser->browser_state()); + std::unique_ptr<web::WebState> webState = + web::WebState::Create(webStateCreateParams); + webStateList.InsertWebState(0, std::move(webState)); + } + webStateList.ActivateWebStateAt(0); + [self start]; state.window.rootViewController = self.viewController;
diff --git a/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm b/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm index 1962c7a..bfdc1c50 100644 --- a/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm +++ b/ios/clean/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm
@@ -47,18 +47,6 @@ #pragma mark - Properties -- (void)setBrowser:(Browser*)browser { - [super setBrowser:browser]; - - for (int i = 0; i < 7; i++) { - web::WebState::CreateParams webStateCreateParams(browser->browser_state()); - std::unique_ptr<web::WebState> webState = - web::WebState::Create(webStateCreateParams); - self.webStateList.InsertWebState(0, std::move(webState)); - } - self.webStateList.ActivateWebStateAt(0); -} - - (WebStateList&)webStateList { return self.browser->web_state_list(); }
diff --git a/ios/clean/chrome/browser/ui/web_contents/web_contents_mediator.mm b/ios/clean/chrome/browser/ui/web_contents/web_contents_mediator.mm index 46d12f9..447209e 100644 --- a/ios/clean/chrome/browser/ui/web_contents/web_contents_mediator.mm +++ b/ios/clean/chrome/browser/ui/web_contents/web_contents_mediator.mm
@@ -100,7 +100,7 @@ } } -// PLACEHOLDER: This navigates the page since the omnibox is not yet hooked up. +// PLACEHOLDER: This navigates an empty webstate to the NTP. - (void)navigateToDefaultPage:(web::WebState*)webState { if (!webState->GetNavigationManager()->GetItemCount()) { web::NavigationManager::WebLoadParams params((GURL(kChromeUINewTabURL)));
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc index cb985f5..79ae584 100644 --- a/net/http/http_response_headers.cc +++ b/net/http/http_response_headers.cc
@@ -580,9 +580,6 @@ return FindHeader(0, name) != std::string::npos; } -HttpResponseHeaders::HttpResponseHeaders() : response_code_(-1) { -} - HttpResponseHeaders::~HttpResponseHeaders() { }
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h index d5392b3..be15c27 100644 --- a/net/http/http_response_headers.h +++ b/net/http/http_response_headers.h
@@ -54,6 +54,8 @@ static const char kContentRange[]; + HttpResponseHeaders() = delete; + // Parses the given raw_headers. raw_headers should be formatted thus: // includes the http status response line, each line is \0-terminated, and // it's terminated by an empty line (ie, 2 \0s in a row). @@ -302,7 +304,6 @@ struct ParsedHeader; typedef std::vector<ParsedHeader> HeaderList; - HttpResponseHeaders(); ~HttpResponseHeaders(); // Initializes from the given raw headers.
diff --git a/net/http2/decoder/decode_http2_structures_test.cc b/net/http2/decoder/decode_http2_structures_test.cc index 71cd286..09e6b56 100644 --- a/net/http2/decoder/decode_http2_structures_test.cc +++ b/net/http2/decoder/decode_http2_structures_test.cc
@@ -120,7 +120,7 @@ if (!HasFailure()) { EXPECT_EQ(5u, structure_.payload_length); EXPECT_EQ(Http2FrameType::HEADERS, structure_.type); - EXPECT_EQ(Http2FrameFlag::FLAG_PADDED, structure_.flags); + EXPECT_EQ(Http2FrameFlag::PADDED, structure_.flags); EXPECT_EQ(1u, structure_.stream_id); } }
diff --git a/net/http2/decoder/http2_frame_decoder.cc b/net/http2/decoder/http2_frame_decoder.cc index 46275027..b0184a61 100644 --- a/net/http2/decoder/http2_frame_decoder.cc +++ b/net/http2/decoder/http2_frame_decoder.cc
@@ -264,7 +264,7 @@ DecodeStatus Http2FrameDecoder::StartDecodingContinuationPayload( DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_END_HEADERS); + RetainFlags(Http2FrameFlag::END_HEADERS); return continuation_payload_decoder_.StartDecodingPayload( &frame_decoder_state_, db); } @@ -278,7 +278,7 @@ } DecodeStatus Http2FrameDecoder::StartDecodingDataPayload(DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_PADDED); + RetainFlags(Http2FrameFlag::END_STREAM | Http2FrameFlag::PADDED); return data_payload_decoder_.StartDecodingPayload(&frame_decoder_state_, db); } DecodeStatus Http2FrameDecoder::ResumeDecodingDataPayload(DecodeBuffer* db) { @@ -299,9 +299,8 @@ } DecodeStatus Http2FrameDecoder::StartDecodingHeadersPayload(DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_END_STREAM | - Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED | - Http2FrameFlag::FLAG_PRIORITY); + RetainFlags(Http2FrameFlag::END_STREAM | Http2FrameFlag::END_HEADERS | + Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY); return headers_payload_decoder_.StartDecodingPayload(&frame_decoder_state_, db); } @@ -313,7 +312,7 @@ } DecodeStatus Http2FrameDecoder::StartDecodingPingPayload(DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_ACK); + RetainFlags(Http2FrameFlag::ACK); return ping_payload_decoder_.StartDecodingPayload(&frame_decoder_state_, db); } DecodeStatus Http2FrameDecoder::ResumeDecodingPingPayload(DecodeBuffer* db) { @@ -339,7 +338,7 @@ DecodeStatus Http2FrameDecoder::StartDecodingPushPromisePayload( DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED); + RetainFlags(Http2FrameFlag::END_HEADERS | Http2FrameFlag::PADDED); return push_promise_payload_decoder_.StartDecodingPayload( &frame_decoder_state_, db); } @@ -367,7 +366,7 @@ } DecodeStatus Http2FrameDecoder::StartDecodingSettingsPayload(DecodeBuffer* db) { - RetainFlags(Http2FrameFlag::FLAG_ACK); + RetainFlags(Http2FrameFlag::ACK); return settings_payload_decoder_.StartDecodingPayload(&frame_decoder_state_, db); }
diff --git a/net/http2/decoder/http2_frame_decoder_test.cc b/net/http2/decoder/http2_frame_decoder_test.cc index 6008947..28e2ef26 100644 --- a/net/http2/decoder/http2_frame_decoder_test.cc +++ b/net/http2/decoder/http2_frame_decoder_test.cc
@@ -297,8 +297,7 @@ 0x01, // Flags: ACK 0x00, 0x00, 0x00, 0x00, // Stream: 0 }; - Http2FrameHeader header(0, Http2FrameType::SETTINGS, Http2FrameFlag::FLAG_ACK, - 0); + Http2FrameHeader header(0, Http2FrameType::SETTINGS, Http2FrameFlag::ACK, 0); FrameParts expected(header); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); } @@ -312,7 +311,7 @@ 0x00, 0x00, 0x00, 0x01, // Promised: 1 (invalid but unchecked here) }; Http2FrameHeader header(4, Http2FrameType::PUSH_PROMISE, - Http2FrameFlag::FLAG_END_HEADERS, 2); + Http2FrameFlag::END_HEADERS, 2); FrameParts expected(header, ""); expected.opt_push_promise = Http2PushPromiseFields{1}; EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -342,7 +341,7 @@ 's', 'o', 'm', 'e', // "some" 'd', 'a', 't', 'a', // "data" }; - Http2FrameHeader header(8, Http2FrameType::PING, Http2FrameFlag::FLAG_ACK, 0); + Http2FrameHeader header(8, Http2FrameType::PING, Http2FrameFlag::ACK, 0); FrameParts expected(header); expected.opt_ping = Http2PingFields{{'s', 'o', 'm', 'e', 'd', 'a', 't', 'a'}}; EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -443,7 +442,7 @@ }; Http2FrameHeader header( 3, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_END_HEADERS, 2); + Http2FrameFlag::END_STREAM | Http2FrameFlag::END_HEADERS, 2); FrameParts expected(header, "abc"); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); } @@ -457,8 +456,8 @@ 0x00, 0x00, 0x00, 0x01, // Parent: 1 (Not Exclusive) 0xffu, // Weight: 256 }; - Http2FrameHeader header(5, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_PRIORITY, 2); + Http2FrameHeader header(5, Http2FrameType::HEADERS, Http2FrameFlag::PRIORITY, + 2); FrameParts expected(header); expected.opt_priority = Http2PriorityFields(1, 256, false); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -494,7 +493,7 @@ 'a', 'b', 'c', // HPACK fragment (doesn't have to be valid) }; Http2FrameHeader header(7, Http2FrameType::PUSH_PROMISE, - Http2FrameFlag::FLAG_END_HEADERS, 255); + Http2FrameFlag::END_HEADERS, 255); FrameParts expected(header, "abc"); expected.opt_push_promise = Http2PushPromiseFields{256}; EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -526,7 +525,7 @@ 'a', 'b', 'c', // Data }; Http2FrameHeader header(3, Http2FrameType::CONTINUATION, - Http2FrameFlag::FLAG_END_HEADERS, 2); + Http2FrameFlag::END_HEADERS, 2); FrameParts expected(header, "abc"); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); } @@ -573,9 +572,9 @@ 'a', 'b', 'c', // Data 0x00, 0x00, 0x00, // Padding }; - Http2FrameHeader header( - 7, Http2FrameType::DATA, - Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_PADDED, 2); + Http2FrameHeader header(7, Http2FrameType::DATA, + Http2FrameFlag::END_STREAM | Http2FrameFlag::PADDED, + 2); size_t total_pad_length = 4; // Including the Pad Length field. FrameParts expected(header, "abc", total_pad_length); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -591,8 +590,8 @@ 'a', 'b', 'c', // HPACK fragment (doesn't have to be valid) 0x00, 0x00, 0x00, // Padding }; - Http2FrameHeader header(7, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_PADDED, 2); + Http2FrameHeader header(7, Http2FrameType::HEADERS, Http2FrameFlag::PADDED, + 2); size_t total_pad_length = 4; // Including the Pad Length field. FrameParts expected(header, "abc", total_pad_length); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays(kFrameData, expected)); @@ -610,11 +609,11 @@ 'a', 'b', 'c', // HPACK fragment (doesn't have to be valid) 0x00, 0x00, 0x00, // Padding }; - Http2FrameHeader header( - 12, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_END_HEADERS | - Http2FrameFlag::FLAG_PADDED | Http2FrameFlag::FLAG_PRIORITY, - 2); + Http2FrameHeader header(12, Http2FrameType::HEADERS, + Http2FrameFlag::END_STREAM | + Http2FrameFlag::END_HEADERS | + Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY, + 2); size_t total_pad_length = 4; // Including the Pad Length field. FrameParts expected(header, "abc", total_pad_length); expected.opt_priority = Http2PriorityFields(1, 17, true); @@ -632,9 +631,9 @@ 'a', 'b', 'c', // HPACK fragment (doesn't have to be valid) 0x00, 0x00, 0x00, // Padding }; - Http2FrameHeader header( - 11, Http2FrameType::PUSH_PROMISE, - Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED, 1); + Http2FrameHeader header(11, Http2FrameType::PUSH_PROMISE, + Http2FrameFlag::END_HEADERS | Http2FrameFlag::PADDED, + 1); size_t total_pad_length = 4; // Including the Pad Length field. FrameParts expected(header, "abc", total_pad_length); expected.opt_push_promise = Http2PushPromiseFields{2}; @@ -651,8 +650,7 @@ 0x08, // Flags: PADDED 0x00, 0x00, 0x00, 0x01, // Stream ID: 1 }; - Http2FrameHeader header(0, Http2FrameType::DATA, Http2FrameFlag::FLAG_PADDED, - 1); + Http2FrameHeader header(0, Http2FrameType::DATA, Http2FrameFlag::PADDED, 1); FrameParts expected(header); expected.opt_missing_length = 1; EXPECT_TRUE(DecodePayloadExpectingError(kFrameData, expected)); @@ -667,8 +665,8 @@ 0xffu, // Pad Len: 255 0x00, // Only one byte of padding }; - Http2FrameHeader header(2, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_PADDED, 65536); + Http2FrameHeader header(2, Http2FrameType::HEADERS, Http2FrameFlag::PADDED, + 65536); FrameParts expected(header); expected.opt_missing_length = 254; EXPECT_TRUE(DecodePayloadExpectingError(kFrameData, expected)); @@ -682,8 +680,8 @@ 0x00, 0x01, 0x00, 0x00, // Stream ID: 65536 0x00, 0x00, 0x00, 0x00, // Priority (truncated) }; - Http2FrameHeader header(4, Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_PRIORITY, 65536); + Http2FrameHeader header(4, Http2FrameType::HEADERS, Http2FrameFlag::PRIORITY, + 65536); EXPECT_TRUE(DecodePayloadExpectingFrameSizeError(kFrameData, header)); } @@ -753,7 +751,7 @@ 0x00, 0x00, 0x00, // Truncated promise id }; Http2FrameHeader header(4, Http2FrameType::PUSH_PROMISE, - Http2FrameFlag::FLAG_PADDED, 1); + Http2FrameFlag::PADDED, 1); EXPECT_TRUE(DecodePayloadExpectingFrameSizeError(kFrameData, header)); } @@ -834,9 +832,9 @@ 'a', 'b', 'c', // Data 0x00, 0x00, 0x00, // Padding }; - Http2FrameHeader header( - 7, Http2FrameType::DATA, - Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_PADDED, 2); + Http2FrameHeader header(7, Http2FrameType::DATA, + Http2FrameFlag::END_STREAM | Http2FrameFlag::PADDED, + 2); FrameParts expected(header); expected.has_frame_size_error = true; auto validator = [&expected, this](const DecodeBuffer& input, @@ -890,8 +888,7 @@ 0x00, 0x00, // Extra 0x00, 0x00, 0x00, 0x00, // Extra }; - Http2FrameHeader header(6, Http2FrameType::SETTINGS, Http2FrameFlag::FLAG_ACK, - 0); + Http2FrameHeader header(6, Http2FrameType::SETTINGS, Http2FrameFlag::ACK, 0); EXPECT_TRUE(DecodePayloadExpectingFrameSizeError(kFrameData, header)); } @@ -905,7 +902,7 @@ 'd', 'a', 't', 'a', // "data" 0x00, // Too much }; - Http2FrameHeader header(9, Http2FrameType::PING, Http2FrameFlag::FLAG_ACK, 0); + Http2FrameHeader header(9, Http2FrameType::PING, Http2FrameFlag::ACK, 0); EXPECT_TRUE(DecodePayloadExpectingFrameSizeError(kFrameData, header)); }
diff --git a/net/http2/decoder/http2_structure_decoder_test.cc b/net/http2/decoder/http2_structure_decoder_test.cc index 06008b8..477da7f 100644 --- a/net/http2/decoder/http2_structure_decoder_test.cc +++ b/net/http2/decoder/http2_structure_decoder_test.cc
@@ -208,7 +208,7 @@ ASSERT_TRUE(DecodeLeadingStructure(kData)); EXPECT_EQ(5u, structure_.payload_length); EXPECT_EQ(Http2FrameType::HEADERS, structure_.type); - EXPECT_EQ(Http2FrameFlag::FLAG_PADDED, structure_.flags); + EXPECT_EQ(Http2FrameFlag::PADDED, structure_.flags); EXPECT_EQ(1u, structure_.stream_id); } {
diff --git a/net/http2/decoder/payload_decoders/continuation_payload_decoder.cc b/net/http2/decoder/payload_decoders/continuation_payload_decoder.cc index ac5371c0..e84316ad2 100644 --- a/net/http2/decoder/payload_decoders/continuation_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/continuation_payload_decoder.cc
@@ -24,7 +24,7 @@ << frame_header; DCHECK_EQ(Http2FrameType::CONTINUATION, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::FLAG_END_HEADERS)); + DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::END_HEADERS)); state->InitializeRemainders(); state->listener()->OnContinuationStart(frame_header);
diff --git a/net/http2/decoder/payload_decoders/data_payload_decoder.cc b/net/http2/decoder/payload_decoders/data_payload_decoder.cc index 794973b..3324140 100644 --- a/net/http2/decoder/payload_decoders/data_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/data_payload_decoder.cc
@@ -41,9 +41,8 @@ DVLOG(2) << "DataPayloadDecoder::StartDecodingPayload: " << frame_header; DCHECK_EQ(Http2FrameType::DATA, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ( - 0, frame_header.flags & - ~(Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_PADDED)); + DCHECK_EQ(0, frame_header.flags & + ~(Http2FrameFlag::END_STREAM | Http2FrameFlag::PADDED)); // Special case for the hoped for common case: unpadded and fits fully into // the decode buffer. TO BE SEEN if that is true. It certainly requires that
diff --git a/net/http2/decoder/payload_decoders/data_payload_decoder_test.cc b/net/http2/decoder/payload_decoders/data_payload_decoder_test.cc index d8c314a7..c6dabad2 100644 --- a/net/http2/decoder/payload_decoders/data_payload_decoder_test.cc +++ b/net/http2/decoder/payload_decoders/data_payload_decoder_test.cc
@@ -37,7 +37,7 @@ // Returns the mask of flags that affect the decoding of the payload (i.e. // flags that that indicate the presence of certain fields or padding). static constexpr uint8_t FlagsAffectingPayloadDecoding() { - return Http2FrameFlag::FLAG_PADDED; + return Http2FrameFlag::PADDED; } static void Randomize(DataPayloadDecoder* p, RandomBase* rng) {
diff --git a/net/http2/decoder/payload_decoders/headers_payload_decoder.cc b/net/http2/decoder/payload_decoders/headers_payload_decoder.cc index b09bf63c..349117a0 100644 --- a/net/http2/decoder/payload_decoders/headers_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/headers_payload_decoder.cc
@@ -47,11 +47,9 @@ DCHECK_EQ(Http2FrameType::HEADERS, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ( - 0, - frame_header.flags & - ~(Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_END_HEADERS | - Http2FrameFlag::FLAG_PADDED | Http2FrameFlag::FLAG_PRIORITY)); + DCHECK_EQ(0, frame_header.flags & + ~(Http2FrameFlag::END_STREAM | Http2FrameFlag::END_HEADERS | + Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY)); // Special case for HEADERS frames that contain only the HPACK block // (fragment or whole) and that fit fully into the decode buffer. @@ -66,8 +64,7 @@ // PADDED and PRIORITY both extra steps to decode, but if neither flag is // set then we can decode faster. - const auto payload_flags = - Http2FrameFlag::FLAG_PADDED | Http2FrameFlag::FLAG_PRIORITY; + const auto payload_flags = Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY; if (!frame_header.HasAnyFlags(payload_flags)) { DVLOG(2) << "StartDecodingPayload !IsPadded && !HasPriority"; if (db->Remaining() == total_length) {
diff --git a/net/http2/decoder/payload_decoders/headers_payload_decoder_test.cc b/net/http2/decoder/payload_decoders/headers_payload_decoder_test.cc index 227047c..0fdce71 100644 --- a/net/http2/decoder/payload_decoders/headers_payload_decoder_test.cc +++ b/net/http2/decoder/payload_decoders/headers_payload_decoder_test.cc
@@ -34,7 +34,7 @@ // Returns the mask of flags that affect the decoding of the payload (i.e. // flags that that indicate the presence of certain fields or padding). static constexpr uint8_t FlagsAffectingPayloadDecoding() { - return Http2FrameFlag::FLAG_PADDED | Http2FrameFlag::FLAG_PRIORITY; + return Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY; } static void Randomize(HeadersPayloadDecoder* p, RandomBase* rng) { @@ -121,7 +121,7 @@ ASSERT_EQ(IsPadded() ? 1u : 0u, frame_builder_.size()); uint8_t flags = RandFlags(); if (has_priority) { - flags |= Http2FrameFlag::FLAG_PRIORITY; + flags |= Http2FrameFlag::PRIORITY; frame_builder_.Append(priority); } @@ -154,8 +154,7 @@ fb.Append(Http2PriorityFields(RandStreamId(), 1 + Random().Rand8(), Random().OneIn(2))); EXPECT_TRUE(VerifyDetectsMultipleFrameSizeErrors( - Http2FrameFlag::FLAG_PRIORITY, fb.buffer(), approve_size, - total_pad_length_)); + Http2FrameFlag::PRIORITY, fb.buffer(), approve_size, total_pad_length_)); } // Confirm we get an error if the PADDED flag is set but the payload is not
diff --git a/net/http2/decoder/payload_decoders/payload_decoder_base_test_util.h b/net/http2/decoder/payload_decoders/payload_decoder_base_test_util.h index 3654278..6c7fcc5 100644 --- a/net/http2/decoder/payload_decoders/payload_decoder_base_test_util.h +++ b/net/http2/decoder/payload_decoders/payload_decoder_base_test_util.h
@@ -266,13 +266,13 @@ VERIFY_EQ(required_flags, required_flags & DecoderPeer::FlagsAffectingPayloadDecoding()); - if (0 != (Http2FrameFlag::FLAG_PADDED & - KnownFlagsMaskForFrameType(frame_type))) { + if (0 != + (Http2FrameFlag::PADDED & KnownFlagsMaskForFrameType(frame_type))) { // Frame type supports padding. if (total_pad_length == 0) { - required_flags &= ~Http2FrameFlag::FLAG_PADDED; + required_flags &= ~Http2FrameFlag::PADDED; } else { - required_flags |= Http2FrameFlag::FLAG_PADDED; + required_flags |= Http2FrameFlag::PADDED; } } else { VERIFY_EQ(0, total_pad_length); @@ -316,8 +316,8 @@ ApproveSize approve_size) { Http2FrameType frame_type = DecoderPeer::FrameType(); uint8_t known_flags = KnownFlagsMaskForFrameType(frame_type); - VERIFY_EQ(0, known_flags & Http2FrameFlag::FLAG_PADDED); - VERIFY_EQ(0, required_flags & Http2FrameFlag::FLAG_PADDED); + VERIFY_EQ(0, known_flags & Http2FrameFlag::PADDED); + VERIFY_EQ(0, required_flags & Http2FrameFlag::PADDED); VERIFY_AND_RETURN_SUCCESS(VerifyDetectsMultipleFrameSizeErrors( required_flags, unpadded_payload, approve_size, 0)); } @@ -380,9 +380,9 @@ uint8_t RandFlags() { uint8_t flags = Base::RandFlags(); if (IsPadded()) { - flags |= Http2FrameFlag::FLAG_PADDED; + flags |= Http2FrameFlag::PADDED; } else { - flags &= ~Http2FrameFlag::FLAG_PADDED; + flags &= ~Http2FrameFlag::PADDED; } return flags; } @@ -421,7 +421,7 @@ // Flags will be selected at random, except PADDED will be set and // flags_to_avoid will not be set. The stream id is selected at random. ::testing::AssertionResult VerifyDetectsPaddingTooLong() { - uint8_t flags = RandFlags() | Http2FrameFlag::FLAG_PADDED; + uint8_t flags = RandFlags() | Http2FrameFlag::PADDED; // Create an all padding payload for total_pad_length_. int payload_length = 0;
diff --git a/net/http2/decoder/payload_decoders/ping_payload_decoder.cc b/net/http2/decoder/payload_decoders/ping_payload_decoder.cc index d8edbb0..248977286 100644 --- a/net/http2/decoder/payload_decoders/ping_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/ping_payload_decoder.cc
@@ -21,7 +21,7 @@ DVLOG(2) << "PingPayloadDecoder::StartDecodingPayload: " << frame_header; DCHECK_EQ(Http2FrameType::PING, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::FLAG_ACK)); + DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::ACK)); // Is the payload entirely in the decode buffer and is it the correct size? // Given the size of the header and payload (17 bytes total), this is most
diff --git a/net/http2/decoder/payload_decoders/ping_payload_decoder_test.cc b/net/http2/decoder/payload_decoders/ping_payload_decoder_test.cc index aefc2820..b280d69 100644 --- a/net/http2/decoder/payload_decoders/ping_payload_decoder_test.cc +++ b/net/http2/decoder/payload_decoders/ping_payload_decoder_test.cc
@@ -87,8 +87,7 @@ Http2FrameBuilder fb; fb.Append(fields); Http2FrameHeader header(fb.size(), Http2FrameType::PING, - RandFlags() & ~Http2FrameFlag::FLAG_ACK, - RandStreamId()); + RandFlags() & ~Http2FrameFlag::ACK, RandStreamId()); set_frame_header(header); FrameParts expected(header); expected.opt_ping = fields; @@ -103,8 +102,7 @@ Http2FrameBuilder fb; fb.Append(fields); Http2FrameHeader header(fb.size(), Http2FrameType::PING, - RandFlags() | Http2FrameFlag::FLAG_ACK, - RandStreamId()); + RandFlags() | Http2FrameFlag::ACK, RandStreamId()); set_frame_header(header); FrameParts expected(header); expected.opt_ping = fields;
diff --git a/net/http2/decoder/payload_decoders/push_promise_payload_decoder.cc b/net/http2/decoder/payload_decoders/push_promise_payload_decoder.cc index f160d1e..0cd36d7 100644 --- a/net/http2/decoder/payload_decoders/push_promise_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/push_promise_payload_decoder.cc
@@ -46,9 +46,8 @@ DCHECK_EQ(Http2FrameType::PUSH_PROMISE, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ( - 0, frame_header.flags & - ~(Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED)); + DCHECK_EQ(0, frame_header.flags & + ~(Http2FrameFlag::END_HEADERS | Http2FrameFlag::PADDED)); if (!frame_header.IsPadded()) { // If it turns out that PUSH_PROMISE frames without padding are sufficiently
diff --git a/net/http2/decoder/payload_decoders/push_promise_payload_decoder_test.cc b/net/http2/decoder/payload_decoders/push_promise_payload_decoder_test.cc index 1b3f642..f4b9340 100644 --- a/net/http2/decoder/payload_decoders/push_promise_payload_decoder_test.cc +++ b/net/http2/decoder/payload_decoders/push_promise_payload_decoder_test.cc
@@ -36,7 +36,7 @@ // Returns the mask of flags that affect the decoding of the payload (i.e. // flags that that indicate the presence of certain fields or padding). static constexpr uint8_t FlagsAffectingPayloadDecoding() { - return Http2FrameFlag::FLAG_PADDED; + return Http2FrameFlag::PADDED; } static void Randomize(PushPromisePayloadDecoder* p, RandomBase* rng) {
diff --git a/net/http2/decoder/payload_decoders/settings_payload_decoder.cc b/net/http2/decoder/payload_decoders/settings_payload_decoder.cc index f8cc52cb..496fe27 100644 --- a/net/http2/decoder/payload_decoders/settings_payload_decoder.cc +++ b/net/http2/decoder/payload_decoders/settings_payload_decoder.cc
@@ -21,7 +21,7 @@ DVLOG(2) << "SettingsPayloadDecoder::StartDecodingPayload: " << frame_header; DCHECK_EQ(Http2FrameType::SETTINGS, frame_header.type); DCHECK_LE(db->Remaining(), total_length); - DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::FLAG_ACK)); + DCHECK_EQ(0, frame_header.flags & ~(Http2FrameFlag::ACK)); if (frame_header.IsAck()) { if (total_length == 0) {
diff --git a/net/http2/decoder/payload_decoders/settings_payload_decoder_test.cc b/net/http2/decoder/payload_decoders/settings_payload_decoder_test.cc index ede51824..e801419 100644 --- a/net/http2/decoder/payload_decoders/settings_payload_decoder_test.cc +++ b/net/http2/decoder/payload_decoders/settings_payload_decoder_test.cc
@@ -33,7 +33,7 @@ // Returns the mask of flags that affect the decoding of the payload (i.e. // flags that that indicate the presence of certain fields or padding). static constexpr uint8_t FlagsAffectingPayloadDecoding() { - return Http2FrameFlag::FLAG_ACK; + return Http2FrameFlag::ACK; } static void Randomize(SettingsPayloadDecoder* p, RandomBase* rng) { @@ -107,7 +107,7 @@ fb.Append(RandSettingsFields()); fb.Append(RandSettingsFields()); fb.Append(RandSettingsFields()); - EXPECT_TRUE(VerifyDetectsFrameSizeError(Http2FrameFlag::FLAG_ACK, fb.buffer(), + EXPECT_TRUE(VerifyDetectsFrameSizeError(Http2FrameFlag::ACK, fb.buffer(), approve_size)); } @@ -115,7 +115,7 @@ TEST_F(SettingsPayloadDecoderTest, SettingsAck) { for (int stream_id = 0; stream_id < 3; ++stream_id) { Http2FrameHeader header(0, Http2FrameType::SETTINGS, - RandFlags() | Http2FrameFlag::FLAG_ACK, stream_id); + RandFlags() | Http2FrameFlag::ACK, stream_id); set_frame_header(header); FrameParts expected(header); EXPECT_TRUE(DecodePayloadAndValidateSeveralWays("", expected)); @@ -145,7 +145,7 @@ const size_t num_settings = 100; const size_t size = Http2SettingFields::EncodedSize() * num_settings; Http2FrameHeader header(size, Http2FrameType::SETTINGS, - RandFlags(), // & ~Http2FrameFlag::FLAG_ACK, + RandFlags(), // & ~Http2FrameFlag::ACK, RandStreamId()); set_frame_header(header); FrameParts expected(header);
diff --git a/net/http2/http2_constants.cc b/net/http2/http2_constants.cc index 82cdf03..dce2ccb 100644 --- a/net/http2/http2_constants.cc +++ b/net/http2/http2_constants.cc
@@ -62,28 +62,28 @@ }; if (flags & 0x01) { if (type == Http2FrameType::DATA || type == Http2FrameType::HEADERS) { - append_and_clear("END_STREAM", Http2FrameFlag::FLAG_END_STREAM); + append_and_clear("END_STREAM", Http2FrameFlag::END_STREAM); } else if (type == Http2FrameType::SETTINGS || type == Http2FrameType::PING) { - append_and_clear("ACK", Http2FrameFlag::FLAG_ACK); + append_and_clear("ACK", Http2FrameFlag::ACK); } } if (flags & 0x04) { if (type == Http2FrameType::HEADERS || type == Http2FrameType::PUSH_PROMISE || type == Http2FrameType::CONTINUATION) { - append_and_clear("END_HEADERS", Http2FrameFlag::FLAG_END_HEADERS); + append_and_clear("END_HEADERS", Http2FrameFlag::END_HEADERS); } } if (flags & 0x08) { if (type == Http2FrameType::DATA || type == Http2FrameType::HEADERS || type == Http2FrameType::PUSH_PROMISE) { - append_and_clear("PADDED", Http2FrameFlag::FLAG_PADDED); + append_and_clear("PADDED", Http2FrameFlag::PADDED); } } if (flags & 0x20) { if (type == Http2FrameType::HEADERS) { - append_and_clear("PRIORITY", Http2FrameFlag::FLAG_PRIORITY); + append_and_clear("PRIORITY", Http2FrameFlag::PRIORITY); } } if (flags != 0) {
diff --git a/net/http2/http2_constants.h b/net/http2/http2_constants.h index 61c6307..3b9c3c3c 100644 --- a/net/http2/http2_constants.h +++ b/net/http2/http2_constants.h
@@ -69,11 +69,11 @@ // TODO(bnc): Remove FLAG_ prefix once enum SpdyFrameType is removed // (both enums have a PRIORITY member). enum Http2FrameFlag { - FLAG_END_STREAM = 0x01, // DATA, HEADERS - FLAG_ACK = 0x01, // SETTINGS, PING - FLAG_END_HEADERS = 0x04, // HEADERS, PUSH_PROMISE, CONTINUATION - FLAG_PADDED = 0x08, // DATA, HEADERS, PUSH_PROMISE - FLAG_PRIORITY = 0x20, // HEADERS + END_STREAM = 0x01, // DATA, HEADERS + ACK = 0x01, // SETTINGS, PING + END_HEADERS = 0x04, // HEADERS, PUSH_PROMISE, CONTINUATION + PADDED = 0x08, // DATA, HEADERS, PUSH_PROMISE + PRIORITY = 0x20, // HEADERS }; // Formats zero or more flags for the specified type of frame. Returns an
diff --git a/net/http2/http2_constants_test.cc b/net/http2/http2_constants_test.cc index f7f24f7..5616d42 100644 --- a/net/http2/http2_constants_test.cc +++ b/net/http2/http2_constants_test.cc
@@ -57,31 +57,29 @@ } TEST(Http2ConstantsTest, Http2FrameFlag) { - EXPECT_EQ(Http2FrameFlag::FLAG_END_STREAM, static_cast<Http2FrameFlag>(0x01)); - EXPECT_EQ(Http2FrameFlag::FLAG_ACK, static_cast<Http2FrameFlag>(0x01)); - EXPECT_EQ(Http2FrameFlag::FLAG_END_HEADERS, - static_cast<Http2FrameFlag>(0x04)); - EXPECT_EQ(Http2FrameFlag::FLAG_PADDED, static_cast<Http2FrameFlag>(0x08)); - EXPECT_EQ(Http2FrameFlag::FLAG_PRIORITY, static_cast<Http2FrameFlag>(0x20)); + EXPECT_EQ(Http2FrameFlag::END_STREAM, static_cast<Http2FrameFlag>(0x01)); + EXPECT_EQ(Http2FrameFlag::ACK, static_cast<Http2FrameFlag>(0x01)); + EXPECT_EQ(Http2FrameFlag::END_HEADERS, static_cast<Http2FrameFlag>(0x04)); + EXPECT_EQ(Http2FrameFlag::PADDED, static_cast<Http2FrameFlag>(0x08)); + EXPECT_EQ(Http2FrameFlag::PRIORITY, static_cast<Http2FrameFlag>(0x20)); - EXPECT_EQ(Http2FrameFlag::FLAG_END_STREAM, 0x01); - EXPECT_EQ(Http2FrameFlag::FLAG_ACK, 0x01); - EXPECT_EQ(Http2FrameFlag::FLAG_END_HEADERS, 0x04); - EXPECT_EQ(Http2FrameFlag::FLAG_PADDED, 0x08); - EXPECT_EQ(Http2FrameFlag::FLAG_PRIORITY, 0x20); + EXPECT_EQ(Http2FrameFlag::END_STREAM, 0x01); + EXPECT_EQ(Http2FrameFlag::ACK, 0x01); + EXPECT_EQ(Http2FrameFlag::END_HEADERS, 0x04); + EXPECT_EQ(Http2FrameFlag::PADDED, 0x08); + EXPECT_EQ(Http2FrameFlag::PRIORITY, 0x20); } TEST(Http2ConstantsTest, Http2FrameFlagsToString) { // Single flags... // 0b00000001 - EXPECT_EQ("END_STREAM", - Http2FrameFlagsToString(Http2FrameType::DATA, - Http2FrameFlag::FLAG_END_STREAM)); + EXPECT_EQ("END_STREAM", Http2FrameFlagsToString(Http2FrameType::DATA, + Http2FrameFlag::END_STREAM)); EXPECT_EQ("END_STREAM", Http2FrameFlagsToString(Http2FrameType::HEADERS, 0x01)); EXPECT_EQ("ACK", Http2FrameFlagsToString(Http2FrameType::SETTINGS, - Http2FrameFlag::FLAG_ACK)); + Http2FrameFlag::ACK)); EXPECT_EQ("ACK", Http2FrameFlagsToString(Http2FrameType::PING, 0x01)); // 0b00000010 @@ -90,7 +88,7 @@ // 0b00000100 EXPECT_EQ("END_HEADERS", Http2FrameFlagsToString(Http2FrameType::HEADERS, - Http2FrameFlag::FLAG_END_HEADERS)); + Http2FrameFlag::END_HEADERS)); EXPECT_EQ("END_HEADERS", Http2FrameFlagsToString(Http2FrameType::PUSH_PROMISE, 0x04)); EXPECT_EQ("END_HEADERS", Http2FrameFlagsToString(0x09, 0x04)); @@ -98,10 +96,10 @@ // 0b00001000 EXPECT_EQ("PADDED", Http2FrameFlagsToString(Http2FrameType::DATA, - Http2FrameFlag::FLAG_PADDED)); + Http2FrameFlag::PADDED)); EXPECT_EQ("PADDED", Http2FrameFlagsToString(Http2FrameType::HEADERS, 0x08)); EXPECT_EQ("PADDED", Http2FrameFlagsToString(0x05, 0x08)); - EXPECT_EQ("0x08", Http2FrameFlagsToString(0xff, Http2FrameFlag::FLAG_PADDED)); + EXPECT_EQ("0x08", Http2FrameFlagsToString(0xff, Http2FrameFlag::PADDED)); // 0b00010000 EXPECT_EQ("0x10", Http2FrameFlagsToString(Http2FrameType::SETTINGS, 0x10));
diff --git a/net/http2/http2_constants_test_util.cc b/net/http2/http2_constants_test_util.cc index 2f4be40..1cad515e 100644 --- a/net/http2/http2_constants_test_util.cc +++ b/net/http2/http2_constants_test_util.cc
@@ -31,32 +31,32 @@ switch (type) { case Http2FrameType::DATA: return { - Http2FrameFlag::FLAG_END_STREAM, - Http2FrameFlag::FLAG_PADDED, + Http2FrameFlag::END_STREAM, + Http2FrameFlag::PADDED, }; case Http2FrameType::HEADERS: return { - Http2FrameFlag::FLAG_END_STREAM, - Http2FrameFlag::FLAG_END_HEADERS, - Http2FrameFlag::FLAG_PADDED, - Http2FrameFlag::FLAG_PRIORITY, + Http2FrameFlag::END_STREAM, + Http2FrameFlag::END_HEADERS, + Http2FrameFlag::PADDED, + Http2FrameFlag::PRIORITY, }; case Http2FrameType::SETTINGS: return { - Http2FrameFlag::FLAG_ACK, + Http2FrameFlag::ACK, }; case Http2FrameType::PUSH_PROMISE: return { - Http2FrameFlag::FLAG_END_HEADERS, - Http2FrameFlag::FLAG_PADDED, + Http2FrameFlag::END_HEADERS, + Http2FrameFlag::PADDED, }; case Http2FrameType::PING: return { - Http2FrameFlag::FLAG_ACK, + Http2FrameFlag::ACK, }; case Http2FrameType::CONTINUATION: return { - Http2FrameFlag::FLAG_END_HEADERS, + Http2FrameFlag::END_HEADERS, }; default: return std::vector<Http2FrameFlag>{}; @@ -103,27 +103,26 @@ uint8_t KnownFlagsMaskForFrameType(Http2FrameType type) { switch (type) { case Http2FrameType::DATA: - return Http2FrameFlag::FLAG_END_STREAM | Http2FrameFlag::FLAG_PADDED; + return Http2FrameFlag::END_STREAM | Http2FrameFlag::PADDED; case Http2FrameType::HEADERS: - return Http2FrameFlag::FLAG_END_STREAM | - Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED | - Http2FrameFlag::FLAG_PRIORITY; + return Http2FrameFlag::END_STREAM | Http2FrameFlag::END_HEADERS | + Http2FrameFlag::PADDED | Http2FrameFlag::PRIORITY; case Http2FrameType::PRIORITY: return 0x00; case Http2FrameType::RST_STREAM: return 0x00; case Http2FrameType::SETTINGS: - return Http2FrameFlag::FLAG_ACK; + return Http2FrameFlag::ACK; case Http2FrameType::PUSH_PROMISE: - return Http2FrameFlag::FLAG_END_HEADERS | Http2FrameFlag::FLAG_PADDED; + return Http2FrameFlag::END_HEADERS | Http2FrameFlag::PADDED; case Http2FrameType::PING: - return Http2FrameFlag::FLAG_ACK; + return Http2FrameFlag::ACK; case Http2FrameType::GOAWAY: return 0x00; case Http2FrameType::WINDOW_UPDATE: return 0x00; case Http2FrameType::CONTINUATION: - return Http2FrameFlag::FLAG_END_HEADERS; + return Http2FrameFlag::END_HEADERS; case Http2FrameType::ALTSVC: return 0x00; default:
diff --git a/net/http2/http2_structures.h b/net/http2/http2_structures.h index 95328e1..d5abc91 100644 --- a/net/http2/http2_structures.h +++ b/net/http2/http2_structures.h
@@ -68,14 +68,14 @@ bool IsEndStream() const { DCHECK(type == Http2FrameType::DATA || type == Http2FrameType::HEADERS) << ToString(); - return (flags & Http2FrameFlag::FLAG_END_STREAM) != 0; + return (flags & Http2FrameFlag::END_STREAM) != 0; } // Is the ACK flag set? bool IsAck() const { DCHECK(type == Http2FrameType::SETTINGS || type == Http2FrameType::PING) << ToString(); - return (flags & Http2FrameFlag::FLAG_ACK) != 0; + return (flags & Http2FrameFlag::ACK) != 0; } // Is the END_HEADERS flag set? @@ -84,7 +84,7 @@ type == Http2FrameType::PUSH_PROMISE || type == Http2FrameType::CONTINUATION) << ToString(); - return (flags & Http2FrameFlag::FLAG_END_HEADERS) != 0; + return (flags & Http2FrameFlag::END_HEADERS) != 0; } // Is the PADDED flag set? @@ -92,13 +92,13 @@ DCHECK(type == Http2FrameType::DATA || type == Http2FrameType::HEADERS || type == Http2FrameType::PUSH_PROMISE) << ToString(); - return (flags & Http2FrameFlag::FLAG_PADDED) != 0; + return (flags & Http2FrameFlag::PADDED) != 0; } // Is the PRIORITY flag set? bool HasPriority() const { DCHECK_EQ(type, Http2FrameType::HEADERS) << ToString(); - return (flags & Http2FrameFlag::FLAG_PRIORITY) != 0; + return (flags & Http2FrameFlag::PRIORITY) != 0; } // Does the encoding of this header start with "HTTP/", indicating that it
diff --git a/net/http2/http2_structures_test.cc b/net/http2/http2_structures_test.cc index cce65fe..d0e914e 100644 --- a/net/http2/http2_structures_test.cc +++ b/net/http2/http2_structures_test.cc
@@ -135,11 +135,10 @@ INSTANTIATE_TEST_CASE_P(IsEndStream, IsEndStreamTest, Combine(ValuesIn(ValidFrameTypes()), - Values(~Http2FrameFlag::FLAG_END_STREAM, - 0xff))); + Values(~Http2FrameFlag::END_STREAM, 0xff))); TEST_P(IsEndStreamTest, IsEndStream) { - const bool is_set = (flags_ & Http2FrameFlag::FLAG_END_STREAM) == - Http2FrameFlag::FLAG_END_STREAM; + const bool is_set = + (flags_ & Http2FrameFlag::END_STREAM) == Http2FrameFlag::END_STREAM; string flags_string; Http2FrameHeader v(0, type_, flags_, 0); switch (type_) { @@ -152,7 +151,7 @@ } else { EXPECT_THAT(flags_string, Not(HasSubstr("END_STREAM"))); } - v.RetainFlags(Http2FrameFlag::FLAG_END_STREAM); + v.RetainFlags(Http2FrameFlag::END_STREAM); EXPECT_EQ(is_set, v.IsEndStream()) << v; { std::stringstream s; @@ -174,10 +173,9 @@ INSTANTIATE_TEST_CASE_P(IsAck, IsACKTest, Combine(ValuesIn(ValidFrameTypes()), - Values(~Http2FrameFlag::FLAG_ACK, 0xff))); + Values(~Http2FrameFlag::ACK, 0xff))); TEST_P(IsACKTest, IsAck) { - const bool is_set = - (flags_ & Http2FrameFlag::FLAG_ACK) == Http2FrameFlag::FLAG_ACK; + const bool is_set = (flags_ & Http2FrameFlag::ACK) == Http2FrameFlag::ACK; string flags_string; Http2FrameHeader v(0, type_, flags_, 0); switch (type_) { @@ -190,7 +188,7 @@ } else { EXPECT_THAT(flags_string, Not(HasSubstr("ACK"))); } - v.RetainFlags(Http2FrameFlag::FLAG_ACK); + v.RetainFlags(Http2FrameFlag::ACK); EXPECT_EQ(is_set, v.IsAck()) << v; { std::stringstream s; @@ -212,11 +210,10 @@ INSTANTIATE_TEST_CASE_P(IsEndHeaders, IsEndHeadersTest, Combine(ValuesIn(ValidFrameTypes()), - Values(~Http2FrameFlag::FLAG_END_HEADERS, - 0xff))); + Values(~Http2FrameFlag::END_HEADERS, 0xff))); TEST_P(IsEndHeadersTest, IsEndHeaders) { - const bool is_set = (flags_ & Http2FrameFlag::FLAG_END_HEADERS) == - Http2FrameFlag::FLAG_END_HEADERS; + const bool is_set = + (flags_ & Http2FrameFlag::END_HEADERS) == Http2FrameFlag::END_HEADERS; string flags_string; Http2FrameHeader v(0, type_, flags_, 0); switch (type_) { @@ -230,7 +227,7 @@ } else { EXPECT_THAT(flags_string, Not(HasSubstr("END_HEADERS"))); } - v.RetainFlags(Http2FrameFlag::FLAG_END_HEADERS); + v.RetainFlags(Http2FrameFlag::END_HEADERS); EXPECT_EQ(is_set, v.IsEndHeaders()) << v; { std::stringstream s; @@ -254,10 +251,10 @@ INSTANTIATE_TEST_CASE_P(IsPadded, IsPaddedTest, Combine(ValuesIn(ValidFrameTypes()), - Values(~Http2FrameFlag::FLAG_PADDED, 0xff))); + Values(~Http2FrameFlag::PADDED, 0xff))); TEST_P(IsPaddedTest, IsPadded) { const bool is_set = - (flags_ & Http2FrameFlag::FLAG_PADDED) == Http2FrameFlag::FLAG_PADDED; + (flags_ & Http2FrameFlag::PADDED) == Http2FrameFlag::PADDED; string flags_string; Http2FrameHeader v(0, type_, flags_, 0); switch (type_) { @@ -271,7 +268,7 @@ } else { EXPECT_THAT(flags_string, Not(HasSubstr("PADDED"))); } - v.RetainFlags(Http2FrameFlag::FLAG_PADDED); + v.RetainFlags(Http2FrameFlag::PADDED); EXPECT_EQ(is_set, v.IsPadded()) << v; { std::stringstream s; @@ -293,10 +290,10 @@ INSTANTIATE_TEST_CASE_P(HasPriority, HasPriorityTest, Combine(ValuesIn(ValidFrameTypes()), - Values(~Http2FrameFlag::FLAG_PRIORITY, 0xff))); + Values(~Http2FrameFlag::PRIORITY, 0xff))); TEST_P(HasPriorityTest, HasPriority) { const bool is_set = - (flags_ & Http2FrameFlag::FLAG_PRIORITY) == Http2FrameFlag::FLAG_PRIORITY; + (flags_ & Http2FrameFlag::PRIORITY) == Http2FrameFlag::PRIORITY; string flags_string; Http2FrameHeader v(0, type_, flags_, 0); switch (type_) { @@ -308,7 +305,7 @@ } else { EXPECT_THAT(flags_string, Not(HasSubstr("PRIORITY"))); } - v.RetainFlags(Http2FrameFlag::FLAG_PRIORITY); + v.RetainFlags(Http2FrameFlag::PRIORITY); EXPECT_EQ(is_set, v.HasPriority()) << v; { std::stringstream s;
diff --git a/net/spdy/http2_frame_decoder_adapter.cc b/net/spdy/http2_frame_decoder_adapter.cc index 8b74e6ec..f214c21 100644 --- a/net/spdy/http2_frame_decoder_adapter.cc +++ b/net/spdy/http2_frame_decoder_adapter.cc
@@ -221,8 +221,7 @@ if (header.type == Http2FrameType::DATA) { // For some reason SpdyFramer still rejects invalid DATA frame flags. - uint8_t valid_flags = - Http2FrameFlag::FLAG_PADDED | Http2FrameFlag::FLAG_END_STREAM; + uint8_t valid_flags = Http2FrameFlag::PADDED | Http2FrameFlag::END_STREAM; if (header.HasAnyFlags(~valid_flags)) { SetSpdyErrorAndNotify(SpdyFramerError::SPDY_INVALID_DATA_FRAME_FLAGS); return false;
diff --git a/styleguide/styleguide.md b/styleguide/styleguide.md index 85f792d..4af45ddd 100644 --- a/styleguide/styleguide.md +++ b/styleguide/styleguide.md
@@ -17,15 +17,17 @@ ## Python -Python code should follow [PEP-8](https://www.python.org/dev/peps/pep-0008/). - -Some existing scripts were originally written following Google's internal -style guideline and have the following two exceptions. New scripts should, -however, be PEP-8 compliant. +Python code should follow [PEP-8](https://www.python.org/dev/peps/pep-0008/), +except: * Use two-space indentation instead of four-space indentation. * Use `CamelCase()` method and function names instead of `unix_hacker_style()` names. +(The rationale for these is mostly legacy: the code was originally written +following Google's internal style guideline, the cost of updating all of the +code to PEP-8 compliance was not small, and consistency was seen to be a +greater virtue than compliance.) + [Depot tools](http://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools.html) contains a local copy of pylint, appropriately configured.
diff --git a/tools/valgrind/asan/asan_symbolize.py b/tools/valgrind/asan/asan_symbolize.py index 8a82f9d6..ecf4276 100755 --- a/tools/valgrind/asan/asan_symbolize.py +++ b/tools/valgrind/asan/asan_symbolize.py
@@ -184,7 +184,7 @@ test_run['original_output_snippet_base64'] = \ test_run['output_snippet_base64'] - test_run['output_snippet'] = symbolized_snippet + test_run['output_snippet'] = symbolized_snippet.decode('utf-8', 'replace') test_run['output_snippet_base64'] = \ base64.b64encode(symbolized_snippet) test_run['snippet_processed_by'] = 'asan_symbolize.py'