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

允许在上传/下载开始之后继续添加新的listener #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AnotherJack
Copy link

之前是必须在网络请求之前添加好所有的listener,但是有些场景需要在网络请求之后还可以添加新的listener。
比如常见的应用市场,app列表页面每一个item会有一个下载按钮,点击后开始下载并显示进度,而点击item进入详情页也会有一个带进度的下载按钮,如果先在列表页面点击了下载,再进入详情页,详情页也应该和列表页同步地获取到进度信息。
主要改动是ProgressRequestBody和ProgressResponseBody中的mListeners由数组改为List,与ProgressManager中的listener指向相同的内存地址,同时构造函数中的listeners也不再有null的情况,在ProgressManager的wrap方法中,如果根据url获取到的listeners为null,会put进去一个空的List并传给ProgressRequestBody/ProgressResponseBody。

@AnotherJack
Copy link
Author

又做了一些改动,ProgressManager中之前是对url使用弱引用,现在改为对listener使用弱引用,即由原来的List改为Set,其中此set是由Collections.newSetFromMap(new WeakHashMap<ProgressListener, Boolean>())创建的,不会对listener有leak。
现在的listener是使用一次就会被回收的,在example中点击下载,下载完成后,过一会儿(等待gc)再点下载会不改进度,因为在上次下载完成后listener不再被引用而被回收。所以使用中要注意在业务代码中处理好对listener的引用

@JessYanCoding
Copy link
Owner

JessYanCoding commented Sep 26, 2019

抱歉,这么久才来回复,当时看到了这个 PR 后,还没认真看里面的代码,就忙其他事情去了,就忘了。。。

感谢贡献的代码,将之前弱引用 URl,改为现在的弱引用 Listener,挺赞的,这可以让各自注册监听器的地方,可以自行管理自己的监听器的生命周期。

但是将之前 if (mRequestListeners.containsKey(key)) 才给当前 Request 添加 ProgressRequestBody 的逻辑,改为不管开发者是否想监听这个这个 URL,只要开发者使用了 ProgressManager 就会对项目中所有请求的 URl 添加 ProgressRequestBody,即便这个请求没有监听的需求,而项目中实际需要监听的 URL 明显比不需要监听的 URL 少很多。

这将对所有 URL 产生影响,个人认为这个优化不是很好,会增加很多无效操作,对不需要监听的 URL 来说没有什么意义

@JessYanCoding
Copy link
Owner

所以我不是很清楚去掉 mRequestListeners.containsKey(key) 的必要性,还是说这次更新有什么地方必须要保证 listeners 不为空,是否可以在继续保留 if (mRequestListeners.containsKey(key)) 的情况下,将 Listener 改为弱引用

@AnotherJack
Copy link
Author

所以我不是很清楚去掉 mRequestListeners.containsKey(key) 的必要性,还是说这次更新有什么地方必须要保证 listeners 不为空,是否可以在继续保留 if (mRequestListeners.containsKey(key)) 的情况下,将 Listener 改为弱引用

这个当时是这样考虑的,如果一个请求开始的时候没有添加listener,而是进行到一半的时候才添加,这样可以让后边加的listener起作用

@JessYanCoding
Copy link
Owner

但是为了完成这个需求,代价太过庞大,需要监听的 URL 在项目中是少数,想实现请求之前未监听但中途需要监听的 URL 更是少数,为了实现这个需求,付出让全部 URL 的 RequestBody 都被重写的代价,个人认为这样不是很好,性价比不是很高。

可以使用文档告诉用户,如果想实现让之前未实现监听,但在请求中途突然需要添加监听的需求,就需要在 URL 开始之前随便添加个空实现的监听器占位,这样只需要让 ProgressRequestBody 中的数组变成 List,就可以实现请求中途添加监听器的需求。

所以我想问下,在这个 PR 中能否只保留对弱引用和数组的优化,去掉对 mRequestListeners.containsKey(key) 的优化,或者你还有什么更好的方案

@AnotherJack
Copy link
Author

但是为了完成这个需求,代价太过庞大,需要监听的 URL 在项目中是少数,想实现请求之前未监听但中途需要监听的 URL 更是少数,为了实现这个需求,付出让全部 URL 的 RequestBody 都被重写的代价,个人认为这样不是很好,性价比不是很高。

可以使用文档告诉用户,如果想实现让之前未实现监听,但在请求中途突然需要添加监听的需求,就需要在 URL 开始之前随便添加个空实现的监听器占位,这样只需要让 ProgressRequestBody 中的数组变成 List,就可以实现请求中途添加监听器的需求。

所以我想问下,在这个 PR 中能否只保留对弱引用和数组的优化,去掉对 mRequestListeners.containsKey(key) 的优化,或者你还有什么更好的方案

可以,让使用者主动添加一个空的listener占位可以解决这个问题

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

2 participants