The password manager infobar is suppressed when the one-click infobar is

The password manager infobar is suppressed when the one-click infobar is
supposed to show itself.  However, in this case the problem was that the
later was not showing.  The problem is that the one-click infobar triggers
on the http header "Google-Accounts-Signin", which it was looking in the
headers of 302 redirect responses, but in some cases it can also appear in
200 responses.

BUG=132849
TEST=Make sure that the one-click infobar is always shown in the right
conditions.  I'm not sure how to trigger a sign in without all the redirects.
I was only able to repro this bug on on Windows XP machine.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152405

[Roger Tawa OOO till Jul 10th, 2012-08-17 19:30:55]

Hi Tim,

Please take a look.  Thanks.

[tim (not reviewing), 2012-08-17 19:43:14]

http://codereview.chromium.org/10827404/diff/1/chrome/browser/renderer_host/c...
File chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
(right):

http://codereview.chromium.org/10827404/diff/1/chrome/browser/renderer_host/c...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:346:
#if defined(ENABLE_ONE_CLICK_SIGNIN)
How come we're still using an ifdef for this?

Also how do we wind up here exactly?  Is this just for the 200 case, or also the
302 case (i.e. are the calls to show the one-click bar each covering mutually
exclusive cases, or is there some overlap?)

[Roger Tawa OOO till Jul 10th, 2012-08-17 20:09:43]

http://codereview.chromium.org/10827404/diff/1/chrome/browser/renderer_host/c...
File chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
(right):

http://codereview.chromium.org/10827404/diff/1/chrome/browser/renderer_host/c...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:346:
#if defined(ENABLE_ONE_CLICK_SIGNIN)
On 2012/08/17 19:43:14, timsteele wrote:
> How come we're still using an ifdef for this?
> 
> Also how do we wind up here exactly?  Is this just for the 200 case, or also
the
> 302 case (i.e. are the calls to show the one-click bar each covering mutually
> exclusive cases, or is there some overlap?)

Not all platforms use one-click sign in, like android and chromeos for example. 
So this feature is turned on only for desktop.

This case handles 200s, its the OnResponseStarted() callback.  The 302 is
handled just below, line 372, in the OnRequestRedirected() callback.

[tim (not reviewing), 2012-08-17 20:11:23]

Ah, thanks for explaining!

LGTM

[commit-bot: I haz the power, 2012-08-20 13:48:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/10827404/1

[commit-bot: I haz the power, 2012-08-20 13:48:04]

Presubmit check for 10827404-1 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

[Roger Tawa OOO till Jul 10th, 2012-08-20 13:56:02]

Hi Brett,

Can you do an owner review please.  Thanks.

[brettw, 2012-08-20 17:22:17]

lgtm

[commit-bot: I haz the power, 2012-08-20 17:50:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/10827404/1

[commit-bot: I haz the power, 2012-08-20 22:18:36]

Change committed as 152405