chrome://plugins Mojo-ification part 1/2, adding interface.

chrome://plugins Mojo-ification part 1/2, adding interface.

BUG=570143
Committed: https://crrev.com/113eb1c41926c7d6dd24fc12c6d06eeb949eb03e
Cr-Commit-Position: refs/heads/master@{#371438}

[dpapad, 2015-12-18 23:03:35]

Patchset #2 (id:20001) has been deleted

[dpapad, 2016-01-06 19:22:52]

Patchset #4 (id:80001) has been deleted

[dpapad, 2016-01-06 19:24:52]

Patchset #4 (id:100001) has been deleted

[dpapad, 2016-01-14 22:36:34]

Description was changed from

==========
chrome://plugins Mojo interface, WIP.

BUG=570143
==========

to

==========
chrome://plugins Mojo part 1/3, adding interface.

BUG=570143
==========

[dpapad, 2016-01-14 22:37:30]

Description was changed from

==========
chrome://plugins Mojo part 1/3, adding interface.

BUG=570143
==========

to

==========
chrome://plugins Mojo-ification part 1/3, adding interface.

BUG=570143
==========

[dpapad, 2016-01-21 02:02:59]

Description was changed from

==========
chrome://plugins Mojo-ification part 1/3, adding interface.

BUG=570143
==========

to

==========
chrome://plugins Mojo-ification part 1/2, adding interface.

BUG=570143
==========

[dpapad, 2016-01-21 20:21:08]

dpapad@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2016-01-22 00:58:34]

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/plugins/BUILD.gn (right):

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/BUILD.gn:1: # Copyright 2015 The Chromium
Authors. All rights reserved.
2016

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/plugins/plugins.mojom (right):

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/plugins.mojom:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
2016

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/plugins.mojom:34: GetPluginsData() =>
(array<PluginData> plugins);
Do you need both GetPluginsData() and OnPluginsUpdated()? I'm wondering if you
make SetClientPage call OnPluginsUpdated() then you shouldn't need the
GetPluginsData?

[dpapad, 2016-01-22 02:00:06]

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/plugins/BUILD.gn (right):

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/BUILD.gn:1: # Copyright 2015 The Chromium
Authors. All rights reserved.
On 2016/01/22 at 00:58:34, sky wrote:
> 2016

Done.

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/plugins/plugins.mojom (right):

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/plugins.mojom:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2016/01/22 at 00:58:34, sky wrote:
> 2016

Done.

https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
chrome/browser/ui/webui/plugins/plugins.mojom:34: GetPluginsData() =>
(array<PluginData> plugins);
On 2016/01/22 at 00:58:34, sky wrote:
> Do you need both GetPluginsData() and OnPluginsUpdated()? I'm wondering if you
make SetClientPage call OnPluginsUpdated() then you shouldn't need the
GetPluginsData?

The JS code is calling GetPluginsData() not only on startup but also within a
recurring 30sec timer, see follow up CL
https://codereview.chromium.org/1576183003 (previous code also did this), so I
don't think I can remove GetPluginsData(). BTW, I am re-using as much code as
possible both in C++ and JS (see follow up CL). 

But still, you raise a good point about this API design dilemma here that will
probably come up in other Mojo interfaces.

My thinking is as follows: One of the benefits of using Mojo is the explicit
association of a request (JS->C++) and a response (C++->JS) using Promises. The
suggested change would make the API resemble more the chrome.send() paradigm,
where a request is sent, and then a method is called as a callback, but the
association between request/response is hidden (documented in comments usually).
Making it explicit that one codepath deals with a direct response and one deals
with plugin info updated from a different chrome://plugins tab seems more
intuitive to me. WDYT?

[dpapad, 2016-01-25 23:55:07]

On 2016/01/22 at 02:00:06, dpapad wrote:
>
https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
> File chrome/browser/ui/webui/plugins/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
> chrome/browser/ui/webui/plugins/BUILD.gn:1: # Copyright 2015 The Chromium
Authors. All rights reserved.
> On 2016/01/22 at 00:58:34, sky wrote:
> > 2016
> 
> Done.
> 
>
https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
> File chrome/browser/ui/webui/plugins/plugins.mojom (right):
> 
>
https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
> chrome/browser/ui/webui/plugins/plugins.mojom:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
> On 2016/01/22 at 00:58:34, sky wrote:
> > 2016
> 
> Done.
> 
>
https://codereview.chromium.org/1524423002/diff/200001/chrome/browser/ui/webu...
> chrome/browser/ui/webui/plugins/plugins.mojom:34: GetPluginsData() =>
(array<PluginData> plugins);
> On 2016/01/22 at 00:58:34, sky wrote:
> > Do you need both GetPluginsData() and OnPluginsUpdated()? I'm wondering if
you make SetClientPage call OnPluginsUpdated() then you shouldn't need the
GetPluginsData?
> 
> The JS code is calling GetPluginsData() not only on startup but also within a
recurring 30sec timer, see follow up CL
https://codereview.chromium.org/1576183003 (previous code also did this), so I
don't think I can remove GetPluginsData(). BTW, I am re-using as much code as
possible both in C++ and JS (see follow up CL). 
> 
> But still, you raise a good point about this API design dilemma here that will
probably come up in other Mojo interfaces.
> 
> My thinking is as follows: One of the benefits of using Mojo is the explicit
association of a request (JS->C++) and a response (C++->JS) using Promises. The
suggested change would make the API resemble more the chrome.send() paradigm,
where a request is sent, and then a method is called as a callback, but the
association between request/response is hidden (documented in comments usually).
Making it explicit that one codepath deals with a direct response and one deals
with plugin info updated from a different chrome://plugins tab seems more
intuitive to me. WDYT?

Friendly ping.

[sky, 2016-01-25 23:57:40]

Sorry, some how forgot this. LGTM

[dpapad, 2016-01-26 00:45:51]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-01-26 00:46:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1524423002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1524423002/220001

[commit-bot: I haz the power, 2016-01-26 01:38:17]

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

[commit-bot: I haz the power, 2016-01-26 01:38:17]

Failed to commit the patch.

[commit-bot: I haz the power, 2016-01-26 01:41:05]

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

[dpapad, 2016-01-26 03:25:30]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-01-26 03:26:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1524423002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1524423002/220001

[commit-bot: I haz the power, 2016-01-26 03:37:10]

Committed patchset #9 (id:220001)

[commit-bot: I haz the power, 2016-01-26 03:39:05]

Description was changed from

==========
chrome://plugins Mojo-ification part 1/2, adding interface.

BUG=570143
==========

to

==========
chrome://plugins Mojo-ification part 1/2, adding interface.

BUG=570143

Committed: https://crrev.com/113eb1c41926c7d6dd24fc12c6d06eeb949eb03e
Cr-Commit-Position: refs/heads/master@{#371438}
==========

[commit-bot: I haz the power, 2016-01-26 03:39:06]

Patchset 9 (id:??) landed as
https://crrev.com/113eb1c41926c7d6dd24fc12c6d06eeb949eb03e
Cr-Commit-Position: refs/heads/master@{#371438}