Make the BrowsingDataCookieHelper create canonical cookies with the correct cookie-domain attribute.

Don't use CanonicalCookie::Create method for creating canonical cookies in the BrowsingDataCookieHelper. This results in wrong default values if no cookie-domain attribute is given in the cookie-string.


BUG=158353
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=164709

[markusheintz_, 2012-10-29 15:59:56]

Jochen pls review this CL. 

I'll add the description and bug asap

[jochen (gone - plz use gerrit), 2012-10-29 16:04:24]

test plz?

[markusheintz_, 2012-10-29 16:26:58]

On 2012/10/29 16:04:24, jochen wrote:
> test plz?

Done

[jochen (gone - plz use gerrit), 2012-10-29 16:29:48]

lgtm with comments addressed

I think the CL description is incorrect. After all, it's the canonical cookie
that creates the values.

Could you please update the description and elaborate a bit on what the issue
was and how this change fixes it?

http://codereview.chromium.org/11341011/diff/4001/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc
(right):

http://codereview.chromium.org/11341011/diff/4001/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:173:
TEST_F(BrowsingDataCookieHelperTest, CannedCookieDomain) {
can you add a test where you feed the same cookie line into a cookie monster and
into the cookie helper, and then check that the cookies match?

[battre, 2012-10-29 18:11:04]

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc
(right):

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:108:
EXPECT_EQ("A", it->Name());
please test EXPECT_EQ("1", it->Value());

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:249: 
nit: -2 new lines

[markusheintz_, 2012-10-29 18:12:59]

http://codereview.chromium.org/11341011/diff/4001/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc
(right):

http://codereview.chromium.org/11341011/diff/4001/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:173:
TEST_F(BrowsingDataCookieHelperTest, CannedCookieDomain) {
On 2012/10/29 16:29:48, jochen wrote:
> can you add a test where you feed the same cookie line into a cookie monster
and
> into the cookie helper, and then check that the cookies match?

Done.

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
File chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc
(right):

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:108:
EXPECT_EQ("A", it->Name());
On 2012/10/29 18:11:04, battre wrote:
> please test EXPECT_EQ("1", it->Value());

Done.

http://codereview.chromium.org/11341011/diff/5003/chrome/browser/browsing_dat...
chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc:249: 
On 2012/10/29 18:11:04, battre wrote:
> nit: -2 new lines

Done.

[battre, 2012-10-29 18:18:16]

lgtm