Fix most tests in //components/autofill/content to not depend on //chrome.

Fix most tests in //components/autofill/content to not depend on //chrome.

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

[blundell, 2013-06-19 15:34:52]

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
File
components/autofill/content/browser/autocheckout/whitelist_manager_unittest.cc
(left):

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
components/autofill/content/browser/autocheckout/whitelist_manager_unittest.cc:97:
profile_.CreateRequestContext();
Why is it OK to remove these calls (here and in the other tests)?

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
File
components/autofill/content/browser/wallet/encryption_escrow_client_unittest.cc
(right):

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
components/autofill/content/browser/wallet/encryption_escrow_client_unittest.cc:112:
content::TestBrowserThread io_thread_;
Is this still needed?

[Jói, 2013-06-19 15:38:04]

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
File
components/autofill/content/browser/autocheckout/whitelist_manager_unittest.cc
(left):

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
components/autofill/content/browser/autocheckout/whitelist_manager_unittest.cc:97:
profile_.CreateRequestContext();
On 2013/06/19 15:34:52, blundell wrote:
> Why is it OK to remove these calls (here and in the other tests)?

In this test, because the URLRequestContextGetter that this creates is not
actually being used.

In the other tests, because content::TestBrowserContext which replaces
TestingProfile, creates the URLRequestContextGetter implicitly, not on a
specific method call.

Tests pass fine after the change, at least locally and I am hopeful on trybots
too.

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
File
components/autofill/content/browser/wallet/encryption_escrow_client_unittest.cc
(right):

https://codereview.chromium.org/17465003/diff/5001/components/autofill/conten...
components/autofill/content/browser/wallet/encryption_escrow_client_unittest.cc:112:
content::TestBrowserThread io_thread_;
On 2013/06/19 15:34:52, blundell wrote:
> Is this still needed?

I'm not sure; let me test.

[Jói, 2013-06-19 17:37:19]

Looks like the io_thread_ thing was no longer needed in a couple of tests;
uploaded a new patchset that removes it.

Cheers,
Jói

[blundell, 2013-06-19 17:53:21]

lgtm

Thanks for the explanations!

[Jói, 2013-06-19 18:06:18]

+isherman@ to sanity check removal of TestBrowserThread IO thread thingies from
tests (blundell@ already reviewed the change).

+phajdan.jr@ for "new" DEPS for //chrome/browser/test (actually just moving an
existing one I think).

[Paweł Hajdan Jr., 2013-06-19 23:00:03]

LGTM with a comment.

https://codereview.chromium.org/17465003/diff/12001/components/autofill/conte...
File components/autofill/content/browser/DEPS (right):

https://codereview.chromium.org/17465003/diff/12001/components/autofill/conte...
components/autofill/content/browser/DEPS:33:
"+chrome/test/base/chrome_render_view_host_test_harness.h",
Why not "!" ?

[Ilya Sherman, 2013-06-19 23:06:14]

Drive-by LGTM with a nit:

https://codereview.chromium.org/17465003/diff/12001/components/autofill/brows...
File components/autofill/browser/autofill_common_test.h (right):

https://codereview.chromium.org/17465003/diff/12001/components/autofill/brows...
components/autofill/browser/autofill_common_test.h:73: void
DisableSystemServices(content::BrowserContext* profile);
nit: Please update the variable name here.

[Jói, 2013-06-21 13:13:23]

https://codereview.chromium.org/17465003/diff/12001/components/autofill/brows...
File components/autofill/browser/autofill_common_test.h (right):

https://codereview.chromium.org/17465003/diff/12001/components/autofill/brows...
components/autofill/browser/autofill_common_test.h:73: void
DisableSystemServices(content::BrowserContext* profile);
On 2013/06/19 23:06:14, Ilya Sherman wrote:
> nit: Please update the variable name here.

Done.

https://codereview.chromium.org/17465003/diff/12001/components/autofill/conte...
File components/autofill/content/browser/DEPS (right):

https://codereview.chromium.org/17465003/diff/12001/components/autofill/conte...
components/autofill/content/browser/DEPS:33:
"+chrome/test/base/chrome_render_view_host_test_harness.h",
On 2013/06/19 23:00:04, Paweł Hajdan Jr. wrote:
> Why not "!" ?

Fixed to use ! here and line below.

[commit-bot: I haz the power, 2013-06-21 13:13:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/17465003/21002

[commit-bot: I haz the power, 2013-06-21 13:13:58]

Failed to apply patch for components/autofill/browser/autofill_common_test.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: components/autofill/browser/autofill_common_test.cc
  |diff --git a/components/autofill/browser/autofill_common_test.cc
b/components/autofill/browser/autofill_common_test.cc
  |index
cd33530a34bb4c02aa81a135411ebb296170aac6..03e1bfabe60d584b9960ffdca46ea1a5c60f3273
100644
  |--- a/components/autofill/browser/autofill_common_test.cc
  |+++ b/components/autofill/browser/autofill_common_test.cc
  --------------------------
  No file to patch.  Skipping patch.
  3 out of 3 hunks ignored

Patch:       components/autofill/browser/autofill_common_test.cc
Index: components/autofill/browser/autofill_common_test.cc
diff --git a/components/autofill/browser/autofill_common_test.cc
b/components/autofill/browser/autofill_common_test.cc
index
cd33530a34bb4c02aa81a135411ebb296170aac6..03e1bfabe60d584b9960ffdca46ea1a5c60f3273
100644
--- a/components/autofill/browser/autofill_common_test.cc
+++ b/components/autofill/browser/autofill_common_test.cc
@@ -7,13 +7,13 @@
 #include "base/guid.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/autofill/browser/autofill_profile.h"
 #include "components/autofill/browser/credit_card.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/user_prefs/user_prefs.h"
 #include "components/webdata/encryptor/encryptor.h"
+#include "content/public/browser/browser_context.h"
 
 namespace autofill {
 namespace test {
@@ -142,7 +142,7 @@ void SetCreditCardInfo(CreditCard* credit_card,
   check_and_set(credit_card, CREDIT_CARD_EXP_4_DIGIT_YEAR, expiration_year);
 }
 
-void DisableSystemServices(Profile* profile) {
+void DisableSystemServices(content::BrowserContext* browser_context) {
   // Use a mock Keychain rather than the OS one to store credit card data.
 #if defined(OS_MACOSX)
   Encryptor::UseMockKeychain(true);
@@ -150,8 +150,8 @@ void DisableSystemServices(Profile* profile) {
 
   // Disable auxiliary profiles for unit testing.  These reach out to system
   // services on the Mac.
-  if (profile) {
-    user_prefs::UserPrefs::Get(profile)->SetBoolean(
+  if (browser_context) {
+    user_prefs::UserPrefs::Get(browser_context)->SetBoolean(
         prefs::kAutofillAuxiliaryProfilesEnabled, false);
   }
 }

[commit-bot: I haz the power, 2013-06-21 13:40:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/17465003/22009

[commit-bot: I haz the power, 2013-06-21 16:41:19]

Change committed as 207859