[cross_file] always set browser blob when constructing XFile from data #8611

navaronbracke · 2025-02-12T14:49:09Z

This PR fixes a bug where files created with XFile.fromData() would ignore the provided bytes if the path is present, too.
Now if the bytes and the path are both provided, the content is read from the bytes directly.

Fixes flutter/flutter#163044

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

navaronbracke · 2025-02-12T14:54:03Z

@ditman I tried to run the tests for cross_file locally, but I can't seem to get that working (been a while since I last ran tests for the packages repo :P )

I tried to follow https://github.com/flutter/flutter/blob/master/docs/ecosystem/testing/Plugin-Tests.md#web-tests
Most notably, I cannot seem to get

All web packages contain a standard test directory in the root of the package that can be run with flutter test

to work? Am I missing a setup step? I updated my chromedriver, but when I try to run the tests I get:

Logs

navaronbracke@MacBook-Pro-van-Navaron cross_file % flutter test --platform=chrome                         
Error: Couldn't resolve the package 'flutter_test' in 'package:flutter_test/flutter_test.dart'.
org-dartlang-app:///main.dart:8:8: Error: Not found: 'package:flutter_test/flutter_test.dart'
import 'package:flutter_test/flutter_test.dart';
       ^
org-dartlang-app:///main.dart:10:13: Error: Type 'WebTest' not found.
Map<String, WebTest> webTestMap = <String, WebTest>{
            ^^^^^^^
org-dartlang-app:///main.dart:10:13: Error: 'WebTest' isn't a type.
Map<String, WebTest> webTestMap = <String, WebTest>{
            ^^^^^^^
org-dartlang-app:///main.dart:10:44: Error: 'WebTest' isn't a type.
Map<String, WebTest> webTestMap = <String, WebTest>{
                                           ^^^^^^^
org-dartlang-app:///main.dart:26:9: Error: 'WebTest' isn't a type.
  final WebTest? webTest = webTestMap[testSelector];
        ^^^^^^^
org-dartlang-app:///main.dart:26:39: Error: Undefined name 'testSelector'.
  final WebTest? webTest = webTestMap[testSelector];
                                      ^^^^^^^^^^^^
org-dartlang-app:///main.dart:28:37: Error: Undefined name 'testSelector'.
    throw Exception('Web test for ${testSelector} not found');
                                    ^^^^^^^^^^^^
org-dartlang-app:///main.dart:30:10: Error: Method not found: 'runWebTest'.
  return runWebTest(webTest);
         ^^^^^^^^^^
Unhandled exception:
Unsupported operation: Unsupported invalid type InvalidType(<invalid>) (InvalidType). Encountered while compiling org-dartlang-app:///main.dart, which contains the type: InterfaceType(IdentityMap<String, <invalid>>).
#0      ProgramCompiler._typeCompilationError (package:dev_compiler/src/kernel/compiler.dart:3442)
#1      ProgramCompiler._emitType (package:dev_compiler/src/kernel/compiler.dart:3410)
#2      ProgramCompiler.visitMapLiteral (package:dev_compiler/src/kernel/compiler.dart:7104)
#3      MapLiteral.accept (package:kernel/src/ast/expressions.dart:4291)
#4      ProgramCompiler._visitExpression (package:dev_compiler/src/kernel/compiler.dart:4066)
#5      ProgramCompiler._visitInitializer (package:dev_compiler/src/kernel/compiler.dart:2112)
#6      ProgramCompiler._emitStaticFieldInitializer.<anonymous closure> (package:dev_compiler/src/kernel/compiler.dart:2739)
#7      ProgramCompiler._withLetScope (package:dev_compiler/src/kernel/compiler.dart:2749)
#8      ProgramCompiler._emitStaticFieldInitializer (package:dev_compiler/src/kernel/compiler.dart:2737)
#9      ProgramCompiler._emitLazyMembers (package:dev_compiler/src/kernel/compiler.dart:2698)
#10     ProgramCompiler._emitTopLevelFields (package:dev_compiler/src/kernel/compiler.dart:2679)
#11     ProgramCompiler._emitLibrary (package:dev_compiler/src/kernel/compiler.dart:914)
#12     List.forEach (dart:core-patch/growable_array.dart:417)
#13     ProgramCompiler.emitModule (package:dev_compiler/src/kernel/compiler.dart:620)
#14     IncrementalJavaScriptBundler.compile (package:frontend_server/src/javascript_bundle.dart:231)
#15     FrontendCompiler.writeJavaScriptBundle (package:frontend_server/frontend_server.dart:874)
<asynchronous suspension>
#16     FrontendCompiler.compile (package:frontend_server/frontend_server.dart:690)
<asynchronous suspension>
#17     listenAndCompile.<anonymous closure> (package:frontend_server/frontend_server.dart:1392)
<asynchronous suspension>
the Dart compiler exited unexpectedly.
Failed to compile
Failed to compile

navaronbracke@MacBook-Pro-van-Navaron cross_file % dart test -p chrome                                    
00:08 +0: loading test/x_file_html_test.dart                                                                                                                                                                                                                                                                                                            
Compiled 14,123,444 input bytes (9,118,618 characters source) to 1,342,354 characters JavaScript in 7.02 seconds using 0.000 MB of memory

00:08 +0 -1: loading test/x_file_html_test.dart [E]                                                                                                                                                                                                                                                                                                     
  Failed to load "test/x_file_html_test.dart":
  Failed to run Chrome: No such file or directory
    Command: chrome --user-data-dir=/private/var/folders/2y/s_50nm_550qfdsqm05grr_pr0000gn/T/dart_test_PsF0uC http://localhost:56251/LSbE2vYp69pWB6ZKoQ6j48OltLUbYrtI/packages/test/src/runner/browser/static/index.html?managerUrl=ws%3A%2F%2Flocalhost%3A56251%2FLSbE2vYp69pWB6ZKoQ6j48OltLUbYrtI%2F0&debug=false --enable-logging=stderr --v=0 --disable-extensions --disable-popup-blocking --bwsi --no-first-run --no-default-browser-check --disable-default-apps --disable-translate --disable-dev-shm-usage --headless --disable-gpu --remote-debugging-port=0 --no-sandbox.
  dart:io-patch/process_patch.dart 402:33                      _ProcessImpl._start
  dart:io-patch/process_patch.dart 38:20                       Process.start
  package:test/src/runner/browser/chromium.dart 57:33          ChromiumBasedBrowser.spawn
  package:test/src/runner/browser/chrome.dart 49:57            new Chrome.<fn>.tryPort
  package:test/src/runner/browser/chrome.dart 76:40            new Chrome.<fn>
  package:test/src/runner/browser/browser.dart 61:39           new Browser.<fn>
  dart:async/zone.dart 1414:13                                 _rootRun
  dart:async/zone.dart 1317:19                                 _CustomZone.run
  dart:async/zone.dart 1838:10                                 _runZoned
  dart:async/zone.dart 1826:12                                 runZonedGuarded
  package:test/src/runner/browser/browser.dart 60:5            new Browser
  package:test/src/runner/browser/chrome.dart                  new Chrome._
  package:test/src/runner/browser/chrome.dart 47:19            new Chrome
  package:test/src/runner/browser/browser_manager.dart 157:27  BrowserManager._newBrowser
  package:test/src/runner/browser/browser_manager.dart 116:19  BrowserManager._start
  package:test/src/runner/browser/browser_manager.dart 106:7   BrowserManager.start
  package:test/src/runner/browser/platform.dart 227:33         BrowserPlatform._browserManagerFor
  ===== asynchronous gap ===========================
  package:test/src/runner/browser/platform.dart 185:26         BrowserPlatform.load
  ===== asynchronous gap ===========================
  package:test_core/src/runner/loader.dart 219:27              Loader.loadFile.<fn>
  ===== asynchronous gap ===========================
  package:test_core/src/runner/load_suite.dart 98:19           new LoadSuite.<fn>.<fn>
  

To run this test again: /Users/navaronbracke/Documents/flutter/bin/cache/dart-sdk/bin/dart test test/x_file_html_test.dart -p chrome --plain-name 'loading test/x_file_html_test.dart'
00:08 +0 -1: Some tests failed.

navaronbracke · 2025-02-12T14:58:39Z

packages/cross_file/lib/src/types/html.dart

The unnamed constructor always sets the blob when the bytes are provided.
This one didn't but we can always do that since the bytes are required for this one.

packages/cross_file/test/x_file_html_test.dart

ditman

Small comment on preferring the passed in path to the generated blob URL for a XFile.fromData.

ditman · 2025-03-28T23:46:20Z

packages/cross_file/lib/src/types/html.dart

This will create a wrong XFile if browserBlob is passed and its path is overridden, down the line, because we've been telling people to use .path on the XFile to access its blob URL; so in this case, if we've created a non empty blob, the path should NOT be used.

I think that in this case, we need to warn users who pass a path that it'll be ignored.

packages/cross_file/test/x_file_html_test.dart

ditman · 2025-03-28T23:52:00Z

navaronbracke@MacBook-Pro-van-Navaron cross_file % flutter test --platform=chrome

Try instead:

dit@dit:/work/flutter/packages/packages/cross_file$ dart test --platform=chrome
00:03 +0: loading test/x_file_html_test.dart
Compiled 14,179,976 input bytes (9,171,791 characters source) to 1,187,453 characters JavaScript in 3.79 seconds using N/A MB of memory

00:06 +13: All tests passed!

You can run the tests for the VM by omitting the platform argument.

dit@dit:/work/flutter/packages/packages/cross_file$ dart test
00:00 +12: All tests passed!

If the chrome executable gives you trouble, you can tweak its path (or name) in the dart_test.yaml file:

https://github.com/ditman/flutter-packages/blob/a1b1cf1ef953b717123dbffdbe8bf8ca0c25a214/packages/cross_file/dart_test.yaml#L5

navaronbracke · 2025-05-09T08:55:30Z

This is ready for another review. The test was updated to verify the path a bit more strictly and I documented that we will be ignoring the path on the web if there are bytes for the file.

When running the tests, I noticed that I got a popup saying the chrome process wanted me to "Continue a download", but after accepting that, the tests finished but the process seemed to hang. Is that avoidable when running the tests in a headless chrome instance?

Piinks · 2025-07-15T22:59:34Z

Hey @ditman are you able to give this another review?

mdebbar

Sorry this fell through the cracks!

As a thank you for your patience, I went ahead and rebased the PR and resolved merge conflicts :)

harryterkelsen

LGTM

auto-submit · 2025-10-29T18:17:27Z

auto label is removed for flutter/packages/8611, Failed to merge flutter/packages/8611 with FormatException: Unexpected end of input (at character 1)

^
.

navaronbracke · 2025-10-29T21:18:46Z

Sorry this fell through the cracks!

As a thank you for your patience, I went ahead and rebased the PR and resolved merge conflicts :)

Thanks!

…e from data (flutter/packages#8611)

flutter/packages@41c6b3d...1a7075b 2025-10-29 [email protected] Roll Flutter from 7cf0dc1 to df72035 (23 revisions) (flutter/packages#10322) 2025-10-29 [email protected] [cross_file] always set browser blob when constructing XFile from data (flutter/packages#8611) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

navaronbracke requested a review from ditman as a code owner February 12, 2025 14:49

github-actions bot added the p: cross_file label Feb 12, 2025

navaronbracke commented Feb 12, 2025

View reviewed changes

packages/cross_file/test/x_file_html_test.dart Outdated Show resolved Hide resolved

stuartmorgan-g self-requested a review March 25, 2025 19:43

ditman reviewed Mar 28, 2025

View reviewed changes

navaronbracke added 2 commits May 9, 2025 10:51

always set browser blob when constructing xfile from data

79eddef

override path if bytes if provided on web; update test

dfe8575

navaronbracke force-pushed the cross_file_web_bytes_fix branch from 0373cfc to dfe8575 Compare May 9, 2025 08:51

navaronbracke requested a review from ditman May 9, 2025 08:52

mkidv mentioned this pull request Jul 8, 2025

[cross_file] Fix: always create _browserBlob in fromData constructor on Web #9579

Closed

11 tasks

Merge branch 'main' into cross_file_web_bytes_fix

a40be4c

yjbanov removed the request for review from ditman October 20, 2025 23:09

mdebbar added the triage-web Should be looked at in web triage label Oct 21, 2025

Merge branch 'main' into cross_file_web_bytes_fix

b887a9d

mdebbar approved these changes Oct 29, 2025

View reviewed changes

harryterkelsen approved these changes Oct 29, 2025

View reviewed changes

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 29, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 29, 2025

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 29, 2025

auto-submit bot merged commit 6d180d9 into flutter:main Oct 29, 2025
80 checks passed

navaronbracke deleted the cross_file_web_bytes_fix branch October 29, 2025 21:18

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 30, 2025

6d180d9df [cross_file] always set browser blob when constructing XFil…

2d3255f

…e from data (flutter/packages#8611)

engine-flutter-autoroll mentioned this pull request Oct 30, 2025

Roll Packages from 41c6b3d7666a to 1a7075b09ef1 (2 revisions) flutter/flutter#177777

Merged

[cross_file] always set browser blob when constructing XFile from data #8611

[cross_file] always set browser blob when constructing XFile from data #8611

Conversation

navaronbracke commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

navaronbracke commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

navaronbracke Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

ditman Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ditman commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

navaronbracke commented May 9, 2025

Uh oh!

Piinks commented Jul 15, 2025

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Oct 29, 2025

Uh oh!

Uh oh!

navaronbracke commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

navaronbracke commented Feb 12, 2025 •

edited

Loading

navaronbracke commented Feb 12, 2025 •

edited

Loading

ditman commented Mar 28, 2025 •

edited

Loading