[Extensions UI] Share more code in ExtensionInstalledBubble

[Extensions UI] Share more code in ExtensionInstalledBubble

Move implemention for getting the how-to-use description to
the platform-agnostic class.

BUG=458301
TBR=sky@chromium.org (OWNERS for ui/extensions; Finnur reviewed)

Committed: https://crrev.com/f8a6bf68e9bafb0cf77cbf132aa69a745b737726
Cr-Commit-Position: refs/heads/master@{#317625}

[Devlin, 2015-02-18 01:06:13]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-02-18 01:06:18]

Patchset #1 (id:20001) has been deleted

[Devlin, 2015-02-18 01:16:06]

rdevlin.cronin@chromium.org changed reviewers:
+ benwells@chromium.org, finnur@chromium.org, rsesek@chromium.org

[Devlin, 2015-02-18 01:16:08]

Just a bit of cleanup during downtime. :)  Take a look when you can.

Robert - ui/cocoa
Finnur - ui/views
Ben - ui/extensions

[Finnur, 2015-02-18 09:40:41]

My part LGTM. Couple of nits.

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
File chrome/browser/ui/extensions/extension_installed_bubble.cc (right):

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:36: // Returns the
keybinding for an extension command, or an empty string if none
Empty string?

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:129: return
base::string16();
nit: We generally prefer:

if (message_id == 0)
  return base::string16();

return extra.empty() ? l10n_util::GetStringUTF16(message_id) :
    l10n_util::GetStringFUTF16(message_id, extra);

[Robert Sesek, 2015-02-18 15:03:00]

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
File chrome/browser/ui/extensions/extension_installed_bubble.cc (right):

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:148: // Parse the
extension command, if one exists.
Can the command change over time?

[Devlin, 2015-02-18 18:08:09]

New patchsets have been uploaded after l-g-t-m from finnur@chromium.org

[Devlin, 2015-02-18 18:08:41]

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
File chrome/browser/ui/extensions/extension_installed_bubble.cc (right):

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:36: // Returns the
keybinding for an extension command, or an empty string if none
On 2015/02/18 09:40:41, Finnur wrote:
> Empty string?

Whoops, forgot to update that comment. Done.

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:129: return
base::string16();
On 2015/02/18 09:40:41, Finnur wrote:
> nit: We generally prefer:
> 
> if (message_id == 0)
>   return base::string16();
> 
> return extra.empty() ? l10n_util::GetStringUTF16(message_id) :
>     l10n_util::GetStringFUTF16(message_id, extra);

Done.

https://codereview.chromium.org/922523005/diff/40001/chrome/browser/ui/extens...
chrome/browser/ui/extensions/extension_installed_bubble.cc:148: // Parse the
extension command, if one exists.
On 2015/02/18 15:03:00, Robert Sesek wrote:
> Can the command change over time?

Technically, yes.  A user could customize an extension command to be different
from the default (from the chrome:extensions page). That said, this is for the
extension installation bubble, so it's not really possible for it to change
before the bubble shows. ;)

[Robert Sesek, 2015-02-18 18:45:32]

cocoa/ LGTM

[benwells, 2015-02-18 20:03:38]

Am on vacation until next week. Feel free to find another reviewer.

[Devlin, 2015-02-18 20:09:24]

rdevlin.cronin@chromium.org changed reviewers:
+ sky@chromium.org
- benwells@chromium.org

[Devlin, 2015-02-18 20:10:55]

rdevlin.cronin@chromium.org changed reviewers:
- sky@chromium.org

[Devlin, 2015-02-18 20:11:57]

On 2015/02/18 20:03:38, benwells (OOO to 24 Feb) wrote:
> Am on vacation until next week. Feel free to find another reviewer.

-Ben

Finnur, do you feel comfortable reviewing ui/extensions (judging from your
comments, I think you already did ;)), and I'll get a rubberstamp from a
top-level ui OWNER?

[Finnur, 2015-02-20 09:53:47]

Yup. No problem. LGTM.
(Was travelling yesterday, didn't see your question until now)

[Devlin, 2015-02-23 17:33:49]

New patchsets have been uploaded after l-g-t-m from
rsesek@chromium.org,finnur@chromium.org

[Devlin, 2015-02-23 17:35:03]

rdevlin.cronin@chromium.org changed reviewers:
+ sky@chromium.org

[Devlin, 2015-02-23 19:11:53]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2015-02-23 19:13:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/922523005/80001

[commit-bot: I haz the power, 2015-02-23 19:43:28]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2015-02-23 19:44:00]

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