loadfontfromlist should send fontchange message to framework #14805

chunhtai · 2019-12-26T06:51:07Z

This pr makes sure the ui.windows loadfontfromlist api will notify framework to rebuild

chunhtai · 2020-01-09T01:45:07Z

This is ready for review

jonahwilliams · 2020-01-09T02:21:47Z

lib/ui/text.dart

+  if (window.onPlatformMessage != null)
+    window.onPlatformMessage(
+      'flutter/system',
+      utf8.encoder.convert(json.encode(<String, dynamic>{'type': 'fontsChange'})).buffer.asByteData(),


This seems wasteful. Could you instead create a static buffer which already contains this data?

jonahwilliams · 2020-01-09T02:21:52Z

lib/web_ui/lib/src/ui/text.dart

+
+FutureOr<void> _sendFontChangeMessage(void result) async {
+  if (window.onPlatformMessage != null)
+    window.onPlatformMessage(


I moved it out to a final variable

chunhtai · 2020-01-09T03:44:25Z

@jonahwilliams thanks for reviewing! I pushed the change

jonahwilliams · 2020-01-09T06:01:06Z

lib/ui/text.dart

+  ).then(_sendFontChangeMessage);
+}
+
+final ByteData _fontChangeMessage = utf8.encoder.convert(


This could also be static, to prevent creating multiple instances

This is not in a class. Do you mean put this field to be a static member for some class? I am not familiar with how dart compile the code, I thought define it in the scope of this file is the same thing as a static member of a class?

Oh, reading fail on my part. it is fine as a top level

jason-simmons

LGTM

jason-simmons · 2020-01-16T20:09:20Z

lib/web_ui/lib/src/ui/text.dart

+      _fontChangeMessage,
+      (_) {},
+    );
+}


Add a newline at the end of the file for consistency

mdebbar

LGTM if LGT @hterkelsen

jonahwilliams

LGTM

harryterkelsen · 2020-01-16T20:48:24Z

lib/ui/text.dart

