Skip to content

Commit

Permalink
Fix PageInfo for https image compression
Browse files Browse the repository at this point in the history
This CL sets/clears the image compression applied bit only for mainframe
navigations.

(cherry picked from commit 7e20524)

Bug: 1182131
Change-Id: Ib952202c97ecd16047a7a7798af25dee41d4287a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2718696
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#858412}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2737382
Cr-Commit-Position: refs/branch-heads/4430@{#143}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
rajendrant authored and Chromium LUCI CQ committed Mar 5, 2021
1 parent 3c7de18 commit 26d484e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "chrome/browser/login_detection/login_detection_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/subresource_redirect/https_image_compression_infobar_decider.h"
#include "chrome/browser/subresource_redirect/subresource_redirect_observer.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
Expand Down Expand Up @@ -105,6 +106,16 @@ class SubresourceRedirectLoginRobotsBrowserTest : public InProcessBrowserTest {
return EvalJs(web_contents, script).ExtractBool();
}

void VerifyImageCompressionPageInfoState(
bool is_https_image_compression_applied,
content::WebContents* web_contents = nullptr) {
if (!web_contents)
web_contents = browser()->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(is_https_image_compression_applied,
subresource_redirect::SubresourceRedirectObserver::
IsHttpsImageCompressionApplied(web_contents));
}

void CreateUkmRecorder() {
ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
}
Expand Down Expand Up @@ -195,6 +206,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
// components/subresource_redirect/common to access it here.
EXPECT_THAT(ukm_metrics, testing::Contains(testing::Key(
ImageCompressionUkm::kRedirectResultNameHash)));
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -246,6 +258,10 @@ IN_PROC_BROWSER_TEST_F(
// components/subresource_redirect/common to access it here.
EXPECT_THAT(ukm_metrics, testing::Contains(testing::Key(
ImageCompressionUkm::kRedirectResultNameHash)));

// Page info would still show compression is enabled even whn no image got
// compressed.
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
Expand All @@ -271,6 +287,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,

robots_rules_server_.VerifyRequestedOrigins({});
image_compression_server_.VerifyRequestedImagePaths({});
VerifyImageCompressionPageInfoState(false);
}

IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
Expand All @@ -295,6 +312,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,

robots_rules_server_.VerifyRequestedOrigins({});
image_compression_server_.VerifyRequestedImagePaths({});
VerifyImageCompressionPageInfoState(false);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -352,6 +370,7 @@ IN_PROC_BROWSER_TEST_F(
// components/subresource_redirect/common to access it here.
EXPECT_THAT(ukm_metrics, testing::Contains(testing::Key(
ImageCompressionUkm::kRedirectResultNameHash)));
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -379,6 +398,7 @@ IN_PROC_BROWSER_TEST_F(
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png"});
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
Expand All @@ -405,6 +425,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png", "/load_image/image.png?foo"});
VerifyImageCompressionPageInfoState(true);
}

// Verify an new image loads fine after robots rules fetch is complete.
Expand Down Expand Up @@ -445,6 +466,7 @@ IN_PROC_BROWSER_TEST_F(
"SubresourceRedirect.RobotsRules.Browser.InMemoryCacheHit", 1);
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png", "/load_image/image.png?foo"});
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -512,6 +534,7 @@ IN_PROC_BROWSER_TEST_F(
"SubresourceRedirect.RobotsRules.Browser.InMemoryCacheHit", 2);
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png", "/load_image/image.png?allowed"});
VerifyImageCompressionPageInfoState(true);
}

// Verifies that LitePages gets blocked due to robots fetch failure, and
Expand Down Expand Up @@ -560,6 +583,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
// No more additional fetches.
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths({});
VerifyImageCompressionPageInfoState(true);
}

// Verifies that when an image load fails, LitePages gets blocked, and
Expand Down Expand Up @@ -620,6 +644,7 @@ IN_PROC_BROWSER_TEST_F(SubresourceRedirectLoginRobotsBrowserTest,
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png"});
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -666,6 +691,7 @@ IN_PROC_BROWSER_TEST_F(

robots_rules_server_.VerifyRequestedOrigins({});
image_compression_server_.VerifyRequestedImagePaths({});
VerifyImageCompressionPageInfoState(false);
}

// Tests images in subframe are compressed.
Expand Down Expand Up @@ -700,6 +726,7 @@ IN_PROC_BROWSER_TEST_F(
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png?mainframe", "/load_image/image.png"});
VerifyImageCompressionPageInfoState(true);
}

// Tests images in cross-origin subframe are compressed.
Expand Down Expand Up @@ -740,6 +767,7 @@ IN_PROC_BROWSER_TEST_F(
https_test_server_.GetURL("foo.com", "/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png?mainframe", "/load_image/image.png"});
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -779,6 +807,8 @@ IN_PROC_BROWSER_TEST_F(
robots_rules_server_.VerifyRequestedOrigins({GetHttpsTestURL("/").spec()});
image_compression_server_.VerifyRequestedImagePaths(
{"/load_image/image.png?mainframe"});
// Main frame still enables image compression.
VerifyImageCompressionPageInfoState(true);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -810,6 +840,7 @@ IN_PROC_BROWSER_TEST_F(

robots_rules_server_.VerifyRequestedOrigins({});
image_compression_server_.VerifyRequestedImagePaths({});
VerifyImageCompressionPageInfoState(false);
}

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -872,6 +903,7 @@ IN_PROC_BROWSER_TEST_F(
net::HTTP_TEMPORARY_REDIRECT, 7);
histogram_tester_.ExpectTotalCount(
"SubresourceRedirect.CompressionAttempt.ServerResponded", 0);
VerifyImageCompressionPageInfoState(true);
}

} // namespace subresource_redirect
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,9 @@ void SubresourceRedirectObserver::DidFinishNavigation(
if (!IsLiteModeEnabled(web_contents()))
return;

// Set to disable compression by default for this navigation.
is_mainframe_https_image_compression_applied_ = false;
// Set to disable compression by default for the mainframe navigation.
if (navigation_handle->IsInMainFrame())
is_mainframe_https_image_compression_applied_ = false;

if (!navigation_handle->GetURL().SchemeIsHTTPOrHTTPS())
return;
Expand Down

0 comments on commit 26d484e

Please sign in to comment.