Prevent automation from reporting login WebUI ready prematurely

Prevent automation from reporting login WebUI ready prematurely

One of the notifications telling the automation provider that the login
WebUI is ready might actually occur in a transient phase while the
browser is starting up and no network has been detected quite yet.

BUG=chromium-os:31067
TEST=Chrome builds, functional tests run


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

[bartfab (slow), 2012-05-18 15:48:04]

Hi Nirnimesh,

You added a comment to CL 10382197 when it was in the CQ already. I only saw the
comment after the CL had landed.

You were asking when the chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN gets
fired. My understanding was that this notification occurs when the attempt to
prepare the login form ends in a network error. This should never be the case in
our tests so that whether the code listens for the notification or not would
make zero difference.

But I noticed the recent VMTest flakiness. It seems to be in tests that attempt
to log in and then do something. I am wondering whether the intuition behind
your question might have been right after all: It may be that as Chrome is
starting up and goes through the transient phase with no network available quite
yet, the notification does get fired. This would signal to PyAuto prematurely
that the login form is ready to go. PyAuto tries to log in and... times out.

If this is indeed the source of the recent flakiness, this CL should fix it.
Could you please review?

[craigdh, 2012-05-18 16:13:17]

On 2012/05/18 15:48:04, bartfab wrote:
> Hi Nirnimesh,
> 
> You added a comment to CL 10382197 when it was in the CQ already. I only saw
the
> comment after the CL had landed.
> 
> You were asking when the chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN gets
> fired. My understanding was that this notification occurs when the attempt to
> prepare the login form ends in a network error. This should never be the case
in
> our tests so that whether the code listens for the notification or not would
> make zero difference.
> 
> But I noticed the recent VMTest flakiness. It seems to be in tests that
attempt
> to log in and then do something. I am wondering whether the intuition behind
> your question might have been right after all: It may be that as Chrome is
> starting up and goes through the transient phase with no network available
quite
> yet, the notification does get fired. This would signal to PyAuto prematurely
> that the login form is ready to go. PyAuto tries to log in and... times out.
> 
> If this is indeed the source of the recent flakiness, this CL should fix it.
> Could you please review?

May also be relevant to http://crosbug.com/30997 and http://crosbug.com/30880

[bartfab (slow), 2012-05-18 16:53:50]

Thanks for pointing out these other bugs. crosbug.com/30880 is especially
interesting as it was filed before my CL landed. So the flakiness was there
before my CL went in. Still, I would like to try this patch to see whether it is
responsible for some additional flakiness.

[Nirnimesh, 2012-05-18 18:54:09]

I don't think  http://crosbug.com/30997 and http://crosbug.com/30880 are
related. 

30997 is a browser crash chromeos::CryptohomeLibraryImpl::LoadSystemSalt
30880 happened after your CL.

That said, this CL should probably go in regardless. If you could, please verify
when this notification is raised, and until then we should not observe for this
notification.

LGTM

[bartfab (slow), 2012-05-18 19:40:42]

Thanks. And yes, I will investigate further what exactly causes this
notification to fire.

[commit-bot: I haz the power, 2012-05-18 19:41:17]

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

[Nirnimesh, 2012-05-18 20:54:19]

On 2012/05/18 18:54:09, Nirnimesh wrote:
> I don't think  http://crosbug.com/30997 and http://crosbug.com/30880 are
> related. 
> 
> 30997 is a browser crash chromeos::CryptohomeLibraryImpl::LoadSystemSalt
> 30880 happened after your CL.

correction: s/after/before/

[commit-bot: I haz the power, 2012-05-18 21:18:10]

Change committed as 137959