Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: file protocol and test cases #1234

Merged
merged 17 commits into from Mar 24, 2022
Merged

fix: file protocol and test cases #1234

merged 17 commits into from Mar 24, 2022

Conversation

wssgcg1213
Copy link
Member

  • 修复 KrakenBundle 不支持 file:// 协议的问题
    • 配套测试
  • 移除了 NetworkAssetBundle, 这个类不是 KrakenBundle 而是 flutter/service 的 AssetBundle, 起的作用是封装 Http 请求, 理论上无必要, 且每次都新建一个 HttpClient, 本次改动改成复用同一个 HttpClient, 逻辑收敛到 NetworkBundle 类上.
    • 后续会把图片的 httpClient 与这个 httpClient 共享

@wssgcg1213 wssgcg1213 requested review from andycall, yuanyan and answershuto and removed request for andycall March 22, 2022 08:24
@@ -61,7 +76,7 @@ abstract class KrakenBundle {
// Uri parsed by uriParser, assigned after resolving.
Uri? _uri;

late ByteData rawBundle;
ByteData? rawBundle;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 rawBundle 似乎 late 是没有问题的?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

late 的前提是 [逻辑上能保证它在被访问时一定有值], 但是目前的逻辑做不到, 原因是:
resolve 阶段会获取数据并存储, 但是存储的字段包括 content/bytecode/rawBundle 三选一, 所以在访问时候判断前三者中某一个有内容即可访问. 但是 late 修饰的字段在未赋值时是不可访问的, 即使判空也会抛出异常, 这个异常不是逻辑预期内的异常. 从另一个角度来看, content/bytecode/rawBundle 三个属性修饰应当一致, 也就是说要么全部 late, 要么全部不 late

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future<Uint8List> get data;
ContentType get contentType;

统一成这样如何, 用 contentType 来标识内容, 返回一个 List 的数据, 怎么消费就由消费者决定.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

更好的设计?

abstract class ResourceLoader -> NetworkResourceLoader/AssetsResourceLoader/FileResourceLoader/... (singleton)?

还没想好, 这里我计划在下个月再进行一次重构, 目标

  1. 将 image/script/link/XHR&fetch 等其它需要加载资源的地方都依赖一个实现 (ResourceLoader), 以减少重复逻辑
  2. 将 bundle 剥离 bridge 依赖, 移动到 foundation 下

可能与 controller 的重构放在一起执行

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好了, step 1 已经改好了, 下个月计划做上面的改动

andycall
andycall previously approved these changes Mar 23, 2022
andycall
andycall previously approved these changes Mar 23, 2022
const String DEFAULT_URL = 'about:blank';
const String ASSETS_PROTOCOL = 'assets:';

final ContentType css = ContentType('text', 'css', charset: 'utf-8');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

css -> _cssMimeType

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_cssMimeType = 'text/css'

@andycall andycall merged commit 6f48f57 into main Mar 24, 2022
@andycall andycall deleted the fix/file-protocol branch March 24, 2022 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants