Abstract suffixing logic away from GetApplicationName

Abstract suffixing logic away from GetApplicationName


BUG=124013, 125362, 133173
TEST=http://goo.gl/ZZ7gE

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142641

[gab, 2012-06-01 00:12:58]

This will go on top of http://codereview.chromium.org/10451074/

The idea is to always suffix GetAppName() unless not-suffixing (or
custom-suffixing) is the actual desired behavior (in which case the user can
call a more specific function GetUnsuffixedAppName()). The suffixing work is
done in browser_distribution.cc for all distributions who only have to give back
their unsuffixed appid.

GetApplicationName() was renamed to GetAppName() so that I could make absolutely
sure I didn't forget any call sites.

The replacement of the GetApplicationName calls were made as such:
 - Logging calls not using suffixes => GetUnsuffixedAppName()
 - Calls who would immediately after GetInstallationSuffix() and add it =>
GetAppName()
 - Calls who would add a suffix they were passed in from somewhere else (i.e.
installer/uninstall) => GetUnsuffixedAppName() + suffix.

I also changed some GetApplicationName() to GetAppShortcutName() where a
localized string is the desired rather than the raw app name.

Let me know what you think, I'm getting a resource DCHECK now related to
l10_string_util when calling GetAppShortcutName from Chrome, I think this is
something greg adressed recently?, I'll sync and rebuild over night see if that
helps.

Cheers,
Gab

PS: The plan is to do the same thing for GetAppId of course in the appid patch.

[grt (UTC plus 2), 2012-06-01 14:11:29]

http://codereview.chromium.org/10446111/diff/1/chrome/installer/util/browser_...
File chrome/installer/util/browser_distribution.cc (right):

http://codereview.chromium.org/10446111/diff/1/chrome/installer/util/browser_...
chrome/installer/util/browser_distribution.cc:155: app_name +=
ShellUtil::GetCurrentInstallationSuffix();
Please move this functionality into ShellUtil and change consumers so that
instead of calling BrowserDistribution::GetApplicationName and then doing stuff
to it, they call ShellUtil::GetApplicationName (or somesuch), which gets the
base part from a BrowserDistribution* and then adds the suffix if needed.

[gab, 2012-06-01 16:29:18]

Done, same logic as before was kept for the new mapping.

http://codereview.chromium.org/10446111/diff/1/chrome/installer/util/browser_...
File chrome/installer/util/browser_distribution.cc (right):

http://codereview.chromium.org/10446111/diff/1/chrome/installer/util/browser_...
chrome/installer/util/browser_distribution.cc:155: app_name +=
ShellUtil::GetCurrentInstallationSuffix();
On 2012/06/01 14:11:29, grt wrote:
> Please move this functionality into ShellUtil and change consumers so that
> instead of calling BrowserDistribution::GetApplicationName and then doing
stuff
> to it, they call ShellUtil::GetApplicationName (or somesuch), which gets the
> base part from a BrowserDistribution* and then adds the suffix if needed.

Done.

[grt (UTC plus 2), 2012-06-08 19:54:21]

lg

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/setup/set...
File chrome/installer/setup/setup_main.cc (right):

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/setup/set...
chrome/installer/setup/setup_main.cc:162: type)->GetBaseAppName();
At some point I had started using the shortcut name for these log messages
rather than the app name since the latter is NOTREACHED() in some
implementations.  Would you mind changing these all to GetAppShortcutName()?

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/util/brow...
File chrome/installer/util/browser_distribution.h (right):

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/util/brow...
chrome/installer/util/browser_distribution.h:70: // Returns the unsuffixed
application name of this program.
I think the mention of "Default Programs" was useful for the reader (i.e., what
the application name is used for).

[gab, 2012-06-11 16:04:40]

Done.

Cheers,
Gab

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/setup/set...
File chrome/installer/setup/setup_main.cc (right):

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/setup/set...
chrome/installer/setup/setup_main.cc:162: type)->GetBaseAppName();
On 2012/06/08 19:54:21, grt wrote:
> At some point I had started using the shortcut name for these log messages
> rather than the app name since the latter is NOTREACHED() in some
> implementations.  Would you mind changing these all to GetAppShortcutName()?

Done.

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/util/brow...
File chrome/installer/util/browser_distribution.h (right):

http://codereview.chromium.org/10446111/diff/20004/chrome/installer/util/brow...
chrome/installer/util/browser_distribution.h:70: // Returns the unsuffixed
application name of this program.
On 2012/06/08 19:54:21, grt wrote:
> I think the mention of "Default Programs" was useful for the reader (i.e.,
what
> the application name is used for).

Done.

[grt (UTC plus 2), 2012-06-11 17:51:50]

lgtm w/ a nit

http://codereview.chromium.org/10446111/diff/18002/chrome/installer/util/shel...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10446111/diff/18002/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1115: RegistryEntry::GetUserEntries(dist,
nit: compress args for less vertical space.

[gab, 2012-06-11 19:33:27]

Thanks, done.

Cheers,
Gab

http://codereview.chromium.org/10446111/diff/18002/chrome/installer/util/shel...
File chrome/installer/util/shell_util.cc (right):

http://codereview.chromium.org/10446111/diff/18002/chrome/installer/util/shel...
chrome/installer/util/shell_util.cc:1115: RegistryEntry::GetUserEntries(dist,
On 2012/06/11 17:51:50, grt wrote:
> nit: compress args for less vertical space.

Done.

[grt (UTC plus 2), 2012-06-13 17:42:11]

lgtm++

[commit-bot: I haz the power, 2012-06-17 13:55:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/10446111/26001

[commit-bot: I haz the power, 2012-06-17 15:23:39]

Change committed as 142641