Fix invalid parsing of cookie in Web request helpers.

Fix invalid parsing of cookie in Web request helpers.

This issue was found by a linter.

The while condition was always false. Thus, the parsing do not move to the end of "...".

There is no unittest for this class?

Add unittest from:
  https://codereview.chromium.org/22156003/

R=battre@chromium.org

BUG=

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

[etienneb, 2013-08-02 17:29:59]

PTAL.

[battre, 2013-08-05 07:59:50]

https://chromiumcodereview.appspot.com/21907006/diff/13001/chrome/browser/ext...
File chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
(right):

https://chromiumcodereview.appspot.com/21907006/diff/13001/chrome/browser/ext...
chrome/browser/extensions/api/web_request/web_request_api_helpers.cc:501: ++i; 
// Skip '"'.
Great catch!!!

The corresponding unit test to this is 

TEST(ExtensionWebRequestHelpersTest,
     TestMergeOnBeforeSendHeadersResponses_Cookies) {

in web_request_api_unittest.cc. Could you please update this test please?

Thank you.

[vabr (Chromium), 2013-08-05 09:18:20]

Thanks, etienneb!

Vaclav

https://chromiumcodereview.appspot.com/21907006/diff/13001/chrome/browser/ext...
File chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
(right):

https://chromiumcodereview.appspot.com/21907006/diff/13001/chrome/browser/ext...
chrome/browser/extensions/api/web_request/web_request_api_helpers.cc:501: ++i; 
// Skip '"'.
I updated the test in https://codereview.chromium.org/22156003. Once you land
this, I'll land that. Or of course feel free to copy that patch, modify it, or
ignore it. :)

On 2013/08/05 07:59:50, battre wrote:
> Great catch!!!
> 
> The corresponding unit test to this is 
> 
> TEST(ExtensionWebRequestHelpersTest,
>      TestMergeOnBeforeSendHeadersResponses_Cookies) {
> 
> in web_request_api_unittest.cc. Could you please update this test please?
> 
> Thank you.

[vabr (Chromium), 2013-08-05 09:20:48]

And LGTM, although I'm not an OWNER of this.

Vaclav

[etienneb, 2013-08-05 13:36:34]

Who is the owner.

I bring the modif of this CL
  https://codereview.chromium.org/22156003/

[etienneb, 2013-08-05 13:37:54]

PTAnL.

The unittest is fixed.
I think battre@ is an owner.

[battre, 2013-08-05 13:59:29]

LGTM under the condition that the test case fails without the modification of
web_request_api_helpers.cc and passes with the modification

[vabr (Chromium), 2013-08-05 14:16:42]

On 2013/08/05 13:59:29, battre wrote:
> LGTM under the condition that the test case fails without the modification of
> web_request_api_helpers.cc and passes with the modification

Dominic, I can confirm that that is true (test passes with the fix, but not
without).

LGTM.

Thanks,
Vaclav

[etienneb, 2013-08-05 14:27:00]

On 2013/08/05 14:16:42, vabr (Chromium) wrote:
> On 2013/08/05 13:59:29, battre wrote:
> > LGTM under the condition that the test case fails without the modification
of
> > web_request_api_helpers.cc and passes with the modification
> 
> Dominic, I can confirm that that is true (test passes with the fix, but not
> without).
> 
> LGTM.
> 
> Thanks,
> Vaclav

Oki, everybody seems happy with this solution.
Committing via CQ.
thanks

[commit-bot: I haz the power, 2013-08-05 14:34:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/etienneb@chromium.org/21907006/21001

[commit-bot: I haz the power, 2013-08-05 18:22:16]

Change committed as 215642