Skip to content

Commit d0581ca

Browse files
mefCommit bot
mef
authored and
Commit bot
committed
[Cronet] Don't spam log if system cookie doesn't have creation time.
BUG=688540 TEST=components/cronet/tools/cr_cronet.py build-test | grep "Cookie" CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Review-Url: https://codereview.chromium.org/2671713006 Cr-Commit-Position: refs/heads/master@{#448982}
1 parent 9ea409c commit d0581ca

File tree

5 files changed

+57
-8
lines changed

5 files changed

+57
-8
lines changed

components/cronet/ios/test/cronet_http_test.mm

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,39 @@ void StartDataTaskAndWaitForCompletion(NSURLSessionDataTask* task) {
322322
EXPECT_TRUE([[delegate_ responseBody] containsString:cookieValue]);
323323
}
324324

325+
TEST_F(HttpTest, SystemCookieWithNullCreationTime) {
326+
const char kCookieHeader[] = "Cookie";
327+
NSString* cookieName = [NSString
328+
stringWithFormat:@"SetSystemCookie-%@", [[NSUUID UUID] UUIDString]];
329+
NSString* cookieValue = [[NSUUID UUID] UUIDString];
330+
NSHTTPCookieStorage* systemCookieStorage =
331+
[NSHTTPCookieStorage sharedHTTPCookieStorage];
332+
NSURL* echoCookieUrl =
333+
net::NSURLWithGURL(GURL(TestServer::GetEchoHeaderURL(kCookieHeader)));
334+
NSHTTPCookie* nullCreationTimeCookie = [NSHTTPCookie cookieWithProperties:@{
335+
NSHTTPCookiePath : [echoCookieUrl path],
336+
NSHTTPCookieName : cookieName,
337+
NSHTTPCookieValue : cookieValue,
338+
NSHTTPCookieDomain : [echoCookieUrl host],
339+
@"Created" : [NSNumber numberWithDouble:0.0],
340+
}];
341+
[systemCookieStorage setCookie:nullCreationTimeCookie];
342+
NSHTTPCookie* normalCookie = [NSHTTPCookie cookieWithProperties:@{
343+
NSHTTPCookiePath : [echoCookieUrl path],
344+
NSHTTPCookieName : [cookieName stringByAppendingString:@"-normal"],
345+
NSHTTPCookieValue : cookieValue,
346+
NSHTTPCookieDomain : [echoCookieUrl host],
347+
}];
348+
[systemCookieStorage setCookie:normalCookie];
349+
StartDataTaskAndWaitForCompletion([session_ dataTaskWithURL:echoCookieUrl]);
350+
[systemCookieStorage deleteCookie:nullCreationTimeCookie];
351+
[systemCookieStorage deleteCookie:normalCookie];
352+
EXPECT_EQ(nil, [delegate_ error]);
353+
// Verify that cookie set in system store was sent to the serever.
354+
EXPECT_TRUE([[delegate_ responseBody] containsString:cookieName]);
355+
EXPECT_TRUE([[delegate_ responseBody] containsString:cookieValue]);
356+
}
357+
325358
TEST_F(HttpTest, FilterOutRequest) {
326359
NSURL* url =
327360
net::NSURLWithGURL(GURL(TestServer::GetEchoHeaderURL("User-Agent")));

ios/BUILD.gn

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,9 @@
44

55
assert(!is_component_build, "component build is unsupported on iOS")
66

7-
declare_args() {
8-
# Control whether cronet is build (this is usually set by the script
9-
# components/cronet/tools/cr_cronet.py as cronet requires specific
10-
# gn args to build correctly).
11-
is_cronet_build = false
7+
import("//ios/features.gni")
128

9+
declare_args() {
1310
# TODO(brettw) bug 684096: Remove these. These are to prevent unused
1411
# override warnings on iOS from the setting of the default_args in the
1512
# toplevel //.gn file. Since iOS doesn't compile V8, GN complains that

ios/features.gni

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Copyright 2017 The Chromium Authors. All rights reserved.
2+
# Use of this source code is governed by a BSD-style license that can be
3+
# found in the LICENSE file.
4+
5+
declare_args() {
6+
# Control whether cronet is build (this is usually set by the script
7+
# components/cronet/tools/cr_cronet.py as cronet requires specific
8+
# gn args to build correctly).
9+
is_cronet_build = false
10+
}

ios/net/BUILD.gn

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,17 @@
22
# Use of this source code is governed by a BSD-style license that can be
33
# found in the LICENSE file.
44

5+
import("//build/buildflag_header.gni")
56
import("//ios/build/config.gni")
7+
import("//ios/features.gni")
68
import("//testing/test.gni")
79
import("//url/features.gni")
810

11+
buildflag_header("ios_net_features") {
12+
header = "ios_net_features.h"
13+
flags = [ "CRONET_BUILD=$is_cronet_build" ]
14+
}
15+
916
group("all_tests") {
1017
testonly = true
1118
deps = [
@@ -15,6 +22,7 @@ group("all_tests") {
1522

1623
source_set("net") {
1724
deps = [
25+
":ios_net_features",
1826
"//base",
1927
"//net",
2028
"//url:url_features",

ios/net/cookies/cookie_store_ios.mm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "ios/net/cookies/cookie_creation_time_manager.h"
2929
#include "ios/net/cookies/cookie_store_ios_client.h"
3030
#include "ios/net/cookies/system_cookie_util.h"
31+
#include "ios/net/ios_net_features.h"
3132
#import "net/base/mac/url_conversions.h"
3233
#include "net/cookies/cookie_util.h"
3334
#include "net/cookies/parsed_cookie.h"
@@ -175,11 +176,11 @@ NSInteger CompareCookies(id a, id b, void* context) {
175176
DCHECK(manager);
176177
base::Time created_a = manager->GetCreationTime(cookie_a);
177178
base::Time created_b = manager->GetCreationTime(cookie_b);
178-
#if !defined(CRNET)
179+
#if !BUILDFLAG(CRONET_BUILD)
179180
// CookieCreationTimeManager is returning creation times that are null.
180-
// Since in CrNet, the cookie store is recreated on startup, let's suppress
181+
// Since in Cronet, the cookie store is recreated on startup, let's suppress
181182
// this warning for now.
182-
// TODO(huey): Instead of suppressing the warning, assign a creation time
183+
// TODO(mef): Instead of suppressing the warning, assign a creation time
183184
// to cookies if one doesn't already exist.
184185
DLOG_IF(ERROR, created_a.is_null() || created_b.is_null())
185186
<< "Cookie without creation date";

0 commit comments

Comments
 (0)