Change RulesRegistryService to use ProfileKeyedAPI.

Change RulesRegistryService to use ProfileKeyedAPI.

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

[Patrick Riordan, 2013-04-25 00:40:31]

I completely took out InitForOTR, because I couldn't find the dependency on
ExtensionSystem being properly initialized. It might be a good idea to run some
ChromeOS browser tests on it before trying to commit to see if this is still an
issue.

[Devlin, 2013-04-26 01:07:09]

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
File chrome/browser/extensions/api/declarative/declarative_api.cc (right):

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
chrome/browser/extensions/api/declarative/declarative_api.cc:40:
ProfileKeyedAPIFactory<RulesRegistryService>::GetForProfile(profile());
Add a convenience method in RulesRegistryService for these (see
OmniboxAPI::Get(Profile) for an example).

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:108:
ProfileKeyedAPIFactory<RulesRegistryService>* RulesRegistryService::
prefer line-wrapping of:
ProfileKeyedAPIFactory<RulesRegistryService>*
RulesRegistryService::GetFactoryInstance() {

(note the lack of indent on the second line - weird, I know)

[Patrick Riordan, 2013-04-26 01:44:54]

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
File chrome/browser/extensions/api/declarative/declarative_api.cc (right):

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
chrome/browser/extensions/api/declarative/declarative_api.cc:40:
ProfileKeyedAPIFactory<RulesRegistryService>::GetForProfile(profile());
On 2013/04/26 01:07:09, D Cronin wrote:
> Add a convenience method in RulesRegistryService for these (see
> OmniboxAPI::Get(Profile) for an example).

Done.

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://codereview.chromium.org/13825014/diff/3002/chrome/browser/extensions/...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:108:
ProfileKeyedAPIFactory<RulesRegistryService>* RulesRegistryService::
On 2013/04/26 01:07:09, D Cronin wrote:
> prefer line-wrapping of:
> ProfileKeyedAPIFactory<RulesRegistryService>*
> RulesRegistryService::GetFactoryInstance() {
> 
> (note the lack of indent on the second line - weird, I know)

Done.

[Devlin, 2013-04-26 01:46:00]

lgtm

https://codereview.chromium.org/13825014/diff/17001/chrome/browser/extensions...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://codereview.chromium.org/13825014/diff/17001/chrome/browser/extensions...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:113:
RulesRegistryService* RulesRegistryService::Get(Profile* profile) {
nit: // static

[Patrick Riordan, 2013-04-26 01:53:26]

+ yoz for c/b/extensions/
+ mirandac for c/b/profiles/

https://codereview.chromium.org/13825014/diff/17001/chrome/browser/extensions...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://codereview.chromium.org/13825014/diff/17001/chrome/browser/extensions...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:113:
RulesRegistryService* RulesRegistryService::Get(Profile* profile) {
On 2013/04/26 01:46:00, D Cronin wrote:
> nit: // static

Done.

[Yoyo Zhou, 2013-04-26 03:49:02]

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
File chrome/browser/extensions/extension_system.cc (left):

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
chrome/browser/extensions/extension_system.cc:366: if (extension_service()) {
extension_service() asks for the regular profile's ExtensionSystem's
ExtensionService. If you can't find where RulesRegistryService needs this to be
initialized, I can't find the reason for it in the original CL that added this
code, so you may be able to remove the constraint. (Fingers crossed.)

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
File chrome/browser/extensions/extension_system.h (left):

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
chrome/browser/extensions/extension_system.h:185: OVERRIDE;  // shared
This comment was out of date. It wasn't shared. There should be a separate
instance for the incognito profile (so it should have
ServiceHasOwnInstanceInIncognito; we don't have a switch for that yet in
ProfileKeyedAPI, but go ahead and add one).

[Miranda Callahan, 2013-04-26 09:54:57]

profiles/* LGTM

On 2013/04/26 03:49:02, Yoyo Zhou wrote:
>
https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
> File chrome/browser/extensions/extension_system.cc (left):
> 
>
https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
> chrome/browser/extensions/extension_system.cc:366: if (extension_service()) {
> extension_service() asks for the regular profile's ExtensionSystem's
> ExtensionService. If you can't find where RulesRegistryService needs this to
be
> initialized, I can't find the reason for it in the original CL that added this
> code, so you may be able to remove the constraint. (Fingers crossed.)
> 
>
https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
> File chrome/browser/extensions/extension_system.h (left):
> 
>
https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
> chrome/browser/extensions/extension_system.h:185: OVERRIDE;  // shared
> This comment was out of date. It wasn't shared. There should be a separate
> instance for the incognito profile (so it should have
> ServiceHasOwnInstanceInIncognito; we don't have a switch for that yet in
> ProfileKeyedAPI, but go ahead and add one).

[Patrick Riordan, 2013-05-01 01:30:34]

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
File chrome/browser/extensions/extension_system.h (left):

https://codereview.chromium.org/13825014/diff/24001/chrome/browser/extensions...
chrome/browser/extensions/extension_system.h:185: OVERRIDE;  // shared
On 2013/04/26 03:49:02, Yoyo Zhou wrote:
> This comment was out of date. It wasn't shared. There should be a separate
> instance for the incognito profile (so it should have
> ServiceHasOwnInstanceInIncognito; we don't have a switch for that yet in
> ProfileKeyedAPI, but go ahead and add one).

Done.

[Yoyo Zhou, 2013-05-01 04:28:55]

LGTM

[commit-bot: I haz the power, 2013-05-01 23:26:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/2...

[commit-bot: I haz the power, 2013-05-01 23:26:12]

Failed to apply patch for chrome/browser/extensions/tab_helper.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/tab_helper.cc
  Hunk #1 FAILED at 87.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/tab_helper.cc.rej

Patch:       chrome/browser/extensions/tab_helper.cc
Index: chrome/browser/extensions/tab_helper.cc
diff --git a/chrome/browser/extensions/tab_helper.cc
b/chrome/browser/extensions/tab_helper.cc
index
5fb4541428f4c6b02088f7298493cf44eadab1ef..3336d0a122f0ba2186ba698c2a62b16307f13afd
100644
--- a/chrome/browser/extensions/tab_helper.cc
+++ b/chrome/browser/extensions/tab_helper.cc
@@ -87,10 +87,8 @@ TabHelper::TabHelper(content::WebContents* web_contents)
       pending_web_app_action_(NONE),
       script_executor_(new ScriptExecutor(web_contents,
                                           &script_execution_observers_)),
-      rules_registry_service_(
-          ExtensionSystem::Get(
-              Profile::FromBrowserContext(web_contents->GetBrowserContext()))->
-          rules_registry_service()),
+      rules_registry_service_(RulesRegistryService::Get(
+          Profile::FromBrowserContext(web_contents->GetBrowserContext()))),
       ALLOW_THIS_IN_INITIALIZER_LIST(image_loader_ptr_factory_(this)) {
   // The ActiveTabPermissionManager requires a session ID; ensure this
   // WebContents has one.

[commit-bot: I haz the power, 2013-05-02 00:04:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/4...

[commit-bot: I haz the power, 2013-05-02 00:40:06]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Yoyo Zhou, 2013-05-02 20:43:54]

On 2013/05/02 00:40:06, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on linux_chromeos_clang.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

Looks like you also need to set kServiceIsNULLWhileTesting to true.

[commit-bot: I haz the power, 2013-05-04 20:27:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/6...

[commit-bot: I haz the power, 2013-05-04 20:33:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/7...

[commit-bot: I haz the power, 2013-05-04 20:53:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/8...

[commit-bot: I haz the power, 2013-05-04 21:39:21]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-05-07 23:08:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/9...

[commit-bot: I haz the power, 2013-05-08 17:42:51]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[Patrick Riordan, 2013-05-09 01:05:29]

Yoyo, you might want to take a look at this again. It turns out that it
does depend on extension_service in certain cases. So I just moved what
was in InitForOTR into the constructor. In these browser tests a semi 
initialized RulesRegistryService will be created.

[Yoyo Zhou, 2013-05-09 01:08:53]

LGTM

https://codereview.chromium.org/13825014/diff/120001/chrome/browser/extension...
File chrome/browser/extensions/api/declarative/rules_registry_service.cc
(right):

https://codereview.chromium.org/13825014/diff/120001/chrome/browser/extension...
chrome/browser/extensions/api/declarative/rules_registry_service.cc:47: // tests
are EnterpriseLoginBlocksForEnterpriseUser and
You don't have to name these tests here; this comment will get out of date.

[commit-bot: I haz the power, 2013-05-09 01:17:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/1...

[commit-bot: I haz the power, 2013-05-09 08:31:18]

Retried try job too often on win_rel for step(s) chrome_frame_net_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-09 14:44:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/1...

[commit-bot: I haz the power, 2013-05-09 18:38:42]

Retried try job too often on win_rel for step(s) chrome_frame_net_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-23 23:55:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/1...

[commit-bot: I haz the power, 2013-05-23 23:59:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/1...

[commit-bot: I haz the power, 2013-05-24 02:24:57]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-28 22:29:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/13825014/1...

[commit-bot: I haz the power, 2013-05-29 02:39:15]

Change committed as 202751

[Dan Beam, 2013-05-29 23:18:42]

FYI: this was reverted here - http://crrev.com/202763 - and then I think I found
the cause - http://crre.vomc/202994 - and will attempt to unrevert for you
now...

[Dan Beam, 2013-05-29 23:19:01]

On 2013/05/29 23:18:42, Dan Beam wrote:
> FYI: this was reverted here - http://crrev.com/202763 - and then I think I
found
> the cause - http://crre.vomc/202994 - and will attempt to unrevert for you
> now...

http://crrev.com/202994 **