[Autofill] Fix flakiness in autofill browsertest.

[Autofill] Fix flakiness in autofill browsertest.

BUG=81451
TEST=AutofillTest.*


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

[Ilya Sherman, 2011-12-13 00:02:56]

Thanks for the patch!  Couple of small comments, then should be good to go :)

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
File chrome/browser/autofill/autofill_browsertest.cc (right):

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:69: class
TestPersonalDataManagerObserver : public PersonalDataManagerObserver {
nit: Perhaps name this "WindowedPersonalDataManagerObserver", to match
"WindowedNotificationObserver" in ui_test_utils.h?

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:78: };
I think this class needs to track whether OnPersonalDataChanged() has already
been called by the time Wait() is called -- otherwise, the call to
RunMessageLoop() in Wait() might never be balanced with a Quit()

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:105: observer.Wait();
nit: Please add a comment describing what this is waiting for.

[binji, 2011-12-13 01:54:24]

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
File chrome/browser/autofill/autofill_browsertest.cc (right):

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:69: class
TestPersonalDataManagerObserver : public PersonalDataManagerObserver {
On 2011/12/13 00:02:56, Ilya Sherman wrote:
> nit: Perhaps name this "WindowedPersonalDataManagerObserver", to match
> "WindowedNotificationObserver" in ui_test_utils.h?

Done.

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:78: };
On 2011/12/13 00:02:56, Ilya Sherman wrote:
> I think this class needs to track whether OnPersonalDataChanged() has already
> been called by the time Wait() is called -- otherwise, the call to
> RunMessageLoop() in Wait() might never be balanced with a Quit()

Done.

http://codereview.chromium.org/8921026/diff/1/chrome/browser/autofill/autofil...
chrome/browser/autofill/autofill_browsertest.cc:105: observer.Wait();
On 2011/12/13 00:02:56, Ilya Sherman wrote:
> nit: Please add a comment describing what this is waiting for.

Done.

[Ilya Sherman, 2011-12-13 02:13:44]

LGTM, thanks =)

[commit-bot: I haz the power, 2011-12-13 02:13:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/binji@chromium.org/8921026/4001

[commit-bot: I haz the power, 2011-12-13 03:45:36]

Change committed as 114177