+  json.encode(<String, dynamic>{'type': 'fontsChange'})
+).buffer.asByteData();
+
+FutureOr<void> _sendFontChangeMessage(void result) async {


nit: I think it would be clearer if this function didn't have an unused parameter named result and instead we wrote .then((_) => _sendFontChangeMessage())

But it's not a hard preference

…lutter/engine#14805)

``` git log 8df1757..ba045a2 --first-parent --oneline 2020-01-17 skia-flutter-autoroll@skia.org Roll src/third_party/dart c8f8c11b70b4..843bd2990881 (5 commits) (flutter/engine#15714) 2020-01-17 aam@google.com Roll dart to c8f8c11b70 (flutter/engine#15708) 2020-01-17 garyq@google.com Samsung fix duplication on punctuation: Update keyboard on finish compose. (flutter/engine#15701) 2020-01-17 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2cd5d43f022c..d58b643f10f5 (21 commits) (flutter/engine#15707) 2020-01-17 chinmaygarde@google.com Disable GPUThreadMerger tests inline instead of via harness flags. (flutter/engine#15706) 2020-01-17 dnfield@google.com implicit casts and add missing docs (flutter/engine#15698) 2020-01-17 47866232+chunhtai@users.noreply.github.com loadfontfromlist should send fontchange message to framework (flutter/engine#14805) ```

…lutter/engine#14805)

flutter/engine@8df1757...98c1aea git log 8df1757..98c1aea --first-parent --oneline 2020-01-17 ferhat@gmail.com Clipping path fix for browsers that don't use correct units when applying clip-path css using svg (flutter/engine#15712) 2020-01-17 dnfield@google.com fix typo, update path metrics docs (flutter/engine#15715) 2020-01-17 skia-flutter-autoroll@skia.org Roll src/third_party/dart c8f8c11b70b4..843bd2990881 (5 commits) (flutter/engine#15714) 2020-01-17 aam@google.com Roll dart to c8f8c11b70 (flutter/engine#15708) 2020-01-17 garyq@google.com Samsung fix duplication on punctuation: Update keyboard on finish compose. (flutter/engine#15701) 2020-01-17 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2cd5d43f022c..d58b643f10f5 (21 commits) (flutter/engine#15707) 2020-01-17 chinmaygarde@google.com Disable GPUThreadMerger tests inline instead of via harness flags. (flutter/engine#15706) 2020-01-17 dnfield@google.com implicit casts and add missing docs (flutter/engine#15698) 2020-01-17 47866232+chunhtai@users.noreply.github.com loadfontfromlist should send fontchange message to framework (flutter/engine#14805) 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 jimgraham@google.com on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

…#14805)

…lutter#14805)" This reverts commit da025ac.

…#14805)

auto-assign bot requested a review from flar December 26, 2019 06:51

googlebot added cla: yes labels Dec 26, 2019

chunhtai requested a review from goderbauer December 27, 2019 01:30

jonahwilliams reviewed Jan 9, 2020

View reviewed changes

chunhtai force-pushed the issues/44460 branch from 61df4bd to 81d9ea1 Compare January 9, 2020 03:40

jonahwilliams reviewed Jan 9, 2020

View reviewed changes

chunhtai force-pushed the issues/44460 branch from 81d9ea1 to e89fd84 Compare January 16, 2020 18:05

chunhtai requested a review from jason-simmons January 16, 2020 18:08

jason-simmons reviewed Jan 16, 2020

View reviewed changes

mdebbar reviewed Jan 16, 2020

View reviewed changes

jonahwilliams approved these changes Jan 16, 2020

View reviewed changes

harryterkelsen approved these changes Jan 16, 2020

View reviewed changes

chunhtai added 7 commits January 16, 2020 15:21

loadfontfromlist should send fontchange message to framework

47f8e04

update

18a2aef

fix test

aefa41c

clean up

d828b0f

addressing comment

7e62f5d

add line at the end

65b958b

clean up future chain

585a7a0

chunhtai force-pushed the issues/44460 branch from 90c2f81 to 585a7a0 Compare January 16, 2020 23:21

chunhtai added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jan 17, 2020

fluttergithubbot merged commit 1f4c593 into flutter:master Jan 17, 2020

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 17, 2020

1f4c593 loadfontfromlist should send fontchange message to framework (f…

ad77457

…lutter/engine#14805)

engine-flutter-autoroll mentioned this pull request Jan 17, 2020

Roll engine 8df1757d3566..ba045a29ee73 (7 commits) flutter/flutter#49071

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 17, 2020

1f4c593 loadfontfromlist should send fontchange message to framework (f…

243c5a2

…lutter/engine#14805)

engine-flutter-autoroll mentioned this pull request Jan 17, 2020

Roll engine 8df1757d3566..ba045a29ee73 (7 commits) flutter/flutter#49074

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 17, 2020

1f4c593 loadfontfromlist should send fontchange message to framework (f…

8d627ec

…lutter/engine#14805)

engine-flutter-autoroll mentioned this pull request Jan 17, 2020

Roll engine 8df1757d3566..98c1aeaa3b1b (9 commits) flutter/flutter#49079

Merged

NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

loadfontfromlist should send fontchange message to framework (flutter…

da025ac

…#14805)

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "loadfontfromlist should send fontchange message to framework (f…

ab440e5

…lutter#14805)" This reverts commit da025ac.

filmil pushed a commit to filmil/engine that referenced this pull request Mar 13, 2020

loadfontfromlist should send fontchange message to framework (flutter…

7832517

…#14805)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

loadfontfromlist should send fontchange message to framework #14805

loadfontfromlist should send fontchange message to framework #14805

chunhtai commented Dec 26, 2019

chunhtai commented Jan 9, 2020

jonahwilliams Jan 9, 2020

jonahwilliams Jan 9, 2020

chunhtai Jan 9, 2020

jonahwilliams Jan 9, 2020

chunhtai commented Jan 9, 2020

jonahwilliams Jan 9, 2020

chunhtai Jan 9, 2020

jonahwilliams Jan 9, 2020

jason-simmons left a comment

jason-simmons Jan 16, 2020

mdebbar left a comment

jonahwilliams left a comment

harryterkelsen Jan 16, 2020

loadfontfromlist should send fontchange message to framework #14805

loadfontfromlist should send fontchange message to framework #14805

Conversation

chunhtai commented Dec 26, 2019

chunhtai commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunhtai commented Jan 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jason-simmons left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdebbar left a comment

Choose a reason for hiding this comment

jonahwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment