Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569
Review-Url: https://codereview.chromium.org/2912323002
Cr-Commit-Position: refs/heads/master@{#476555}
Committed: https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6ccaeadaa70ec

[junwei, 2017-05-31 08:29:26]

junwei.fu@intel.com changed reviewers:
+ jam@chromium.org, yzshen@chromium.org

[junwei, 2017-05-31 08:29:27]

PTAL, thanks.

[yzshen1, 2017-05-31 17:46:40]

Please consider switching the tests to use the new mode. :)

LGTM

[junwei, 2017-06-01 00:24:35]

The CQ bit was checked by junwei.fu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 00:25:56]

Dry run: 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, 2017-06-01 02:24:07]

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

[commit-bot: I haz the power, 2017-06-01 02:24:08]

Dry run: This issue passed the CQ dry run.

[junwei, 2017-06-01 02:36:07]

On 2017/05/31 17:46:40, yzshen1 wrote:
> Please consider switching the tests to use the new mode. :)
> 
> LGTM

It's my pleasure to do it, thanks.

[junwei, 2017-06-01 02:36:25]

The CQ bit was checked by junwei.fu@intel.com

[commit-bot: I haz the power, 2017-06-01 02:36:51]

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, 2017-06-01 02:45:36]

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

[commit-bot: I haz the power, 2017-06-01 02:45:37]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[junwei, 2017-06-01 02:48:37]

The CQ bit was checked by junwei.fu@intel.com

[commit-bot: I haz the power, 2017-06-01 02:48:55]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[junwei, 2017-06-01 02:57:44]

junwei.fu@intel.com changed reviewers:
+ robertphillips@google.com
- jam@chromium.org

[commit-bot: I haz the power, 2017-06-01 02:58:20]

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

[commit-bot: I haz the power, 2017-06-01 02:58:21]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[robertphillips, 2017-06-01 11:55:47]

Description was changed from

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569
==========

to

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569
==========

[robertphillips, 2017-06-01 11:55:47]

robertphillips@google.com changed reviewers:
+ mtklein@google.com

[junwei, 2017-06-02 00:29:04]

On 2017/06/01 02:58:21, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

mtklein@ robertphillips@ PTAL, thanks.

[mtklein_C, 2017-06-02 00:33:04]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2017-06-02 00:33:05]

¯\_(ツ)_/¯

lgtm

[junwei, 2017-06-02 00:47:25]

The CQ bit was checked by junwei.fu@intel.com

[commit-bot: I haz the power, 2017-06-02 00:48: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, 2017-06-02 03:51:51]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496364445058920, "parent_rev":
"54d7eb0b3a09f39e9db65452d72209bee09fd006", "commit_rev":
"9821a80606c582307bc791c798c6ccaeadaa70ec"}

[commit-bot: I haz the power, 2017-06-02 03:52:00]

Description was changed from

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569
==========

to

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569

