components: Extract pref_registry component out of user_prefs.

components: Extract pref_registry component out of user_prefs.

The reason for doing this is that user_prefs depends on content_browser,
which brings a lot of extra dependencies that takes a lot of time to build
(yeah, I'm looking at you Blink and V8) and bookmarks is one of the targets
that should not depend on content.

BUG=359565
TEST=green trybots
R=blundell@chromium.org, sky@chromium.org, yoz@chromium.org

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

[tfarina, 2014-05-08 04:03:46]

ptal.

this also benefits autofill_core_browser.

[tfarina, 2014-05-08 04:37:09]

+Yoyo for extensions (rubber stamp?).

[Yoyo Zhou, 2014-05-08 22:16:44]

extensions LGTM

[blundell, 2014-05-09 08:26:23]

Thanks! Looks good, but why not move it to //components/pref_registry in this
CL? Are you planning to do that in an immediate followup?

https://codereview.chromium.org/271793003/diff/30002/components/user_prefs/pr...
File components/user_prefs/pref_registry_export.h (right):

https://codereview.chromium.org/271793003/diff/30002/components/user_prefs/pr...
components/user_prefs/pref_registry_export.h:5: #ifndef
COMPONENTS_PREF_REGISTRY_PREF_REGISTRY_EXPORT_H_
This path is wrong.

[tfarina, 2014-05-09 17:27:17]

On 2014/05/09 08:26:23, blundell wrote:
> Thanks! Looks good, but why not move it to //components/pref_registry in this
> CL?
You can see in patch set 5 the pain I wanted to avoid.

Done. I went ahead and created it.

Colin, please, take another look.

https://codereview.chromium.org/271793003/diff/30002/components/user_prefs/pr...
File components/user_prefs/pref_registry_export.h (right):

https://codereview.chromium.org/271793003/diff/30002/components/user_prefs/pr...
components/user_prefs/pref_registry_export.h:5: #ifndef
COMPONENTS_PREF_REGISTRY_PREF_REGISTRY_EXPORT_H_
On 2014/05/09 08:26:23, blundell wrote:
> This path is wrong.

Done.

[blundell, 2014-05-12 07:13:31]

LGTM with nits

https://codereview.chromium.org/271793003/diff/140001/components/autofill/cor...
File components/autofill/core/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/autofill/cor...
components/autofill/core/DEPS:3:
"+components/pref_registry/pref_registry_syncable.h",
nit: It would be fine for this to be just components/pref_registry now.

https://codereview.chromium.org/271793003/diff/140001/components/password_man...
File components/password_manager/core/browser/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/password_man...
components/password_manager/core/browser/DEPS:4: "+content/public/browser",
Nit: Could you remove this line while you're here? It's wrong and doesn't appear
to be necessary.

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
File components/pref_registry/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
components/pref_registry/DEPS:2: "-content",
Nit: This isn't actually necessary.

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
File components/pref_registry/pref_registry_syncable.h (right):

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
components/pref_registry/pref_registry_syncable.h:22: namespace user_prefs {
This namespace should be pref_registry now. I'm fine with that happening in a
followup to avoid increasing noise on this CL.

[tfarina, 2014-05-12 20:49:21]

Scott, I need your approval for the chrome/ changes (all trivial).

https://codereview.chromium.org/271793003/diff/140001/components/autofill/cor...
File components/autofill/core/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/autofill/cor...
components/autofill/core/DEPS:3:
"+components/pref_registry/pref_registry_syncable.h",
On 2014/05/12 07:13:32, blundell wrote:
> nit: It would be fine for this to be just components/pref_registry now.

Done.

https://codereview.chromium.org/271793003/diff/140001/components/password_man...
File components/password_manager/core/browser/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/password_man...
components/password_manager/core/browser/DEPS:4: "+content/public/browser",
On 2014/05/12 07:13:32, blundell wrote:
> Nit: Could you remove this line while you're here? It's wrong and doesn't
appear
> to be necessary.

Done.

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
File components/pref_registry/DEPS (right):

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
components/pref_registry/DEPS:2: "-content",
On 2014/05/12 07:13:32, blundell wrote:
> Nit: This isn't actually necessary.

Done.

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
File components/pref_registry/pref_registry_syncable.h (right):

https://codereview.chromium.org/271793003/diff/140001/components/pref_registr...
components/pref_registry/pref_registry_syncable.h:22: namespace user_prefs {
On 2014/05/12 07:13:32, blundell wrote:
> This namespace should be pref_registry now. I'm fine with that happening in a
> followup to avoid increasing noise on this CL.
Added a TODO in my name.

[sky, 2014-05-12 22:05:49]

https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS#new...
chrome/browser/DEPS:37: "+components/pref_registry",
How about putting this in chrome/browser and chrome/test and not here?

[tfarina, 2014-05-13 02:41:36]

https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS#new...
chrome/browser/DEPS:37: "+components/pref_registry",
On 2014/05/12 22:05:50, sky wrote:
> How about putting this in chrome/browser and chrome/test and not here?

Moved this entry to chrome/DEPS, assuming that is what you meant.

chrome/test/DEPS already has an entry for components:
https://chromium.googlesource.com/chromium/chromium/+/trunk/chrome/test/DEPS

[sky, 2014-05-13 15:49:49]

On Mon, May 12, 2014 at 7:41 PM,  <tfarina@chromium.org> wrote:
>
> https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
> File chrome/browser/DEPS (right):
>
>
https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS#new...
> chrome/browser/DEPS:37: "+components/pref_registry",
> On 2014/05/12 22:05:50, sky wrote:
>>
>> How about putting this in chrome/browser and chrome/test and not here?
>
>
> Moved this entry to chrome/DEPS, assuming that is what you meant.

No. I want it only were needed: chrome/browser. We don't want to allow
using this from chrome/common.

  -Scott

>
> chrome/test/DEPS already has an entry for components:
> https://chromium.googlesource.com/chromium/chromium/+/trunk/chrome/test/DEPS
>
> https://codereview.chromium.org/271793003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-05-13 16:04:51]

On Tue, May 13, 2014 at 12:49 PM, Scott Violet <sky@chromium.org> wrote:

> On Mon, May 12, 2014 at 7:41 PM,  <tfarina@chromium.org> wrote:
> >
> >
> https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
> > File chrome/browser/DEPS (right):
> >
> >
>
https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS#new...
> > chrome/browser/DEPS:37: "+components/pref_registry",
> > On 2014/05/12 22:05:50, sky wrote:
> >>
> >> How about putting this in chrome/browser and chrome/test and not here?
> >
> >
> > Moved this entry to chrome/DEPS, assuming that is what you meant.
>
> No. I want it only were needed: chrome/browser. We don't want to allow
> using this from chrome/common.
>
> So why did you ask me to move it out of chrome/browser/DEPS? Or did you
confuse and thought it was in chrome/DEPS rather than chrome/browser/DEPS,
because you said " How about putting this in chrome/browser", while it was
already there in patch set 9? There was only an entry in
chrome/browser/DEPS at patch set 9.

https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2014-05-13 16:32:31]

Sorry, for some reason I thought you initially had it in chrome/DEPS.
chrome/browser/DEPS is the right place.

  -Scott

On Tue, May 13, 2014 at 9:04 AM, Thiago Farina <tfarina@chromium.org> wrote:
>
>
>
> On Tue, May 13, 2014 at 12:49 PM, Scott Violet <sky@chromium.org> wrote:
>>
>> On Mon, May 12, 2014 at 7:41 PM,  <tfarina@chromium.org> wrote:
>> >
>> >
>> > https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
>> > File chrome/browser/DEPS (right):
>> >
>> >
>> >
https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS#new...
>> > chrome/browser/DEPS:37: "+components/pref_registry",
>> > On 2014/05/12 22:05:50, sky wrote:
>> >>
>> >> How about putting this in chrome/browser and chrome/test and not here?
>> >
>> >
>> > Moved this entry to chrome/DEPS, assuming that is what you meant.
>>
>> No. I want it only were needed: chrome/browser. We don't want to allow
>> using this from chrome/common.
>>
> So why did you ask me to move it out of chrome/browser/DEPS? Or did you
> confuse and thought it was in chrome/DEPS rather than chrome/browser/DEPS,
> because you said " How about putting this in chrome/browser", while it was
> already there in patch set 9? There was only an entry in chrome/browser/DEPS
> at patch set 9.
>
> https://codereview.chromium.org/271793003/diff/160001/chrome/browser/DEPS
>
> --
> Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-05-14 03:47:43]

Scott, I moved the entry back to chrome/browser/DEPS (I think that is the right
thing to do). We already have an entry in chrome/test/DEPS.

Holding it causes much pain on me because I have to deal with merge conflicts
every time I rebase and it takes a long time to pass the presubmit to upload
(because it is a large change).

[sky, 2014-05-14 18:19:18]

LGTM

[tfarina, 2014-05-14 20:29:54]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-14 20:30:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/271793003/260001

[commit-bot: I haz the power, 2014-05-14 21:26:51]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-05-14 21:30:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-14 21:30:43]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)

