Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reland "Refactor JSNumber.toDart and Object.toJS" #43363

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Reland "Refactor JSNumber.toDart and Object.toJS" #43363

merged 1 commit into from
Jul 12, 2023

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Jun 30, 2023 •
edited
Loading

This reverts commit dce75ab.

This also makes some small changes to make onBenchmark a JSExportedDartFunction instead of a JSBoxedDartObject. This is for changes in flutter/flutter#129436 and to account for the fact that flutter/packages provides an allowInterop'd function. Benchmarks tests pass with this CL.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jun 30, 2023
@srujzs srujzs requested a review from eyebrowsoffire July 5, 2023 16:01
@srujzs srujzs force-pushed the todartchanges branch 3 times, most recently from dca5367 to 99d62b0 Compare July 11, 2023 19:11
@srujzs
Reland "Refactor JSNumber.toDart and Object.toJS"
27112ad 
This reverts commit dce75ab.

This also makes some small changes to make onBenchmark a
JSExportedDartFunction instead of a JSBoxedDartObject and
updates new invocations of JSNumber.toDart.
@srujzs
Copy link
Contributor Author

srujzs commented Jul 11, 2023

With the changes in dart2wasm to throw an exception instead of a Wasm RuntimeError, profiler_test passes now. PTAL @eyebrowsoffire, thanks!

@srujzs srujzs mentioned this pull request Jul 11, 2023
Merged
8 tasks
@srujzs srujzs merged commit feaadf8 into flutter:main Jul 12, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 12, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 12, 2023
@engine-flutter-autoroll
feaadf830 Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/…
a7c78f4 
…engine#43363)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 13, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 16e2ab7e986c to 1b1ccdd1f527 (13 revisions) (#…
47ba59c 
…130458)

flutter/engine@16e2ab7...1b1ccdd

