Commit b7acd773 authored by Michael Crouse's avatar Michael Crouse Committed by Commit Bot

Use the smallest image within srcset for data saver users.

This selects the smallest image within a srcset for data saver users.
This creates a feature flag to control this functionality.

Bug: 606941
Change-Id: I1cbb6e543035d9803bb041fb9df1e5cd8c286801
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2044810Reviewed-by: default avatarrajendrant <rajendrant@chromium.org>
Reviewed-by: default avatarYoav Weiss <yoavweiss@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742938}
parent 6705c68f
...@@ -6,10 +6,12 @@ ...@@ -6,10 +6,12 @@
#include "base/barrier_closure.h" #include "base/barrier_closure.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings.h" #include "chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings.h"
#include "chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings_factory.h" #include "chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings_factory.h"
#include "chrome/browser/previews/previews_test_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_commands.h"
...@@ -24,6 +26,7 @@ ...@@ -24,6 +26,7 @@
#include "net/test/embedded_test_server/http_request.h" #include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h" #include "net/test/embedded_test_server/http_response.h"
#include "services/network/public/cpp/network_quality_tracker.h" #include "services/network/public/cpp/network_quality_tracker.h"
#include "third_party/blink/public/common/features.h"
namespace { namespace {
...@@ -619,3 +622,61 @@ IN_PROC_BROWSER_TEST_P(DataSaverForWorkerBrowserTest, ...@@ -619,3 +622,61 @@ IN_PROC_BROWSER_TEST_P(DataSaverForWorkerBrowserTest,
content::EvalJs(GetActiveWebContents(), content::EvalJs(GetActiveWebContents(),
"fetch_from_page('/echoheader?Save-Data');")); "fetch_from_page('/echoheader?Save-Data');"));
} }
class DataSaverWithImageServerBrowserTest : public InProcessBrowserTest {
public:
void SetUp() override {
test_server_.reset(new net::EmbeddedTestServer());
test_server_->RegisterRequestMonitor(base::BindRepeating(
&DataSaverWithImageServerBrowserTest::MonitorImageRequest,
base::Unretained(this)));
test_server_->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
LOG(WARNING) << GetChromeTestDataDir();
ASSERT_TRUE(test_server_->Start());
scoped_feature_list_.InitWithFeatures({blink::features::kSaveDataImgSrcset},
{});
InProcessBrowserTest::SetUp();
}
void EnableDataSaver(bool enabled) {
SetDataSaverEnabled(browser()->profile(), enabled);
}
void SetImagesNotToLoad(const std::vector<std::string>& imgs_not_to_load) {
imgs_not_to_load_ = std::vector<std::string>(imgs_not_to_load);
}
std::unique_ptr<net::EmbeddedTestServer> test_server_;
private:
// Called by |test_server_|.
void MonitorImageRequest(const net::test_server::HttpRequest& request) {
for (const auto& img : imgs_not_to_load_)
EXPECT_FALSE(request.GetURL().path() == img);
}
base::test::ScopedFeatureList scoped_feature_list_;
std::vector<std::string> imgs_not_to_load_;
};
IN_PROC_BROWSER_TEST_F(DataSaverWithImageServerBrowserTest,
ImgSrcset_DataSaverEnabled) {
EnableDataSaver(true);
SetImagesNotToLoad({"/data_saver/red.jpg"});
base::HistogramTester histogram_tester;
ui_test_utils::NavigateToURL(
browser(), test_server_->GetURL("/data_saver/image_srcset.html"));
}
IN_PROC_BROWSER_TEST_F(DataSaverWithImageServerBrowserTest,
ImgSrcset_DataSaverDisabled) {
EnableDataSaver(false);
SetImagesNotToLoad({"/data_saver/green.jpg"});
base::HistogramTester histogram_tester;
ui_test_utils::NavigateToURL(
browser(), test_server_->GetURL("/data_saver/image_srcset.html"));
}
<!DOCTYPE html>
<body>
<!-- Datasaver should choose red.png, otherwise green.png. -->
<img srcset="red.png 1w, green.png 4w" width=16 height=16>
</body>
...@@ -298,6 +298,11 @@ const base::Feature kLightweightNoStatePrefetch_FetchFonts{ ...@@ -298,6 +298,11 @@ const base::Feature kLightweightNoStatePrefetch_FetchFonts{
const base::Feature kForceWebContentsDarkMode{ const base::Feature kForceWebContentsDarkMode{
"WebContentsForceDark", base::FEATURE_DISABLED_BY_DEFAULT}; "WebContentsForceDark", base::FEATURE_DISABLED_BY_DEFAULT};
// A feature to enable using the smallest image specified within image srcset
// for users with Save Data enabled.
const base::Feature kSaveDataImgSrcset{"SaveDataImgSrcset",
base::FEATURE_DISABLED_BY_DEFAULT};
// Which algorithm should be used for color inversion? // Which algorithm should be used for color inversion?
const base::FeatureParam<ForceDarkInversionMethod>::Option const base::FeatureParam<ForceDarkInversionMethod>::Option
forcedark_inversion_method_options[] = { forcedark_inversion_method_options[] = {
......
...@@ -88,6 +88,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kLightweightNoStatePrefetch; ...@@ -88,6 +88,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kLightweightNoStatePrefetch;
BLINK_COMMON_EXPORT extern const base::Feature BLINK_COMMON_EXPORT extern const base::Feature
kLightweightNoStatePrefetch_FetchFonts; kLightweightNoStatePrefetch_FetchFonts;
BLINK_COMMON_EXPORT extern const base::Feature kSaveDataImgSrcset;
BLINK_COMMON_EXPORT extern const base::Feature kForceWebContentsDarkMode; BLINK_COMMON_EXPORT extern const base::Feature kForceWebContentsDarkMode;
BLINK_COMMON_EXPORT extern const base::FeatureParam<ForceDarkInversionMethod> BLINK_COMMON_EXPORT extern const base::FeatureParam<ForceDarkInversionMethod>
kForceDarkInversionMethodParam; kForceDarkInversionMethodParam;
......
...@@ -33,6 +33,8 @@ ...@@ -33,6 +33,8 @@
#include <algorithm> #include <algorithm>
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/web_network_state_notifier.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/frame_console.h" #include "third_party/blink/renderer/core/frame/frame_console.h"
#include "third_party/blink/renderer/core/frame/local_frame.h" #include "third_party/blink/renderer/core/frame/local_frame.h"
...@@ -448,8 +450,12 @@ static ImageCandidate PickBestImageCandidate( ...@@ -448,8 +450,12 @@ static ImageCandidate PickBestImageCandidate(
de_duped_image_candidates.push_back(&image); de_duped_image_candidates.push_back(&image);
prev_density = image.Density(); prev_density = image.Density();
} }
unsigned winner = unsigned winner =
SelectionLogic(de_duped_image_candidates, device_scale_factor); blink::WebNetworkStateNotifier::SaveDataEnabled() &&
base::FeatureList::IsEnabled(blink::features::kSaveDataImgSrcset)
? 0
: SelectionLogic(de_duped_image_candidates, device_scale_factor);
DCHECK_LT(winner, de_duped_image_candidates.size()); DCHECK_LT(winner, de_duped_image_candidates.size());
winner = AvoidDownloadIfHigherDensityResourceIsInCache( winner = AvoidDownloadIfHigherDensityResourceIsInCache(
de_duped_image_candidates, winner, document); de_duped_image_candidates, winner, document);
......
...@@ -4,9 +4,13 @@ ...@@ -4,9 +4,13 @@
#include "third_party/blink/renderer/core/html/parser/html_srcset_parser.h" #include "third_party/blink/renderer/core/html/parser/html_srcset_parser.h"
#include "testing/gtest/include/gtest/gtest.h"
#include <limits.h> #include <limits.h>
#include "base/test/scoped_feature_list.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/web_network_state_notifier.h"
namespace blink { namespace blink {
typedef struct { typedef struct {
...@@ -173,4 +177,150 @@ TEST(HTMLSrcsetParserTest, Basic) { ...@@ -173,4 +177,150 @@ TEST(HTMLSrcsetParserTest, Basic) {
} }
} }
TEST(HTMLSrcsetParserTest, SaveDataEnabledBasic) {
SrcsetParserTestCase test_cases[] = {
// 0
{2.0, 0.5, "", "data:,a 1w, data:,b 2x", "data:,a", 2.0, 1},
{2.0, 1, "", "data:,a 2w, data:,b 2x", "data:,a", 2.0, 2},
{2.0, -1, "", "1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0, -1},
{2.0, -1, "", "1x.gif 1q, 2x.gif 2x", "2x.gif", 2.0, -1},
{1.0, -1, "", "1x.gif 1q, 2x.gif 2x", "2x.gif", 2.0, -1},
{1.0, -1, "", "1x.gif 1x 100h, 2x.gif 2x", "2x.gif", 2.0, -1}, // 5
{1.0, -1, "", "1x.gif 1x 100w, 2x.gif 2x", "2x.gif", 2.0, -1},
{1.0, -1, "", "1x.gif 1x 100h 100w, 2x.gif 2x", "2x.gif", 2.0, -1},
{2.0, -1, "", "1x.gif 1x, 2x.gif -2x", "1x.gif", 1.0, -1},
{2.0, -1, "", "0x.gif 0x", "0x.gif", 0.0, -1},
{2.0, -1, "", "0x.gif -0x", "0x.gif", 0.0, -1}, // 10
{2.0, -1, "", "neg.gif -2x", "", 1.0, -1},
{2.0, -1, "", "1x.gif 1x, 2x.gif 2q", "1x.gif", 1.0, -1},
{2.0, -1, "", "1x.gif, 2x.gif 2q", "1x.gif", 1.0, -1},
{2.0, -1, "", "1x.gif , 2x.gif 2q", "1x.gif", 1.0, -1},
{2.0, -1, "1x.gif 1x, 2x.gif 2x", "1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0,
-1}, // 15
{1.0, -1, "1x.gif 1x, 2x.gif 2x", "1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0,
-1},
{1.0, -1, "1x.gif 1x, 2x.gif 2x", "", "1x.gif 1x, 2x.gif 2x", 1.0, -1},
{2.0, -1, "src.gif", "1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0, -1},
{1.0, -1, "src.gif", "1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0, -1},
{1.0, -1, "src.gif", "2x.gif 2x", "src.gif", 1.0, -1}, // 20
{2.0, -1, "src.gif", "2x.gif 2x", "src.gif", 1.0, -1},
{2.0, -1, "src.gif", "2x.gif 2px", "src.gif", 1.0, -1},
{2.0, -1, "src.gif", "2x.gif 2ex", "src.gif", 1.0, -1},
{10.0, -1, "src.gif", "2x.gif 2e1x", "src.gif", 1.0, -1},
{2.0, -1, "src.gif", "2x.gif 2e1x", "src.gif", 1.0, -1}, // 25
{2.0, -1, "src.gif", "2x.gif +2x", "src.gif", 1.0, -1},
{1.5, -1, "src.gif", "2x.gif 2x", "src.gif", 1.0, -1},
{2.5, -1, "src.gif", "2x.gif 2x", "src.gif", 1.0, -1},
{2.5, -1, "src.gif", "2x.gif 2x, 3x.gif 3x", "src.gif", 1.0, -1},
{2.0, -1, "", "1x,, , x ,2x ", "1x", 1.0, -1}, // 30
{2.0, -1, "", "1x,, , x ,2x ", "1x", 1.0, -1},
{2.0, -1, "", ",,1x,, , x ,2x ", "1x", 1.0, -1},
{2.0, -1, "", ",,1x,,", "1x", 1.0, -1},
{2.0, -1, "", ",1x,", "1x", 1.0, -1},
{2.0, -1, "",
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUg 1x, 2x.gif 2x",
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUg", 1.0, -1}, // 35
{2.0, -1, "",
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUg 2x, 1x.gif 1x", "1x.gif",
1.0, -1},
{2.0, -1, "",
"1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100z, 5x.gif 5, dx.gif dx, "
"2x.gif 2x ,",
"1x", 1.0, -1},
{4.0, -1, "",
"1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100h, 5x.gif 5, dx.gif dx, "
"2x.gif 2x ,",
"1x", 1.0, -1},
{4.0, -1, "",
"1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100z, 5x.gif 5, dx.gif dx, "
"2x.gif 2x ,",
"1x", 1.0, -1},
{1.0, -1, "",
"1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100z, 5x.gif 5, dx.gif dx, "
"2x.gif 2x ,",
"1x", 1.0, -1}, // 40
{5.0, -1, "",
"1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100z, 5x.gif 5, dx.gif dx, "
"2x.gif 2x ,",
"1x", 1.0, -1},
{2.0, -1, "",
"1x.gif 1x, "
"data:image/"
"svg+xml;base64,"
"PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxMDAiIGh"
"laWdodD0iMTAwIj4KCTxyZWN0IHdpZHRoPSIxMDAiIGhlaWdodD0iMTAwIiBmaWxsPSJncm"
"VlbiIvPgo8L3N2Zz4K 2x",
"1x.gif", 1.0, -1},
{2.0, -1, "1x.gif",
"data:image/"
"svg+xml;base64,"
"PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxMDAiIGh"
"laWdodD0iMTAwIj4KCTxyZWN0IHdpZHRoPSIxMDAiIGhlaWdodD0iMTAwIiBmaWxsPSJncm"
"VlbiIvPgo8L3N2Zz4K 2x",
"1x.gif", 1.0, -1},
{2.0, -1, "1x.svg#red", "1x.svg#green 2x", "1x.svg#red", 1.0, -1},
{2.0, -1, "", "1x.svg#red 1x, 1x.svg#green 2x", "1x.svg#red", 1.0,
-1}, // 45
{1.0, 400, "", "400.gif 400w, 6000.gif 6000w", "400.gif", 1.0, 400},
{1.0, 400, "", "400.gif 400pw, 6000.gif 6000w", "6000.gif", 15.0, 6000},
{1.0, 400, "fallback.gif", "400.gif 400pw", "fallback.gif", 1.0, -1},
{1.0, 400, "fallback.gif", "400.gif +400w", "fallback.gif", 1.0, -1},
{1.0, 400, "", "400.gif 400w 400h, 6000.gif 6000w", "400.gif", 1.0,
400}, // 50
{4.0, 400, "", "400.gif 400w, 6000.gif 6000w", "400.gif", 1.0, 400},
{3.8, 400, "", "400.gif 400w, 6000.gif 6000w", "400.gif", 1.0, 400},
{0.9, 800, "src.gif", "400.gif 400w", "400.gif", 0.5, 400},
{0.9, 800, "src.gif", "1x.gif 1x, 400.gif 400w", "400.gif", 0.5, 400},
{0.9, 800, "src.gif", "1x.gif 0.6x, 400.gif 400w", "400.gif", 0.5,
400}, // 55
{0.9, 800, "src.gif", "1x.gif 1x, 400.gif 720w", "400.gif", 0.9, 720},
{0.9, 800, "src.gif", "1x.gif 1x, 400.gif 719w", "400.gif", 719.0 / 800.0,
719},
{2.0, 800, "src.gif", "400.gif 400w", "400.gif", 0.5, 400},
{1.0, 400, "src.gif", "800.gif 800w", "800.gif", 2.0, 800},
{1.0, 400, "src.gif", "0.gif 0w, 800.gif 800w", "800.gif", 2.0,
800}, // 60
{1.0, 400, "src.gif", "0.gif 0w, 2x.gif 2x", "src.gif", 1.0, -1},
{1.0, 400, "src.gif", "800.gif 2x, 1600.gif 1600w", "800.gif", 2.0, -1},
{1.0, 400, "", "400.gif 400w, 2x.gif 2x", "400.gif", 1.0, 400},
{2.0, 400, "", "400.gif 400w, 2x.gif 2x", "400.gif", 1.0, 400},
{1.0, 0, "", "400.gif 400w, 6000.gif 6000w", "400.gif",
std::numeric_limits<float>::infinity(), 400}, // 65
{2.0, -1, "", ", 1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0, -1},
{1.0, -1, "", ",1x.gif 1x, 2x.gif 2x", "1x.gif", 1.0, -1},
{1.0, -1, "", ",1x.gif 1.x , 2x.gif 2x", "2x.gif", 2.0, -1},
{1.2, -1, "", ",1x.gif 1x, 1.4x.gif 1.4x, 2x.gif 2x", "1x.gif", 1.0, -1},
{1.0, -1, "", "inf.gif 0.00000000001x", "inf.gif", 1e-11, -1}, // 70
{1.0, -1, "", "data:,a ( , data:,b 1x, ), data:,c", "data:,c", 1.0, -1},
{1.0, 1, "", "data:,a 1w 1h", "data:,a", 1.0, 1},
{1.0, -1, "", ",1x.gif 1x future-descriptor(3x, 4h, whatever), 2x.gif 2x",
"2x.gif", 2.0, -1},
{2.0, -1, "", ",1x.gif 1x future-descriptor(3x, 4h, whatever), 2x.gif 2x",
"2x.gif", 2.0, -1},
{1.0, -1, "", "data:,a 1 w", "", 1.0, -1}, // 75
{1.0, -1, "", "data:,a 1 w", "", 1.0, -1},
{1.0, -1, "", "data:,a +1x", "", 1.0, -1},
{1.0, -1, "", "data:,a +1x", "", 1.0, -1},
{1.0, -1, "", "data:,a 1.0x", "data:,a", 1.0, -1},
{1.0, -1, "", "1%20and%202.gif 1x", "1%20and%202.gif", 1.0, -1}, // 80
{1.0, 700, "", "data:,a 0.5x, data:,b 1400w", "data:,a", 0.5, -1},
{0, 0, nullptr, nullptr, nullptr,
0} // Do not remove the terminator line.
};
blink::WebNetworkStateNotifier::SetSaveDataEnabled(true);
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures({blink::features::kSaveDataImgSrcset},
{});
for (unsigned i = 0; test_cases[i].src_input; ++i) {
SrcsetParserTestCase test = test_cases[i];
ImageCandidate candidate = BestFitSourceForImageAttributes(
test.device_scale_factor, test.effective_size, test.src_input,
test.srcset_input);
ASSERT_EQ(test.output_density, candidate.Density());
ASSERT_EQ(test.output_resource_width, candidate.GetResourceWidth());
ASSERT_EQ(test.output_url, candidate.ToString().Ascii());
}
}
} // namespace blink } // namespace blink
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