Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
Consider CriOS to be chrome-like, meaning that we need also
Browse files Browse the repository at this point in the history
accept:image/webp to send webp.

Fixes #1256
  • Loading branch information
jmarantz committed Jan 28, 2016
1 parent aa4ec14 commit 23947d0
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 1 deletion.
12 changes: 12 additions & 0 deletions net/instaweb/rewriter/device_properties_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ TEST_F(DevicePropertiesTest, NoWebpForChromeWithoutAcceptHeader) {
EXPECT_FALSE(device_properties_.SupportsWebpRewrittenUrls());
EXPECT_FALSE(device_properties_.SupportsWebpLosslessAlpha());

device_properties_.SetUserAgent(
UserAgentMatcherTestBase::kCriOS48UserAgent);
EXPECT_FALSE(device_properties_.SupportsWebpInPlace());
EXPECT_FALSE(device_properties_.SupportsWebpRewrittenUrls());
EXPECT_FALSE(device_properties_.SupportsWebpLosslessAlpha());

// However, Chrome 25 and 37 will get webp due to the accept header.
RequestHeaders headers;
headers.Add(HttpAttributes::kAccept, "image/webp");
Expand All @@ -100,6 +106,12 @@ TEST_F(DevicePropertiesTest, NoWebpForChromeWithoutAcceptHeader) {
EXPECT_TRUE(device_properties_.SupportsWebpInPlace());
EXPECT_TRUE(device_properties_.SupportsWebpRewrittenUrls());
EXPECT_FALSE(device_properties_.SupportsWebpLosslessAlpha());

device_properties_.SetUserAgent(
UserAgentMatcherTestBase::kCriOS48UserAgent);
EXPECT_TRUE(device_properties_.SupportsWebpInPlace());
EXPECT_TRUE(device_properties_.SupportsWebpRewrittenUrls());
EXPECT_TRUE(device_properties_.SupportsWebpLosslessAlpha());
}

TEST_F(DevicePropertiesTest, WebpUserAgentIdentificationAccept) {
Expand Down
4 changes: 3 additions & 1 deletion pagespeed/kernel/http/user_agent_matcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,9 @@ bool UserAgentMatcher::IsiOSUserAgent(const StringPiece& user_agent) const {
}

bool UserAgentMatcher::IsChromeLike(const StringPiece& user_agent) const {
return user_agent.find("Chrome/") != StringPiece::npos;
return
(user_agent.find("Chrome/") != StringPiece::npos) ||
(user_agent.find("CriOS/") != StringPiece::npos);
}

bool UserAgentMatcher::GetChromeBuildNumber(const StringPiece& user_agent,
Expand Down
4 changes: 4 additions & 0 deletions pagespeed/kernel/http/user_agent_matcher_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ const char UserAgentMatcherTestBase::kCriOS32UserAgent[] =
"Mozilla/5.0 (iPhone; CPU iPhone OS 7_0_4 like Mac OS X) "
"AppleWebKit/537.51.1 (KHTML, like Gecko) CriOS/32.0.1700.21 "
"Mobile/11B554a Safari/9537.53 (A7BED55D-B09E-484F-B2FE-5E4952E9B87E)";
const char UserAgentMatcherTestBase::kCriOS48UserAgent[] =
"Mozilla/5.0 (iPhone; CPU iPhone OS 9_2 like Mac OS X) "
"AppleWebKit/601.1 (KHTML, like Gecko) CriOS/48.0.2564.87 "
"Mobile/13C75 Safari/601.1.46";
const char UserAgentMatcherTestBase::kDoCoMoMobileUserAgent[] =
"DoCoMo/1.0/D505iS/c20/TB/W20H10";
const char UserAgentMatcherTestBase::kFirefox1UserAgent[] =
Expand Down
1 change: 1 addition & 0 deletions pagespeed/kernel/http/user_agent_matcher_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class UserAgentMatcherTestBase : public testing::Test {
static const char kCompalUserAgent[];
static const char kCriOS31UserAgent[];
static const char kCriOS32UserAgent[];
static const char kCriOS48UserAgent[];
static const char kDoCoMoMobileUserAgent[];
static const char kFirefox1UserAgent[];
static const char kFirefox3UserAgent[];
Expand Down

3 comments on commit 23947d0

@HansVanEijsden
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to give you all a big THANK YOU for the quick work you did to patch this issue.

@m3digi
Copy link

@m3digi m3digi commented on 23947d0 Feb 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still seeing this issue in 1.10.33.2-7600. How do I resolve?

@jmarantz
Copy link
Contributor Author

@jmarantz jmarantz commented on 23947d0 Feb 22, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.