Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(147)

Issue 2779533002: Revert of Mojo JS bindings: change module loading solution. (Closed)

Created:
3 years, 9 months ago by hayato
Modified:
3 years, 9 months ago
CC:
Aaron Boodman, abarth-chromium, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, tfarina, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Mojo JS bindings: change module loading solution. (patchset #7 id:120001 of https://codereview.chromium.org/2759563004/ ) Reason for revert: Consistent failure: webkit_tests failing on 6 builders 33 since the first detection e.g. https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.10 This CL is suspicious because mojo/module-loading.html has been failing. https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.webkit%2FWebKit_Mac10.10%2F31815%2F%2B%2Frecipes%2Fsteps%2Fwebkit_tests%2F0%2Fstdout Regressions: Unexpected text-only failures (1) mojo/module-loading.html [ Failure ] 09:20:53.708 5958 worker/0 virtual/mojo-loading/http/tests/bluetooth/https/requestDevice/cross-origin-iframe.html output stderr lines: 09:20:53.709 5958 [09:20:53.354] vtDecompressionDuctCreate signalled err=-8973 (err) (Could not select and open decoder instance) at /SourceCache/CoreMedia_frameworks/CoreMedia-1562.235/Sources/VideoToolbox/VTDecompressionSession.c line 1181 09:20:53.709 5958 <<<< VTVideoEncoderSelection >>>> VTSelectAndCreateVideoEncoderInstanceInternal: no video encoder found for 'avc1' Original issue's description: > Mojo JS bindings: change module loading solution. > > This change takes place on the mojo/public/js/new_bindings copy so it doesn't > affect existing users. > > - This change gets rid of AMD module loading. Now the bindings API is defined in > the "mojoBindings" namespace. At build time, all bindings files are combined > into a single file "mojo_bindings.js". Users should use <script> tag to include > this file (as well as generated mojom.js files). > > - Generated mojom.js files export their definitions under the same namespace as > the "module" statement in the corresponding mojom files. > > - This change also adds a "use_new_js_bindings" option to the generator. It > duplicates the control message mojom files in order to generate two > different flavors of JS bindings. > > - The new bindings use the Mojo system API defined by Web IDL. > > BUG=699569 > > Review-Url: https://codereview.chromium.org/2759563004 > Cr-Commit-Position: refs/heads/master@{#459654} > Committed: https://chromium.googlesource.com/chromium/src/+/e6a5534bb3fe61b5224f1a22e43ba957190ad5d0 TBR=rockot@chromium.org,dcheng@chromium.org,alokp@chromium.org,yzshen@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=699569 Review-Url: https://codereview.chromium.org/2779533002 Cr-Commit-Position: refs/heads/master@{#459745} Committed: https://chromium.googlesource.com/chromium/src/+/06cde11f9f3843eb13e3d3d73296a07e9e8601f5

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+304 lines, -545 lines) Patch
M mojo/public/interfaces/bindings/BUILD.gn View 1 chunk +0 lines, -12 lines 0 comments Download
D mojo/public/interfaces/bindings/new_bindings/OWNERS View 1 chunk +0 lines, -2 lines 0 comments Download
D mojo/public/interfaces/bindings/new_bindings/interface_control_messages.mojom View 1 chunk +0 lines, -67 lines 0 comments Download
D mojo/public/interfaces/bindings/new_bindings/pipe_control_messages.mojom View 1 chunk +0 lines, -46 lines 0 comments Download
M mojo/public/interfaces/bindings/tests/BUILD.gn View 2 chunks +0 lines, -10 lines 0 comments Download
D mojo/public/interfaces/bindings/tests/echo.mojom View 1 chunk +0 lines, -12 lines 0 comments Download
D mojo/public/interfaces/bindings/tests/echo_import.mojom View 1 chunk +0 lines, -10 lines 0 comments Download
M mojo/public/js/BUILD.gn View 1 chunk +0 lines, -38 lines 0 comments Download
D mojo/public/js/new_bindings/base.js View 1 chunk +0 lines, -33 lines 0 comments Download
M mojo/public/js/new_bindings/bindings.js View 8 chunks +33 lines, -24 lines 0 comments Download
M mojo/public/js/new_bindings/buffer.js View 2 chunks +5 lines, -4 lines 0 comments Download
M mojo/public/js/new_bindings/codec.js View 10 chunks +56 lines, -52 lines 0 comments Download
M mojo/public/js/new_bindings/connector.js View 4 chunks +30 lines, -20 lines 0 comments Download
M mojo/public/js/new_bindings/interface_types.js View 3 chunks +14 lines, -8 lines 0 comments Download
M mojo/public/js/new_bindings/lib/control_message_handler.js View 2 chunks +38 lines, -33 lines 0 comments Download
M mojo/public/js/new_bindings/lib/control_message_proxy.js View 5 chunks +34 lines, -29 lines 0 comments Download
M mojo/public/js/new_bindings/router.js View 8 chunks +26 lines, -13 lines 0 comments Download
M mojo/public/js/new_bindings/unicode.js View 2 chunks +7 lines, -7 lines 0 comments Download
M mojo/public/js/new_bindings/validator.js View 21 chunks +45 lines, -44 lines 0 comments Download
M mojo/public/tools/bindings/generators/js_templates/module.amd.tmpl View 1 chunk +6 lines, -27 lines 0 comments Download
M mojo/public/tools/bindings/generators/js_templates/module_definition.tmpl View 3 chunks +8 lines, -7 lines 0 comments Download
M mojo/public/tools/bindings/generators/mojom_js_generator.py View 1 chunk +0 lines, -1 line 0 comments Download
M mojo/public/tools/bindings/mojom.gni View 2 chunks +0 lines, -12 lines 0 comments Download
M mojo/public/tools/bindings/mojom_bindings_generator.py View 2 chunks +0 lines, -5 lines 0 comments Download
M mojo/public/tools/bindings/pylib/mojom/generate/generator.py View 2 chunks +2 lines, -3 lines 0 comments Download
D third_party/WebKit/LayoutTests/mojo/module-loading.html View 1 chunk +0 lines, -26 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
hayato
Created Revert of Mojo JS bindings: change module loading solution.
3 years, 9 months ago (2017-03-27 04:38:23 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2779533002/1
3 years, 9 months ago (2017-03-27 04:38:42 UTC) #3
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/177730) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
3 years, 9 months ago (2017-03-27 04:41:11 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2779533002/1
3 years, 9 months ago (2017-03-27 08:52:11 UTC) #7
dcheng
LGTM
3 years, 9 months ago (2017-03-27 08:57:16 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/06cde11f9f3843eb13e3d3d73296a07e9e8601f5
3 years, 9 months ago (2017-03-27 10:56:02 UTC) #11
yzshen1
3 years, 9 months ago (2017-03-27 16:59:22 UTC) #12
Message was sent while issue was closed.
On 2017/03/27 10:56:02, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
>
https://chromium.googlesource.com/chromium/src/+/06cde11f9f3843eb13e3d3d73296...


Sorry for didn't notice the failure earlier.
It seems those broken bots are all Mac. But the CL passed the CQ which has got
Mac trybots.

The output doesn't show any useful information. I will try to build with the
same config as the failed bots and see whether I can repro.

The following are the failed bots:
====================================================
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.9
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1...
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1...
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1...
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1...
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac1...

Powered by Google App Engine
This is Rietveld 408576698