2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 4e989b1564ee to bedc92598644 (1 revision) (flutter/engine#43617)
2023-07-12 flar@google.com move rtree and canvas_spy sources to Fuchsia sub-directory (flutter/engine#43615)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 68e3c0b3eea7 to 4e989b1564ee (6 revisions) (flutter/engine#43614)
2023-07-12 ian@hixie.ch Document (and assert) that channel names can't contains nulls (flutter/engine#43593)
2023-07-12 58529443+srujzs@users.noreply.github.com Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/engine#43363)
2023-07-12 31859944+LongCatIsLooong@users.noreply.github.com Add a flag to `ParagraphBuilder` for rounding hack migration (flutter/engine#43118)
2023-07-12 john@johnmccutchan.com [Impeller] Fixes for asymmetric stencil descriptors (flutter/engine#43535)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from adeeb8d50f7c to 68e3c0b3eea7 (1 revision) (flutter/engine#43609)
2023-07-12 flar@google.com Add comment to use of 3x3 mapRect in TransformLayer (flutter/engine#43608)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from a251a36ea519 to adeeb8d50f7c (1 revision) (flutter/engine#43606)
2023-07-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b95f6531c726 to 8f8f281ccdc6 (2 revisions) (flutter/engine#43607)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from ebc149cff431 to a251a36ea519 (2 revisions) (flutter/engine#43604)
2023-07-12 jonahwilliams@google.com [Impeller] Use new SkParagraph APIs for stroked text. (flutter/engine#41735)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 13, 2023
@engine-flutter-autoroll
feaadf830 Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/…
96d7747 
…engine#43363)
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Jul 13, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 16e2ab7e986c to b4080361f2f9 (28 revisions) (#…
ee4208f 
…130496)

Roll Flutter Engine from 16e2ab7e986c to b4080361f2f9 (28 revisions)

flutter/engine@16e2ab7...b408036

2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from e5ec341bc3ca to 56b68ce6196c (1 revision) (flutter/engine#43633)
2023-07-13 skia-flutter-autoroll@skia.org Roll Dart SDK from f499e91e8cb2 to ade4dae923f3 (1 revision) (flutter/engine#43632)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 811b046c673b to e5ec341bc3ca (1 revision) (flutter/engine#43631)
2023-07-13 ian@hixie.ch Make GOMA state automatic by default (flutter/engine#43584)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from c8da0c657c4e to 811b046c673b (3 revisions) (flutter/engine#43630)
2023-07-13 chinmaygarde@google.com [Impeller] Remove unactionable error logs and use structure chains for instance creation. (flutter/engine#43629)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 7f391ea9164e to c8da0c657c4e (1 revision) (flutter/engine#43628)
2023-07-13 chinmaygarde@google.com [Impeller] Add RAII wrappers for VMA objects. (flutter/engine#43626)
2023-07-13 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 1STsUj0X5YgpiSNEb... to xBJq6PsO5ebblODMe... (flutter/engine#43627)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 6ed93436d57c to 7f391ea9164e (1 revision) (flutter/engine#43625)
2023-07-13 skia-flutter-autoroll@skia.org Roll Dart SDK from 8f8f281ccdc6 to f499e91e8cb2 (3 revisions) (flutter/engine#43623)
2023-07-13 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 0fvk838jTDNQ_l43k... to 3C7P0w8ySmtqpyi3S... (flutter/engine#43622)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from bedc92598644 to 6ed93436d57c (3 revisions) (flutter/engine#43621)
2023-07-13 jonahwilliams@google.com [Impeller] Add support to embedder for Impeller on GL (via Angle on Windows). (flutter/engine#43388)
2023-07-12 jonahwilliams@google.com [Impeller] Allocate buffers out of a pool on the raster thread. (flutter/engine#43564)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 4e989b1564ee to bedc92598644 (1 revision) (flutter/engine#43617)
2023-07-12 flar@google.com move rtree and canvas_spy sources to Fuchsia sub-directory (flutter/engine#43615)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 68e3c0b3eea7 to 4e989b1564ee (6 revisions) (flutter/engine#43614)
2023-07-12 ian@hixie.ch Document (and assert) that channel names can't contains nulls (flutter/engine#43593)
2023-07-12 58529443+srujzs@users.noreply.github.com Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/engine#43363)
2023-07-12 31859944+LongCatIsLooong@users.noreply.github.com Add a flag to `ParagraphBuilder` for rounding hack migration (flutter/engine#43118)
2023-07-12 john@johnmccutchan.com [Impeller] Fixes for asymmetric stencil descriptors (flutter/engine#43535)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from adeeb8d50f7c to 68e3c0b3eea7 (1 revision) (flutter/engine#43609)
2023-07-12 flar@google.com Add comment to use of 3x3 mapRect in TransformLayer (flutter/engine#43608)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from a251a36ea519 to adeeb8d50f7c (1 revision) (flutter/engine#43606)
2023-07-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b95f6531c726 to 8f8f281ccdc6 (2 revisions) (flutter/engine#43607)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from ebc149cff431 to a251a36ea519 (2 revisions) (flutter/engine#43604)
2023-07-12 jonahwilliams@google.com [Impeller] Use new SkParagraph APIs for stroked text. (flutter/engine#41735)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 1STsUj0X5Ygp to xBJq6PsO5ebb
  fuchsia/sdk/core/mac-amd64 from 0fvk838jTDNQ to 3C7P0w8ySmtq

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
...
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 13, 2023
@engine-flutter-autoroll @LouiseHsu
Roll Flutter Engine from 16e2ab7e986c to 1b1ccdd1f527 (13 revisions) (f…
b575927 
…lutter#130458)

flutter/engine@16e2ab7...1b1ccdd

2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 4e989b1564ee to bedc92598644 (1 revision) (flutter/engine#43617)
2023-07-12 flar@google.com move rtree and canvas_spy sources to Fuchsia sub-directory (flutter/engine#43615)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 68e3c0b3eea7 to 4e989b1564ee (6 revisions) (flutter/engine#43614)
2023-07-12 ian@hixie.ch Document (and assert) that channel names can't contains nulls (flutter/engine#43593)
2023-07-12 58529443+srujzs@users.noreply.github.com Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/engine#43363)
2023-07-12 31859944+LongCatIsLooong@users.noreply.github.com Add a flag to `ParagraphBuilder` for rounding hack migration (flutter/engine#43118)
2023-07-12 john@johnmccutchan.com [Impeller] Fixes for asymmetric stencil descriptors (flutter/engine#43535)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from adeeb8d50f7c to 68e3c0b3eea7 (1 revision) (flutter/engine#43609)
2023-07-12 flar@google.com Add comment to use of 3x3 mapRect in TransformLayer (flutter/engine#43608)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from a251a36ea519 to adeeb8d50f7c (1 revision) (flutter/engine#43606)
2023-07-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b95f6531c726 to 8f8f281ccdc6 (2 revisions) (flutter/engine#43607)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from ebc149cff431 to a251a36ea519 (2 revisions) (flutter/engine#43604)
2023-07-12 jonahwilliams@google.com [Impeller] Use new SkParagraph APIs for stroked text. (flutter/engine#41735)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 13, 2023
@engine-flutter-autoroll @LouiseHsu
Roll Flutter Engine from 16e2ab7e986c to b4080361f2f9 (28 revisions) (f…
d6f8d61 
…lutter#130496)

Roll Flutter Engine from 16e2ab7e986c to b4080361f2f9 (28 revisions)

flutter/engine@16e2ab7...b408036

2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from e5ec341bc3ca to 56b68ce6196c (1 revision) (flutter/engine#43633)
2023-07-13 skia-flutter-autoroll@skia.org Roll Dart SDK from f499e91e8cb2 to ade4dae923f3 (1 revision) (flutter/engine#43632)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 811b046c673b to e5ec341bc3ca (1 revision) (flutter/engine#43631)
2023-07-13 ian@hixie.ch Make GOMA state automatic by default (flutter/engine#43584)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from c8da0c657c4e to 811b046c673b (3 revisions) (flutter/engine#43630)
2023-07-13 chinmaygarde@google.com [Impeller] Remove unactionable error logs and use structure chains for instance creation. (flutter/engine#43629)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 7f391ea9164e to c8da0c657c4e (1 revision) (flutter/engine#43628)
2023-07-13 chinmaygarde@google.com [Impeller] Add RAII wrappers for VMA objects. (flutter/engine#43626)
2023-07-13 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 1STsUj0X5YgpiSNEb... to xBJq6PsO5ebblODMe... (flutter/engine#43627)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from 6ed93436d57c to 7f391ea9164e (1 revision) (flutter/engine#43625)
2023-07-13 skia-flutter-autoroll@skia.org Roll Dart SDK from 8f8f281ccdc6 to f499e91e8cb2 (3 revisions) (flutter/engine#43623)
2023-07-13 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 0fvk838jTDNQ_l43k... to 3C7P0w8ySmtqpyi3S... (flutter/engine#43622)
2023-07-13 skia-flutter-autoroll@skia.org Roll Skia from bedc92598644 to 6ed93436d57c (3 revisions) (flutter/engine#43621)
2023-07-13 jonahwilliams@google.com [Impeller] Add support to embedder for Impeller on GL (via Angle on Windows). (flutter/engine#43388)
2023-07-12 jonahwilliams@google.com [Impeller] Allocate buffers out of a pool on the raster thread. (flutter/engine#43564)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 4e989b1564ee to bedc92598644 (1 revision) (flutter/engine#43617)
2023-07-12 flar@google.com move rtree and canvas_spy sources to Fuchsia sub-directory (flutter/engine#43615)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from 68e3c0b3eea7 to 4e989b1564ee (6 revisions) (flutter/engine#43614)
2023-07-12 ian@hixie.ch Document (and assert) that channel names can't contains nulls (flutter/engine#43593)
2023-07-12 58529443+srujzs@users.noreply.github.com Reland "Refactor JSNumber.toDart and Object.toJS" (flutter/engine#43363)
2023-07-12 31859944+LongCatIsLooong@users.noreply.github.com Add a flag to `ParagraphBuilder` for rounding hack migration (flutter/engine#43118)
2023-07-12 john@johnmccutchan.com [Impeller] Fixes for asymmetric stencil descriptors (flutter/engine#43535)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from adeeb8d50f7c to 68e3c0b3eea7 (1 revision) (flutter/engine#43609)
2023-07-12 flar@google.com Add comment to use of 3x3 mapRect in TransformLayer (flutter/engine#43608)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from a251a36ea519 to adeeb8d50f7c (1 revision) (flutter/engine#43606)
2023-07-12 skia-flutter-autoroll@skia.org Roll Dart SDK from b95f6531c726 to 8f8f281ccdc6 (2 revisions) (flutter/engine#43607)
2023-07-12 skia-flutter-autoroll@skia.org Roll Skia from ebc149cff431 to a251a36ea519 (2 revisions) (flutter/engine#43604)
2023-07-12 jonahwilliams@google.com [Impeller] Use new SkParagraph APIs for stroked text. (flutter/engine#41735)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 1STsUj0X5Ygp to xBJq6PsO5ebb
  fuchsia/sdk/core/mac-amd64 from 0fvk838jTDNQ to 3C7P0w8ySmtq

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
...
kjlubick pushed a commit to kjlubick/engine that referenced this pull request Jul 14, 2023
@srujzs @kjlubick
Reland "Refactor JSNumber.toDart and Object.toJS" (flutter#43363)
ddc6f44 
This reverts commit dce75ab.

This also makes some small changes to make onBenchmark a
JSExportedDartFunction instead of a JSBoxedDartObject. This is for
changes in flutter/flutter#129436 and to account
for the fact that flutter/packages provides an `allowInterop`'d
function. Benchmarks tests pass with this CL.

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [ ] All existing and new tests are passing.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joshualitt joshualitt joshualitt approved these changes

@eyebrowsoffire eyebrowsoffire Awaiting requested review from eyebrowsoffire

Assignees
No one assigned
Labels
platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@srujzs @joshualitt