Review-Url: https://codereview.chromium.org/2912323002
Cr-Commit-Position: refs/heads/master@{#476555}
Committed:
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...
==========

[commit-bot: I haz the power, 2017-06-02 03:52:02]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...

[yzshen1, 2017-06-02 15:09:08]

On 2017/06/02 03:52:02, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
>
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...

Probably no need to revert, but I just noticed that the description of this CL
is incorrect. Please make sure the first line accurately describes what the CL
does next time.

[junwei, 2017-06-05 00:43:21]

On 2017/06/02 15:09:08, yzshen1 wrote:
> On 2017/06/02 03:52:02, commit-bot: I haz the power wrote:
> > Committed patchset #1 (id:1) as
> >
>
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...
> 
> Probably no need to revert, but I just noticed that the description of this CL
> is incorrect. Please make sure the first line accurately describes what the CL
> does next time.

Thanks for reminding, i will pay attention to it.

[junwei, 2017-06-07 07:50:48]

Description was changed from

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569

Review-Url: https://codereview.chromium.org/2912323002
Cr-Commit-Position: refs/heads/master@{#476555}
Committed:
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...
==========

to

==========
Mojo JS bindings: flip the default value of use_new_js_bindings to true.

Test cases will time out when CL [1] using skia mojo bitmap.

[1] https://codereview.chromium.org/2875243002#

BUG=699569

Review-Url: https://codereview.chromium.org/2912323002
Cr-Commit-Position: refs/heads/master@{#476555}
Committed:
https://chromium.googlesource.com/chromium/src/+/9821a80606c582307bc791c798c6...
==========

[junwei, 2017-06-07 07:50:48]

junwei.fu@intel.com changed reviewers:
- mtklein@google.com, robertphillips@google.com

[junwei, 2017-06-07 08:13:07]

On 2017/05/31 17:46:40, yzshen1 wrote:
> Please consider switching the tests to use the new mode. :)
> 
> LGTM

I am trying to switch shape detection layout tests to use new js binding, but
textdetection.mojom [1] is depend on geometry.mojom which still use old js
binding, does it mean all of using geometry.mojom need to switch new js binding?
such as media/capture/mojo/BUILD.gn [2]

[1]
https://cs.chromium.org/chromium/src/services/shape_detection/public/interfac...
[2] https://cs.chromium.org/chromium/src/media/capture/mojo/BUILD.gn

[yzshen1, 2017-06-07 16:47:42]

On 2017/06/07 08:13:07, junwei wrote:
> On 2017/05/31 17:46:40, yzshen1 wrote:
> > Please consider switching the tests to use the new mode. :)
> > 
> > LGTM
> 
> I am trying to switch shape detection layout tests to use new js binding, but
> textdetection.mojom [1] is depend on geometry.mojom which still use old js
> binding, does it mean all of using geometry.mojom need to switch new js
binding?
> such as media/capture/mojo/BUILD.gn [2]

Thanks for looking into this!

If a mojom target depends on geometry.mojom, *and* this mojom target is used in
JavaScript, then yes we need to switch this mojom target.
If it is never used in JavaScript anywhere, then no.

Looking at media/capture/mojo/BUILD.gn:
The "capture_types" target is already using the new mode (Because it doesn't
have use_new_js_bindings = false.) That probably means it is never used in
JavaScript so we don't care the inconsistent between "capture_types" and
"geometry".

The "image_capture" target doesn't depend on "geometry", so it should be
irrelevant to the problem we are discussing.


> 
> [1]
>
https://cs.chromium.org/chromium/src/services/shape_detection/public/interfac...
> [2] https://cs.chromium.org/chromium/src/media/capture/mojo/BUILD.gn

[yzshen1, 2017-06-07 16:51:55]

On 2017/06/07 16:47:42, yzshen1 wrote:
> On 2017/06/07 08:13:07, junwei wrote:
> > On 2017/05/31 17:46:40, yzshen1 wrote:
> > > Please consider switching the tests to use the new mode. :)
> > > 
> > > LGTM
> > 
> > I am trying to switch shape detection layout tests to use new js binding,
but
> > textdetection.mojom [1] is depend on geometry.mojom which still use old js
> > binding, does it mean all of using geometry.mojom need to switch new js
> binding?
> > such as media/capture/mojo/BUILD.gn [2]
> 
> Thanks for looking into this!
> 
> If a mojom target depends on geometry.mojom, *and* this mojom target is used
in
> JavaScript, then yes we need to switch this mojom target.
> If it is never used in JavaScript anywhere, then no.
> 
> Looking at media/capture/mojo/BUILD.gn:
> The "capture_types" target is already using the new mode (Because it doesn't
> have use_new_js_bindings = false.) That probably means it is never used in
> JavaScript so we don't care the inconsistent between "capture_types" and
> "geometry".

I confirmed that "capture_types" is never used in JavaScript:
https://cs.chromium.org/search/?q=file:(js%7Chtml)$+video_capture_types.mojom...


> 
> The "image_capture" target doesn't depend on "geometry", so it should be
> irrelevant to the problem we are discussing.
> 
> 
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/services/shape_detection/public/interfac...
> > [2] https://cs.chromium.org/chromium/src/media/capture/mojo/BUILD.gn

[junwei, 2017-06-08 01:46:24]

