RegistryEntry: Moved ApplicationInfo and static methods out of class.

RegistryEntry: Moved ApplicationInfo and static methods out of class.

shell_util's RegistryEntry previously contained a struct ApplicationInfo
and a bunch of static methods for specific tasks (that are not intrinsic
to the RegistryEntry class). Really, they are just clients of
RegistryEntry that were contained inside it because its constructor was
private. Now, they are just in shell_util's anonymous namespace.

BUG=419972
Committed: https://crrev.com/1a01eb332d33162f6287cdf8bad995e8802c530b
Cr-Commit-Position: refs/heads/master@{#362608}

[Matt Giuca, 2015-11-30 02:55:09]

mgiuca@chromium.org changed reviewers:
+ gab@chromium.org

[Matt Giuca, 2015-11-30 02:55:09]

Follow-up to https://codereview.chromium.org/1486493002/.

[Matt Giuca, 2015-11-30 03:05:54]

Patchset #2 (id:20001) has been deleted

[Matt Giuca, 2015-11-30 03:11:10]

Added notes explaining some of the large code moves. As with
https://codereview.chromium.org/1472273003, the big adds and removes are
unavoidable, but I haven't changed any code, just moved it. (Though I did run
ClangFormat and reflow the comments to fit the new space.)

https://codereview.chromium.org/1486513002/diff/40001/chrome/installer/util/s...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/1486513002/diff/40001/chrome/installer/util/s...
chrome/installer/util/shell_util.cc:325: void
RegistryEntry::AddToWorkItemList(HKEY root, WorkItemList* items) const {
Note: These method definitions were moved up so they appear right under the
class (we don't want them below the functions that are no longer part of this
class).

I haven't changed any code inside these methods, just moved the code.

https://codereview.chromium.org/1486513002/diff/40001/chrome/installer/util/s...
chrome/installer/util/shell_util.cc:480: void GetProgIdEntries(const
ApplicationInfo& app_info,
Note: This function got swapped with GetChromeProgIdEntries to avoid having to
forward-declare it.

https://codereview.chromium.org/1486513002/diff/40001/chrome/installer/util/s...
chrome/installer/util/shell_util.cc:685: void GetAppExtRegistrationEntries(const
base::string16& prog_id,
Note: This function got swapped with GetChromeAppRegistrationEntries to avoid
having to forward-declare it.

[gab, 2015-11-30 13:33:12]

lgtm as long as these are moved to their own file (e.g.
shell_registry_util.(h|cc) after RegistryEntry is made public; otherwise it
makes the registry code too scattered in this humongous file IMO. I'd mentioned
this a while back when these discussions started and I think we agree on this
but just making sure.

Thanks!

[Matt Giuca, 2015-12-01 00:51:01]

Yes, the plan is to move it to a separate file, but I am doing everything in
baby steps this time (since last time I tried to do it in one big go, it was
much too hard to review).

[gab, 2015-12-01 20:11:45]

On 2015/12/01 00:51:01, Matt Giuca wrote:
> Yes, the plan is to move it to a separate file, but I am doing everything in
> baby steps this time (since last time I tried to do it in one big go, it was
> much too hard to review).

Indeed, I really like the baby steps approach you're taking, just confirming we
have the same path in mind :-).

Thanks,
Gab

[Matt Giuca, 2015-12-01 22:59:27]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-12-01 22:59:28]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/1486513002/#ps60001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-12-01 23:00:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1486513002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1486513002/60001

[commit-bot: I haz the power, 2015-12-01 23:46:09]

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

[commit-bot: I haz the power, 2015-12-01 23:46:10]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Matt Giuca, 2015-12-02 02:19:03]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2015-12-02 02:21:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1486513002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1486513002/60001

[commit-bot: I haz the power, 2015-12-02 02:58:09]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-12-02 02:59:16]

Description was changed from

==========
RegistryEntry: Moved ApplicationInfo and static methods out of class.

shell_util's RegistryEntry previously contained a struct ApplicationInfo
and a bunch of static methods for specific tasks (that are not intrinsic
to the RegistryEntry class). Really, they are just clients of
RegistryEntry that were contained inside it because its constructor was
private. Now, they are just in shell_util's anonymous namespace.

BUG=419972
==========

to

==========
RegistryEntry: Moved ApplicationInfo and static methods out of class.

shell_util's RegistryEntry previously contained a struct ApplicationInfo
and a bunch of static methods for specific tasks (that are not intrinsic
to the RegistryEntry class). Really, they are just clients of
RegistryEntry that were contained inside it because its constructor was
private. Now, they are just in shell_util's anonymous namespace.

BUG=419972

Committed: https://crrev.com/1a01eb332d33162f6287cdf8bad995e8802c530b
Cr-Commit-Position: refs/heads/master@{#362608}
==========

[commit-bot: I haz the power, 2015-12-02 02:59:17]

Patchset 3 (id:??) landed as
https://crrev.com/1a01eb332d33162f6287cdf8bad995e8802c530b
Cr-Commit-Position: refs/heads/master@{#362608}

[gab, 2016-01-11 16:29:12]

On 2015/11/30 13:33:12, gab wrote:
> lgtm as long as these are moved to their own file (e.g.
> shell_registry_util.(h|cc) after RegistryEntry is made public; otherwise it
> makes the registry code too scattered in this humongous file IMO. I'd
mentioned
> this a while back when these discussions started and I think we agree on this
> but just making sure.


ping on this, I know you've been making steady progress towards this but just
want to make sure it's still in the plans after the holidays to complete this as
mentioned in comment #7.

Thanks,
Gab

[Matt Giuca, 2016-01-11 23:48:34]

On 2016/01/11 16:29:12, gab wrote:
> On 2015/11/30 13:33:12, gab wrote:
> > lgtm as long as these are moved to their own file (e.g.
> > shell_registry_util.(h|cc) after RegistryEntry is made public; otherwise it
> > makes the registry code too scattered in this humongous file IMO. I'd
> mentioned
> > this a while back when these discussions started and I think we agree on
this
> > but just making sure.
> 
> 
> ping on this, I know you've been making steady progress towards this but just
> want to make sure it's still in the plans after the holidays to complete this
as
> mentioned in comment #7.
> 
> Thanks,
> Gab

Um I forgot about this. Thanks for the reminder.