Commit c4e3ab82 authored by Shawn Pickett's avatar Shawn Pickett Committed by Commit Bot

Only enable media battery changes when running on battery

Bug: 984851

In http://crrev.com/c/1708013, a change was committed to improve
battery life by avoiding unnecessary media caching. This submission,
driven by feedback from http://crbug.com/984851, makes an additional
adjustment in preparation for testing trials.

It adds a check to verify that the device is running on battery
and is not connected to AC power. Since the goal is to improve battery
life, this scopes the change so that it doesn't impact scenarios when
the device is connected to power. The potential disadvantage is that
the caching behavior can now be different depending on whether the
device is connected or not. Since the entire change is under a feature
key, the idea is to verify that the reduction in media caching does
not lead to any discernible difference in user-facing functionality
between the scenarios during the testing trials.

Change-Id: I8b73da81a509962701e30cfd43f36b34b0dc70b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830069Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Reviewed-by: default avatarChrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Shawn Pickett <shawnpi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#734104}
parent 1c163748
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "base/location.h" #include "base/location.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/power_monitor/power_monitor.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_number_conversions.h" // For HexEncode. #include "base/strings/string_number_conversions.h" // For HexEncode.
...@@ -81,6 +82,12 @@ void RecordNoStoreHeaderHistogram(int load_flags, ...@@ -81,6 +82,12 @@ void RecordNoStoreHeaderHistogram(int load_flags,
} }
} }
bool IsOnBatteryPower() {
if (base::PowerMonitor::IsInitialized())
return base::PowerMonitor::IsOnBatteryPower();
return false;
}
enum ExternallyConditionalizedType { enum ExternallyConditionalizedType {
EXTERNALLY_CONDITIONALIZED_CACHE_REQUIRES_VALIDATION, EXTERNALLY_CONDITIONALIZED_CACHE_REQUIRES_VALIDATION,
EXTERNALLY_CONDITIONALIZED_CACHE_USABLE, EXTERNALLY_CONDITIONALIZED_CACHE_USABLE,
...@@ -1881,7 +1888,7 @@ int HttpCache::Transaction::DoUpdateCachedResponse() { ...@@ -1881,7 +1888,7 @@ int HttpCache::Transaction::DoUpdateCachedResponse() {
} }
if (response_.headers->HasHeaderValue("cache-control", "no-store") || if (response_.headers->HasHeaderValue("cache-control", "no-store") ||
ShouldDisableMediaCaching(response_.headers.get())) { ShouldDisableCaching(response_.headers.get())) {
if (!entry_->doomed) { if (!entry_->doomed) {
int ret = cache_->DoomEntry(cache_key_, nullptr); int ret = cache_->DoomEntry(cache_key_, nullptr);
DCHECK_EQ(OK, ret); DCHECK_EQ(OK, ret);
...@@ -3110,7 +3117,7 @@ int HttpCache::Transaction::WriteResponseInfoToEntry( ...@@ -3110,7 +3117,7 @@ int HttpCache::Transaction::WriteResponseInfoToEntry(
// status to a net error and replay the net error. // status to a net error and replay the net error.
if ((response.headers->HasHeaderValue("cache-control", "no-store")) || if ((response.headers->HasHeaderValue("cache-control", "no-store")) ||
IsCertStatusError(response.ssl_info.cert_status) || IsCertStatusError(response.ssl_info.cert_status) ||
ShouldDisableMediaCaching(response.headers.get())) { ShouldDisableCaching(response.headers.get())) {
bool stopped = StopCachingImpl(false); bool stopped = StopCachingImpl(false);
DCHECK(stopped); DCHECK(stopped);
if (net_log_.IsCapturing()) if (net_log_.IsCapturing())
...@@ -3593,23 +3600,23 @@ void HttpCache::Transaction::TransitionToState(State state) { ...@@ -3593,23 +3600,23 @@ void HttpCache::Transaction::TransitionToState(State state) {
next_state_ = state; next_state_ = state;
} }
bool HttpCache::Transaction::ShouldDisableMediaCaching( bool HttpCache::Transaction::ShouldDisableCaching(
const HttpResponseHeaders* headers) const { const HttpResponseHeaders* headers) const {
bool disable_caching = false; bool disable_caching = false;
if (base::FeatureList::IsEnabled(features::kTurnOffStreamingMediaCaching)) { if (base::FeatureList::IsEnabled(features::kTurnOffStreamingMediaCaching) &&
// If the acquired content is 'large' and not already cached, and we have IsOnBatteryPower()) {
// a MIME type of audio or video, then disable the cache for this response. // If we're running on battery, and the acquired content is 'large' and
// We based our initial definition of 'large' on the disk cache maximum // not already cached, and we have a MIME type of audio or video, then
// block size of 16K, which we observed captures the majority of responses // disable the cache for this response. We based our initial definition of
// from various MSE implementations. // 'large' on the disk cache maximum block size of 16K, which we observed
// captures the majority of responses from various MSE implementations.
static constexpr int kMaxContentSize = 4096 * 4; static constexpr int kMaxContentSize = 4096 * 4;
std::string mime_type; std::string mime_type;
base::CompareCase insensitive_ascii = base::CompareCase::INSENSITIVE_ASCII;
if (headers->GetContentLength() > kMaxContentSize && if (headers->GetContentLength() > kMaxContentSize &&
headers->response_code() != 304 && headers->GetMimeType(&mime_type) && headers->response_code() != 304 && headers->GetMimeType(&mime_type) &&
(base::StartsWith(mime_type, "video", (base::StartsWith(mime_type, "video", insensitive_ascii) ||
base::CompareCase::INSENSITIVE_ASCII) || base::StartsWith(mime_type, "audio", insensitive_ascii))) {
base::StartsWith(mime_type, "audio",
base::CompareCase::INSENSITIVE_ASCII))) {
disable_caching = true; disable_caching = true;
MediaCacheStatusResponseHistogram( MediaCacheStatusResponseHistogram(
MediaResponseCacheType::kMediaResponseTransactionCacheDisabled); MediaResponseCacheType::kMediaResponseTransactionCacheDisabled);
......
...@@ -563,8 +563,8 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction { ...@@ -563,8 +563,8 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction {
// Saves network transaction info using |transaction|. // Saves network transaction info using |transaction|.
void SaveNetworkTransactionInfo(const HttpTransaction& transaction); void SaveNetworkTransactionInfo(const HttpTransaction& transaction);
// Disables caching for media content when running on battery. // Disables caching for large content when running on battery.
bool ShouldDisableMediaCaching(const HttpResponseHeaders* headers) const; bool ShouldDisableCaching(const HttpResponseHeaders* headers) const;
State next_state_; State next_state_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment