Show "Paired" for already paired WebUSB devices on the chooser

Show "Paired" for already paired WebUSB devices on the chooser

Since the chooser can show "Paired" text for the already paired devices,
WebUSB can use this feature now. This CL added code to do that.

BUG=649502
Committed: https://crrev.com/001f50f635f2e97346c8d3303e68bd2c3cec776e
Cr-Commit-Position: refs/heads/master@{#421753}

[juncai, 2016-09-22 21:58:58]

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

[commit-bot: I haz the power, 2016-09-22 21:59:43]

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

[juncai, 2016-09-22 22:04:33]

juncai@chromium.org changed reviewers:
+ reillyg@chromium.org

[juncai, 2016-09-22 22:04:33]

Please take a look.

[commit-bot: I haz the power, 2016-09-22 22:46:36]

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

[commit-bot: I haz the power, 2016-09-22 22:46:37]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-09-23 02:13:49]

Instead of creating a WebUSBPermissionProvider here, which wasn't really meant
for this purpose I would just copy this part of the logic from
HasDevicePermission:

WebContents* web_contents =
    WebContents::FromRenderFrameHost(render_frame_host_);
GURL embedding_origin =
    web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin();
GURL requesting_origin =
    render_frame_host_->GetLastCommittedURL().GetOrigin();
Profile* profile =
    Profile::FromBrowserContext(web_contents->GetBrowserContext());
UsbChooserContext* chooser_context =
    UsbChooserContextFactory::GetForProfile(profile);

return chooser_context->HasDevicePermission(requesting_origin, embedding_origin,
                                            device);

[juncai, 2016-09-23 18:53:25]

On 2016/09/23 02:13:49, Reilly Grant wrote:
> Instead of creating a WebUSBPermissionProvider here, which wasn't really meant
> for this purpose I would just copy this part of the logic from
> HasDevicePermission:
> 
> WebContents* web_contents =
>     WebContents::FromRenderFrameHost(render_frame_host_);
> GURL embedding_origin =
>     web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin();
> GURL requesting_origin =
>     render_frame_host_->GetLastCommittedURL().GetOrigin();
> Profile* profile =
>     Profile::FromBrowserContext(web_contents->GetBrowserContext());
> UsbChooserContext* chooser_context =
>     UsbChooserContextFactory::GetForProfile(profile);
> 
> return chooser_context->HasDevicePermission(requesting_origin,
embedding_origin,
>                                             device);

Since similar logic needs to be implemented on Android after the chooser on
Android can show "Paired" text, I am wondering if it makes sense to have a
static member function at WebUSBPermissionProvider to check if a device is
paired or not, and it will reduce code duplication. What do you think?

[Reilly Grant (use Gerrit), 2016-09-27 09:01:17]

On 2016/09/23 at 18:53:25, juncai wrote:
> On 2016/09/23 02:13:49, Reilly Grant wrote:
> > Instead of creating a WebUSBPermissionProvider here, which wasn't really
meant
> > for this purpose I would just copy this part of the logic from
> > HasDevicePermission:
> > 
> > WebContents* web_contents =
> >     WebContents::FromRenderFrameHost(render_frame_host_);
> > GURL embedding_origin =
> >     web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin();
> > GURL requesting_origin =
> >     render_frame_host_->GetLastCommittedURL().GetOrigin();
> > Profile* profile =
> >     Profile::FromBrowserContext(web_contents->GetBrowserContext());
> > UsbChooserContext* chooser_context =
> >     UsbChooserContextFactory::GetForProfile(profile);
> > 
> > return chooser_context->HasDevicePermission(requesting_origin,
embedding_origin,
> >                                             device);
> 
> Since similar logic needs to be implemented on Android after the chooser on
Android can show "Paired" text, I am wondering if it makes sense to have a
static member function at WebUSBPermissionProvider to check if a device is
paired or not, and it will reduce code duplication. What do you think?

That seems reasonable but shouldn't Android be using UsbChooserController as
well?

[juncai, 2016-09-27 17:42:45]

> That seems reasonable but shouldn't Android be using UsbChooserController as
> well?

Android doesn't use UsbChooserController, which is for desktops. Android uses
UsbChooserDialogAndroid.

[Reilly Grant (use Gerrit), 2016-09-28 02:09:54]

On 2016/09/27 at 17:42:45, juncai wrote:
> > That seems reasonable but shouldn't Android be using UsbChooserController as
> > well?
> 
> Android doesn't use UsbChooserController, which is for desktops. Android uses
UsbChooserDialogAndroid.

We should work on reducing that duplication.

[juncai, 2016-09-28 21:06:09]

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

[commit-bot: I haz the power, 2016-09-28 21:06:25]

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

[juncai, 2016-09-28 21:15:28]

On 2016/09/28 02:09:54, Reilly Grant wrote:
> We should work on reducing that duplication.

I took a look at the UsbChooserController and UsbChooserDialogAndroid, it seems
that there is little duplication. Maybe there is something I missed?
UsbChooserDialogAndroid mostly has Android specific code, which make sense to
have a separate implementation.

I moved the implementation of WebUSBPermissionProvider::HasDevicePermission to a
static function in this patch. Please take a look. Thanks.

[commit-bot: I haz the power, 2016-09-28 21:46:44]

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

[commit-bot: I haz the power, 2016-09-28 21:46:45]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-09-29 01:27:55]

lgtm

[juncai, 2016-09-29 05:08:58]

The CQ bit was checked by juncai@chromium.org

[commit-bot: I haz the power, 2016-09-29 05:09:17]

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-09-29 05:15:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-29 05:17:59]

Description was changed from

==========
Show "Paired" for already paired WebUSB devices on the chooser

Since the chooser can show "Paired" text for the already paired devices,
WebUSB can use this feature now. This CL added code to do that.

BUG=649502
==========

to

==========
Show "Paired" for already paired WebUSB devices on the chooser

Since the chooser can show "Paired" text for the already paired devices,
WebUSB can use this feature now. This CL added code to do that.

BUG=649502

Committed: https://crrev.com/001f50f635f2e97346c8d3303e68bd2c3cec776e
Cr-Commit-Position: refs/heads/master@{#421753}
==========

[commit-bot: I haz the power, 2016-09-29 05:18:00]

Patchset 2 (id:??) landed as
https://crrev.com/001f50f635f2e97346c8d3303e68bd2c3cec776e
Cr-Commit-Position: refs/heads/master@{#421753}