Skip to content

Commit

Permalink
[fuchsia][client-hints] Temporarily disable Client Hints on WebEngine
Browse files Browse the repository at this point in the history
Client Hints caused a few apps to break. Disable it while
investigating specific root cause of breakage.

Bug: 1356277, b/243040792
Change-Id: If3d3c7809b6eb57a653ce20ece9f9c11a11761d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3854418
Reviewed-by: Fabrice de Gans <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
Auto-Submit: David Song <wintermelons@google.com>
Cr-Commit-Position: refs/heads/main@{#1038960}
  • Loading branch information
wintermelons authored and Chromium LUCI CQ committed Aug 24, 2022
1 parent 9570972 commit edd99fc
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 17 deletions.
36 changes: 20 additions & 16 deletions fuchsia_web/webengine/browser/client_hints_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ void ExpectStringIsNonNegativeNumber(std::string& str) {

} // namespace

class ClientHintsTest : public FrameImplTestBaseWithServer {
// TODO(crbug.com/1356277): Client Hints temporarily disabled as it is causing
// several apps to fail. Re-enable Client Hints tests after breakage is fixed.
class DISABLED_ClientHintsTest : public FrameImplTestBaseWithServer {
public:
ClientHintsTest() = default;
~ClientHintsTest() override = default;
ClientHintsTest(const ClientHintsTest&) = delete;
ClientHintsTest& operator=(const ClientHintsTest&) = delete;
DISABLED_ClientHintsTest() = default;
~DISABLED_ClientHintsTest() override = default;
DISABLED_ClientHintsTest(const DISABLED_ClientHintsTest&) = delete;
DISABLED_ClientHintsTest& operator=(const DISABLED_ClientHintsTest&) = delete;

void SetUpOnMainThread() override {
FrameImplTestBaseWithServer::SetUpOnMainThread();
Expand Down Expand Up @@ -122,7 +124,7 @@ class ClientHintsTest : public FrameImplTestBaseWithServer {
FrameForTest frame_for_test_;
};

IN_PROC_BROWSER_TEST_F(ClientHintsTest, NumericalClientHints) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, NumericalClientHints) {
SetClientHintsForTestServerToRequest(std::string(kRoundTripTimeCH) + "," +
std::string(kDeviceMemoryCH));
GetAndVerifyClientHint(kRoundTripTimeCH,
Expand All @@ -131,7 +133,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest, NumericalClientHints) {
base::BindRepeating(&ExpectStringIsNonNegativeNumber));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, InvalidClientHint) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, InvalidClientHint) {
// Check browser handles requests for an invalid Client Hint.
SetClientHintsForTestServerToRequest("not-a-client-hint");
GetAndVerifyClientHint("not-a-client-hint",
Expand All @@ -143,7 +145,8 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest, InvalidClientHint) {
// Low-entropy User Agent Client Hints are sent by default without the origin
// needing to request them. For a list of low-entropy Client Hints, see
// https://wicg.github.io/client-hints-infrastructure/#low-entropy-hint-table/
IN_PROC_BROWSER_TEST_F(ClientHintsTest, LowEntropyClientHintsAreSentByDefault) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest,
LowEntropyClientHintsAreSentByDefault) {
GetAndVerifyClientHint(
kUserAgentCH, base::BindRepeating([](std::string& str) {
EXPECT_TRUE(str.find("Chromium") != std::string::npos);
Expand All @@ -152,7 +155,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest, LowEntropyClientHintsAreSentByDefault) {
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest,
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest,
LowEntropyClientHintsAreSentWhenRequested) {
SetClientHintsForTestServerToRequest(kUserAgentCH);
GetAndVerifyClientHint(
Expand All @@ -163,15 +166,15 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest,
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest,
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest,
HighEntropyClientHintsAreNotSentByDefault) {
GetAndVerifyClientHint(kFullVersionListCH,
base::BindRepeating([](std::string& str) {
EXPECT_EQ(str, kHeaderNotPresent);
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest,
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest,
HighEntropyClientHintsAreSentWhenRequested) {
SetClientHintsForTestServerToRequest(kFullVersionListCH);
GetAndVerifyClientHint(
Expand All @@ -182,7 +185,7 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest,
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, ArchitectureIsArmOrX86) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, ArchitectureIsArmOrX86) {
SetClientHintsForTestServerToRequest(kArchCH);
GetAndVerifyClientHint(kArchCH, base::BindRepeating([](std::string& str) {
#if defined(ARCH_CPU_X86_64)
Expand All @@ -195,22 +198,22 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest, ArchitectureIsArmOrX86) {
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, BitnessIs64) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, BitnessIs64) {
SetClientHintsForTestServerToRequest(kBitnessCH);
GetAndVerifyClientHint(kBitnessCH, base::BindRepeating([](std::string& str) {
EXPECT_EQ(str, "\"64\"");
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, PlatformIsFuchsia) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, PlatformIsFuchsia) {
// Platform is a low-entropy Client Hint, so no need for test server to
// request it.
GetAndVerifyClientHint(kPlatformCH, base::BindRepeating([](std::string& str) {
EXPECT_EQ(str, "\"Fuchsia\"");
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, RemoveClientHint) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest, RemoveClientHint) {
SetClientHintsForTestServerToRequest(std::string(kRoundTripTimeCH) + "," +
std::string(kDeviceMemoryCH));
GetAndVerifyClientHint(kDeviceMemoryCH,
Expand All @@ -225,7 +228,8 @@ IN_PROC_BROWSER_TEST_F(ClientHintsTest, RemoveClientHint) {
}));
}

IN_PROC_BROWSER_TEST_F(ClientHintsTest, AdditionalClientHintsAreAlwaysSent) {
IN_PROC_BROWSER_TEST_F(DISABLED_ClientHintsTest,
AdditionalClientHintsAreAlwaysSent) {
SetClientHintsForTestServerToRequest(kRoundTripTimeCH);

// Enable device memory as an additional Client Hint.
Expand Down
4 changes: 3 additions & 1 deletion fuchsia_web/webengine/browser/web_engine_browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ WebEngineBrowserContext::GetPermissionControllerDelegate() {

content::ClientHintsControllerDelegate*
WebEngineBrowserContext::GetClientHintsControllerDelegate() {
return &client_hints_delegate_;
// TODO(crbug.com/1356277): Temporarily disable Client Hints as it is causing
// several apps to fail. Re-enable Client Hints after breakage is fixed.
return nullptr;
}

content::BackgroundFetchDelegate*
Expand Down

0 comments on commit edd99fc

Please sign in to comment.