[tfarina, 2014-05-14 23:41:05]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-14 23:43:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/271793003/280001

[tfarina, 2014-05-15 00:11:14]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-15 00:14:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/271793003/300001

[commit-bot: I haz the power, 2014-05-15 03:27:51]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)

[commit-bot: I haz the power, 2014-05-15 07:07:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-15 07:07:39]

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

Patch:       chrome/browser/extensions/external_component_loader.cc
Index: chrome/browser/extensions/external_component_loader.cc
diff --git a/chrome/browser/extensions/external_component_loader.cc
b/chrome/browser/extensions/external_component_loader.cc
index
98e332171c3cb186b27b1fbd53ad093872b6d248..1d0e0e4b0bc1e4b8298f883d6d251657d844d4c9
100644
--- a/chrome/browser/extensions/external_component_loader.cc
+++ b/chrome/browser/extensions/external_component_loader.cc
@@ -15,8 +15,8 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
 #include "components/sync_driver/pref_names.h"
-#include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace extensions {

[tfarina, 2014-05-15 09:27:42]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-15 09:27:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/271793003/310001

[commit-bot: I haz the power, 2014-05-15 11:03:08]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-15 11:06:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-15 11:06:10]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[tfarina, 2014-05-15 14:34:11]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-15 14:35:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/271793003/310001

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

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-05-15 16:01:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-15 16:01:51]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[tfarina, 2014-05-15 17:37:38]

Committed patchset #17 manually as r270722 (presubmit successful).