On 2017/06/07 16:51:55, yzshen1 wrote:
> On 2017/06/07 16:47:42, yzshen1 wrote:
> > On 2017/06/07 08:13:07, junwei wrote:
> > > On 2017/05/31 17:46:40, yzshen1 wrote:
> > > > Please consider switching the tests to use the new mode. :)
> > > > 
> > > > LGTM
> > > 
> > > I am trying to switch shape detection layout tests to use new js binding,
> but
> > > textdetection.mojom [1] is depend on geometry.mojom which still use old js
> > > binding, does it mean all of using geometry.mojom need to switch new js
> > binding?
> > > such as media/capture/mojo/BUILD.gn [2]
> > 
> > Thanks for looking into this!
> > 
> > If a mojom target depends on geometry.mojom, *and* this mojom target is used
> in
> > JavaScript, then yes we need to switch this mojom target.
> > If it is never used in JavaScript anywhere, then no.
> > 
> > Looking at media/capture/mojo/BUILD.gn:
> > The "capture_types" target is already using the new mode (Because it doesn't
> > have use_new_js_bindings = false.) That probably means it is never used in
> > JavaScript so we don't care the inconsistent between "capture_types" and
> > "geometry".
> 
> I confirmed that "capture_types" is never used in JavaScript:
>
https://cs.chromium.org/search/?q=file:(js%7Chtml)$+video_capture_types.mojom...
> 
> 
> > 
> > The "image_capture" target doesn't depend on "geometry", so it should be
> > irrelevant to the problem we are discussing.
> > 
> > 
> > > 
> > > [1]
> > >
> >
>
https://cs.chromium.org/chromium/src/services/shape_detection/public/interfac...
> > > [2] https://cs.chromium.org/chromium/src/media/capture/mojo/BUILD.gn

I got it, thanks.

Loading mojo/public/js/bindings module switched to <script
src="file:///gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>.
How to use content/public/renderer/interfaces in new binding?

[yzshen1, 2017-06-08 17:35:15]

On 2017/06/08 01:46:24, junwei wrote:
> On 2017/06/07 16:51:55, yzshen1 wrote:
> > On 2017/06/07 16:47:42, yzshen1 wrote:
> > > On 2017/06/07 08:13:07, junwei wrote:
> > > > On 2017/05/31 17:46:40, yzshen1 wrote:
> > > > > Please consider switching the tests to use the new mode. :)
> > > > > 
> > > > > LGTM
> > > > 
> > > > I am trying to switch shape detection layout tests to use new js
binding,
> > but
> > > > textdetection.mojom [1] is depend on geometry.mojom which still use old
js
> > > > binding, does it mean all of using geometry.mojom need to switch new js
> > > binding?
> > > > such as media/capture/mojo/BUILD.gn [2]
> > > 
> > > Thanks for looking into this!
> > > 
> > > If a mojom target depends on geometry.mojom, *and* this mojom target is
used
> > in
> > > JavaScript, then yes we need to switch this mojom target.
> > > If it is never used in JavaScript anywhere, then no.
> > > 
> > > Looking at media/capture/mojo/BUILD.gn:
> > > The "capture_types" target is already using the new mode (Because it
doesn't
> > > have use_new_js_bindings = false.) That probably means it is never used in
> > > JavaScript so we don't care the inconsistent between "capture_types" and
> > > "geometry".
> > 
> > I confirmed that "capture_types" is never used in JavaScript:
> >
>
https://cs.chromium.org/search/?q=file:(js%7Chtml)$+video_capture_types.mojom...
> > 
> > 
> > > 
> > > The "image_capture" target doesn't depend on "geometry", so it should be
> > > irrelevant to the problem we are discussing.
> > > 
> > > 
> > > > 
> > > > [1]
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/services/shape_detection/public/interfac...
> > > > [2] https://cs.chromium.org/chromium/src/media/capture/mojo/BUILD.gn
> 
> I got it, thanks.
> 
> Loading mojo/public/js/bindings module switched to <script
> src="file:///gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>.
> How to use content/public/renderer/interfaces in new binding?

(I would suggest we discuss such issue on a separate mailing thread in the
future, since it is not directly related to this patch.)
Do you mean you need to use "addInterfaceOverrideForTesting"?

There is a new way doing this. Please see MojoInterfaceInterceptor in
//src/third_party/WebKit/LayoutTests/mojo/bind-interface.html