Fix unit test leak in DeviceOAuth2TokenServiceTest.

Fix unit test leak in DeviceOAuth2TokenServiceTest.

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

[David Roche, 2013-04-19 02:53:30]

The trybot failures seem unrelated to my change.

I verified locally that memcheck failed before my change and passed after.

[Alexander Potapenko, 2013-04-19 12:16:39]

On 2013/04/19 02:53:30, David Roche wrote:
> The trybot failures seem unrelated to my change.
> 
> I verified locally that memcheck failed before my change and passed after.

LGTM

[commit-bot: I haz the power, 2013-04-19 16:41:48]

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

[commit-bot: I haz the power, 2013-04-19 16:41:49]

Failed to apply patch for tools/valgrind/memcheck/suppressions.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/valgrind/memcheck/suppressions.txt
  Hunk #1 FAILED at 7069.
  1 out of 1 hunk FAILED -- saving rejects to file
tools/valgrind/memcheck/suppressions.txt.rej

Patch:       tools/valgrind/memcheck/suppressions.txt
Index: tools/valgrind/memcheck/suppressions.txt
diff --git a/tools/valgrind/memcheck/suppressions.txt
b/tools/valgrind/memcheck/suppressions.txt
index
f96b98e8a2fedb5038ccd49baea8e7aa0174cc1b..89264a962cd02947bb7a0d37f0e4448d02f086a2
100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -7069,9 +7069,3 @@
    ...
   
fun:_ZN10extensions51WebstoreInlineInstallerTest_DomainVerification_Test8TestBodyEv
 }
-{
-   bug_233149
-   Memcheck:Leak
-   fun:_Znw*
-  
fun:_ZN8chromeos52DeviceOAuth2TokenServiceTest_SaveEncryptedToken_Test8TestBodyEv
-}

[commit-bot: I haz the power, 2013-04-19 16:56:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidroche@chromium.org/14367006/6001

[commit-bot: I haz the power, 2013-04-19 16:56:41]

Presubmit check for 14367006-6001 failed and returned exit status 1.

INFO:root:Found 2 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/tools/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/tools/valgrind/memcheck/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[David Roche, 2013-04-19 16:58:10]

Mattias, can you approve the Chrome OS change?

[Mattias Nissler (ping if slow), 2013-04-19 17:45:19]

You have my LGTM to commit this provided you understand why this change actually
makes the memory leak go away :)

https://codereview.chromium.org/14367006/diff/6001/chrome/browser/chromeos/se...
File chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc
(right):

https://codereview.chromium.org/14367006/diff/6001/chrome/browser/chromeos/se...
chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc:64: new
net::TestURLRequestContextGetter(message_loop_.message_loop_proxy()),
I don't see how this fixes the problem, can you enlighten me?

[David Roche, 2013-04-19 22:05:03]

https://codereview.chromium.org/14367006/diff/6001/chrome/browser/chromeos/se...
File chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc
(right):

https://codereview.chromium.org/14367006/diff/6001/chrome/browser/chromeos/se...
chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc:64: new
net::TestURLRequestContextGetter(message_loop_.message_loop_proxy()),
On 2013/04/19 17:45:19, Mattias Nissler wrote:
> I don't see how this fixes the problem, can you enlighten me?

So, the key is in URLRequestContextGetter::OnDestruct().  If it has a task
runner on the current thread it will immediately 'delete this', but if the task
runner is on a diff thread, then it will

  network_task_runner->DeleteSoon(FROM_HERE, this)

which would only run if my test called RunUntilIdle() on that task runner, which
it wasn't.

However, since this test isn't even using the URLRequestContextGetter anyway
(it's just needed to pass a DCHECK), it was simpler to just give it a task
runner from the current message loop and thereby trigger an immediate delete.

[commit-bot: I haz the power, 2013-04-19 22:06:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davidroche@chromium.org/14367006/6001

[commit-bot: I haz the power, 2013-04-20 01:51:03]

Change committed as 195365