Move ProgID methods from BrowserDistribution into install_static.

Move ProgID methods from BrowserDistribution into install_static.

This is another step in bringing brand-specific constants together into
one place, eventually leading to the demise of BrowserDistribution.

BUG=373987
Review-Url: https://codereview.chromium.org/2764963002
Cr-Commit-Position: refs/heads/master@{#458693}
Committed: https://chromium.googlesource.com/chromium/src/+/ddefa6dd9d43afc790ea00b386bd028072d30349

[grt (UTC plus 2), 2017-03-21 15:04:58]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[grt (UTC plus 2), 2017-03-21 15:05:09]

grt@chromium.org changed reviewers:
+ huangs@chromium.org

[grt (UTC plus 2), 2017-03-21 15:05:10]

PTAL

[commit-bot: I haz the power, 2017-03-21 15:05:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[huangs, 2017-03-21 15:35:48]

LGTM with NITs.

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
File chrome/install_static/install_modes_unittest.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_modes_unittest.cc:56:
ASSERT_THAT(lstrlen(mode.prog_id_prefix), Le(11));
https://msdn.microsoft.com/library/windows/desktop/dd542719.aspx
also states:
- Contain no punctuation (including underscores) except one or more periods.
- Not start with a digit.

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
File chrome/install_static/install_util.h (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_util.h:83: // Returns the browser's ProgID prefix
(e.g., ChromeHTML, ChromiumHTM, etc...).
NIT: "etc..." => "etc." or "..."

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_util.h:88: // therefore the return value of this
function must have a maximum length of 39
Might be nice to keep the formula on its own line.

Reminder that you removed link to http://crbug.com/153349, when you removed
comments in browser_distribution.cc. Don't know if you want to keep that.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
chrome/installer/setup/uninstall.cc:641:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
|dist| gets used on line 632. Move the DCHECK there?

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
chrome/installer/setup/uninstall.cc:654:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
This check is now redundant.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:352:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
Pointer comparison! Also, if we can get BrowserDistribution::GetDistribution(),
then we can remove |dist| param? If this is transient code before refactoring,
please add TODO. If it's not worth refactoring, please add comment saying this
is legacy code.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:797:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
Same as above.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:1039: base::string16
prog_id(install_static::GetProgIdPrefix());
Note: These 3 lines can be combined, depending on your taste (personally I
wouldn't).

[commit-bot: I haz the power, 2017-03-21 15:41:50]

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

[commit-bot: I haz the power, 2017-03-21 15:41:52]

Dry run: This issue passed the CQ dry run.

[grt (UTC plus 2), 2017-03-22 08:03:36]

Thanks.

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
File chrome/install_static/install_modes_unittest.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_modes_unittest.cc:56:
ASSERT_THAT(lstrlen(mode.prog_id_prefix), Le(11));
On 2017/03/21 15:35:48, huangs wrote:
> https://msdn.microsoft.com/library/windows/desktop/dd542719.aspx
> also states:
> - Contain no punctuation (including underscores) except one or more periods.
> - Not start with a digit.

tests added

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
File chrome/install_static/install_util.h (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_util.h:83: // Returns the browser's ProgID prefix
(e.g., ChromeHTML, ChromiumHTM, etc...).
On 2017/03/21 15:35:48, huangs wrote:
> NIT: "etc..." => "etc." or "..."

Done.

https://codereview.chromium.org/2764963002/diff/1/chrome/install_static/insta...
chrome/install_static/install_util.h:88: // therefore the return value of this
function must have a maximum length of 39
On 2017/03/21 15:35:48, huangs wrote:
> Might be nice to keep the formula on its own line.

Done.

> Reminder that you removed link to http://crbug.com/153349, when you removed
> comments in browser_distribution.cc. Don't know if you want to keep that.

Thanks for checking. I don't think there's a need for that comment any longer.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
File chrome/installer/setup/uninstall.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
chrome/installer/setup/uninstall.cc:641:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
On 2017/03/21 15:35:48, huangs wrote:
> |dist| gets used on line 632. Move the DCHECK there?

I have it here because it's relevant for the call to GetProgIdPrefix that
immediately follows. These will all go away when BrowserDistribution dies.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/setup/unin...
chrome/installer/setup/uninstall.cc:654:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
On 2017/03/21 15:35:48, huangs wrote:
> This check is now redundant.

Done.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
File chrome/installer/util/shell_util.cc (right):

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:352:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
On 2017/03/21 15:35:48, huangs wrote:
> Pointer comparison! Also, if we can get
BrowserDistribution::GetDistribution(),
> then we can remove |dist| param? If this is transient code before refactoring,
> please add TODO. If it's not worth refactoring, please add comment saying this
> is legacy code.

Done.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:797:
DCHECK_EQ(BrowserDistribution::GetDistribution(), dist);
On 2017/03/21 15:35:48, huangs wrote:
> Same as above.

Done.

https://codereview.chromium.org/2764963002/diff/1/chrome/installer/util/shell...
chrome/installer/util/shell_util.cc:1039: base::string16
prog_id(install_static::GetProgIdPrefix());
On 2017/03/21 15:35:48, huangs wrote:
> Note: These 3 lines can be combined, depending on your taste (personally I
> wouldn't).

Acknowledged.

[grt (UTC plus 2), 2017-03-22 08:03:41]

The CQ bit was checked by grt@chromium.org

[grt (UTC plus 2), 2017-03-22 08:03:41]

The patchset sent to the CQ was uploaded after l-g-t-m from huangs@chromium.org
Link to the patchset: https://codereview.chromium.org/2764963002/#ps20001
(title: "huangs comments")

[commit-bot: I haz the power, 2017-03-22 08:03:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 09:03:32]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490169821260210,
"parent_rev": "7d4caadfaa86b424f3c2459563aa5a369f468976", "commit_rev":
"ddefa6dd9d43afc790ea00b386bd028072d30349"}

[commit-bot: I haz the power, 2017-03-22 09:04:13]

Description was changed from

==========
Move ProgID methods from BrowserDistribution into install_static.

This is another step in bringing brand-specific constants together into
one place, eventually leading to the demise of BrowserDistribution.

BUG=373987
==========

to

==========
Move ProgID methods from BrowserDistribution into install_static.

This is another step in bringing brand-specific constants together into
one place, eventually leading to the demise of BrowserDistribution.

BUG=373987

Review-Url: https://codereview.chromium.org/2764963002
Cr-Commit-Position: refs/heads/master@{#458693}
Committed:
https://chromium.googlesource.com/chromium/src/+/ddefa6dd9d43afc790ea00b386bd...
==========

[commit-bot: I haz the power, 2017-03-22 09:04:14]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ddefa6dd9d43afc790ea00b386bd...