Refactor dependency on Profile to BrowserContext in spell check.

chrome/browser/spellchecker/spellcheck_service_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/path_service.h"
 #include "base/synchronization/waitable_event.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
 #include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/browser/ui/browser.h"
@@ skipping 43 matching lines @@
   EXPECT_EQ(arraysize(kCorruptedBDICT), actual);
 
   // Attach an event to the SpellcheckService object so we can receive its
   // status updates.
   base::WaitableEvent event(true, false);
   SpellcheckService::AttachStatusEvent(&event);
 
   // Ensure that the SpellcheckService object does not already exist. Otherwise
   // the next line will not force creation of the SpellcheckService and the
   // test will fail.
-  SpellcheckService* service =
-      SpellcheckServiceFactory::GetForProfileWithoutCreating(GetProfile());
+  SpellcheckService* service = static_cast<SpellcheckService*>(
+        GetInstance()->GetServiceForBrowserContext(context, false));

[groby-ooo-7-16, 2013/09/10 19:09:51]

Does this get the context somewhere? Can't see that, might need to add it

[msmith.v3, 2013/09/10 19:27:50]

Yep - copy-paste error. Sure that's going to break something. Will fix that
momentarily.

[msmith.v3, 2013/09/11 19:25:07]

This turned out to be problematic. The reason for the original
SpellcheckServiceFactory::GetForProfileWithoutCreating method was
GetServiceForBrowserContext() is protected - I can't call it here. This line
seems to only be here to make debugging the test a little easier - the real test
is the actual creation of the service and the event trigger. I see two options -
remove this line and the ASSERT_EQ, or put back the
SpellcheckServiceFactory::GetForContextWithoutCreating method.just for this
test. Thoughts?

[groby-ooo-7-16, 2013/09/12 21:35:53]

Expose it in spellcheck_factory.h via
FRIEND_TEST_ALL_PREFIXED(SpellcheckServiceBrowserTest, DeleteCorruptedBDICT); 

(goes into the private: section)

On 2013/09/11 19:25:07, msmith.v3 wrote:

[msmith.v3, 2013/09/13 17:02:14]

Done.

   ASSERT_EQ(NULL, service);
 
   // Getting the spellcheck_service will initialize the SpellcheckService
   // object with the corrupted BDICT file created above since the hunspell
   // dictionary is loaded in the SpellcheckService constructor right now.
   // The SpellCheckHost object will send a BDICT_CORRUPTED event.
-  SpellcheckServiceFactory::GetForProfile(GetProfile());
+  SpellcheckServiceFactory::GetForContext(GetProfile());

[groby-ooo-7-16, 2013/09/10 19:09:51]

Use context.

[msmith.v3, 2013/09/10 19:27:50]

What's the proper way to grab the context inside one of these types of tests? I
see a way to grab the profile; should I just rename the GetProfile() method in
this case?

[groby-ooo-7-16, 2013/09/10 20:48:07]

GetProfile() is fine - I was just assuming you'd assign it to context and reuse
that here :)

On 2013/09/10 19:27:50, msmith.v3 wrote:

[msmith.v3, 2013/09/13 17:02:14]

Done.

 
   // Check the received event. Also we check if Chrome has successfully deleted
   // the corrupted dictionary. We delete the corrupted dictionary to avoid
   // leaking it when this test fails.
   content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
   content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
   EXPECT_EQ(SpellcheckService::BDICT_CORRUPTED,
             SpellcheckService::GetStatusEvent());
   if (base::PathExists(bdict_path)) {
     ADD_FAILURE();
     EXPECT_TRUE(base::DeleteFile(bdict_path, true));
   }
 }