Display extension name on device chooser title

Display extension name on device chooser title

When WebBluetooth is used from chrome-extension page, the chooser title
displays Chrome extension ID. It would be better to display the extension
name if it exists. This CL fixes it.

BUG=626149
Committed: https://crrev.com/a29e116d910898c6f783a2d23cba17a20b5c503c
Cr-Commit-Position: refs/heads/master@{#404934}

[juncai, 2016-07-11 21:42:01]

juncai@chromium.org changed reviewers:
+ msw@chromium.org, reillyg@chromium.org, rsesek@chromium.org

[juncai, 2016-07-11 21:42:01]

rsesek@chromium.org: Please review changes in 
//chrome/browser/ui/cocoa/

msw@chromium.org: Please review changes in 
//chrome/browser/ui/views/

reillyg@chromium.org: Please review changes in 
//chrome/browser/chooser_controller/

[Robert Sesek, 2016-07-11 21:43:53]

cocoa/ LGTM

[Reilly Grant (use Gerrit), 2016-07-11 21:48:03]

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:46: if
(title_prefix.empty()) {
We can determine if this is an extension URL or not by checking if the scheme is
chrome-extension:// instead of always looking up the URL in the extension
registry. This keeps the two paths more separate.

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:51: // Add quote if the
it is a Chrome app/extension name.
Quotation marks need to be localized. Instead of re-using the string create one
string for an origin and one for an app name.

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right):

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view.cc:20: :
chooser_content_view_(nullptr) {
This doesn't need to be initialized here because it is unconditionally
initialized below.

[juncai, 2016-07-11 23:16:45]

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:46: if
(title_prefix.empty()) {
On 2016/07/11 21:48:03, Reilly Grant wrote:
> We can determine if this is an extension URL or not by checking if the scheme
is
> chrome-extension:// instead of always looking up the URL in the extension
> registry. This keeps the two paths more separate.

Done.

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:51: // Add quote if the
it is a Chrome app/extension name.
On 2016/07/11 21:48:02, Reilly Grant wrote:
> Quotation marks need to be localized. Instead of re-using the string create
one
> string for an origin and one for an app name.

Done.

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/chooser_dialog_view.cc (right):

https://codereview.chromium.org/2122073004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/chooser_dialog_view.cc:20: :
chooser_content_view_(nullptr) {
On 2016/07/11 21:48:03, Reilly Grant wrote:
> This doesn't need to be initialized here because it is unconditionally
> initialized below.

Done.

[Reilly Grant (use Gerrit), 2016-07-11 23:31:40]

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14869: +      <message
name="IDS_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME" desc="The label that is used to
introduce chooser details to the user in a popup.">
nit: These message descriptions should explain why there are two different
strings.

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14870: +        <ph
name="CHROME_EXTENSION_NAME">"$1"<ex>Chrome extension name</ex></ph> wants to
connect to:
This might work but most other examples put the quotation marks around the <ph>
tag instead of inside like this:

"<ph name="CHROME_EXTENSION_NAME">$1<ex>Chrome extension name</ex></ph>"

https://codereview.chromium.org/2122073004/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:55:
title_prefix_has_quote_ = false;
This field is suspicious. Please just move the body of this function into
GetTitle(). It doesn't need to be separated this way.

[juncai, 2016-07-12 00:00:42]

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14869: +      <message
name="IDS_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME" desc="The label that is used to
introduce chooser details to the user in a popup.">
On 2016/07/11 23:31:40, Reilly Grant wrote:
> nit: These message descriptions should explain why there are two different
> strings.

Done.

https://codereview.chromium.org/2122073004/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:14870: +        <ph
name="CHROME_EXTENSION_NAME">"$1"<ex>Chrome extension name</ex></ph> wants to
connect to:
On 2016/07/11 23:31:40, Reilly Grant wrote:
> This might work but most other examples put the quotation marks around the
<ph>
> tag instead of inside like this:
> 
> "<ph name="CHROME_EXTENSION_NAME">$1<ex>Chrome extension name</ex></ph>"

Done.

https://codereview.chromium.org/2122073004/diff/40001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/40001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:55:
title_prefix_has_quote_ = false;
On 2016/07/11 23:31:40, Reilly Grant wrote:
> This field is suspicious. Please just move the body of this function into
> GetTitle(). It doesn't need to be separated this way.

Done.

[Reilly Grant (use Gerrit), 2016-07-12 00:12:05]

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:39:
GURL(origin.Serialize()));
You can use extension_registry->enabled_extensions().GetByID(origin.host()).

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:41: title_prefix =
base::UTF8ToUTF16(extension->name());
No need to set title_prefix here, just return
l10n_util::GetStringFUTF16(IDS_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME,
base::UTF8ToUTF16(extension->name()).

[juncai, 2016-07-12 01:14:44]

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
File chrome/browser/chooser_controller/chooser_controller.cc (right):

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:39:
GURL(origin.Serialize()));
On 2016/07/12 00:12:05, Reilly Grant wrote:
> You can use extension_registry->enabled_extensions().GetByID(origin.host()).

Done.

https://codereview.chromium.org/2122073004/diff/80001/chrome/browser/chooser_...
chrome/browser/chooser_controller/chooser_controller.cc:41: title_prefix =
base::UTF8ToUTF16(extension->name());
On 2016/07/12 00:12:05, Reilly Grant wrote:
> No need to set title_prefix here, just return
> l10n_util::GetStringFUTF16(IDS_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME,
> base::UTF8ToUTF16(extension->name()).

Done.

[Reilly Grant (use Gerrit), 2016-07-12 15:02:19]

lgtm

[msw, 2016-07-12 18:38:02]

nice; lgtm

[juncai, 2016-07-12 19:10:29]

The CQ bit was checked by juncai@chromium.org

[juncai, 2016-07-12 19:10:30]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/2122073004/#ps140001
(title: "removed unused variable")

[commit-bot: I haz the power, 2016-07-12 19:12:18]

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, 2016-07-13 02:53:48]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-07-13 02:54:40]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 02:55:36]

Description was changed from

==========
Display extension name on device chooser title

When WebBluetooth is used from chrome-extension page, the chooser title
displays Chrome extension ID. It would be better to display the extension
name if it exists. This CL fixes it.

BUG=626149
==========

to

==========
Display extension name on device chooser title

When WebBluetooth is used from chrome-extension page, the chooser title
displays Chrome extension ID. It would be better to display the extension
name if it exists. This CL fixes it.

BUG=626149

Committed: https://crrev.com/a29e116d910898c6f783a2d23cba17a20b5c503c
Cr-Commit-Position: refs/heads/master@{#404934}
==========

[commit-bot: I haz the power, 2016-07-13 02:55:37]

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