fix: fix ui command buffer instance leak. #1014
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI Failed @andycall |
yuanyan
reviewed
Dec 24, 2021
| @@ -91,7 +91,6 @@ void initJSPagePool(int poolSize) { | |||
| // When dart hot restarted, should dispose previous bridge and clear task message queue. | |||
| if (inited) { | |||
| disposeAllPages(); | |||
| foundation::UICommandBuffer::instance(0)->clear(); | |||
| }; | |||
| pageContextPool = new kraken::KrakenPage*[poolSize]; | |||
There was a problem hiding this comment.
KrakenPage 好像换成 KrakenView 更合适些，不是所有的 kraken 都是 page
There was a problem hiding this comment.
// A Page roughly corresponds to a tab or popup window in a browser. It owns a
// tree of frames (a blink::FrameTree). The root frame is called the main frame.
Page 借鉴了 blink 里面的概念，等价于一个标签页。后面实现多个 kraken 嵌套的功能的时候，可以再里面扩展出 frames 就可以了。一个 Kraken 就等于一个 page，至少现在的设计就是这样。
yuanyan
reviewed
Dec 24, 2021
| // UnitTest will free page after test suit complete. | ||
|
|
||
| // In order to avoid accessing pageContextPool when the page is being released. We need to clear the value in pageContextPool before releasing. | ||
| pageContextPool[contextId] = nullptr; |
There was a problem hiding this comment.
pageContextPool -> contextPool 好像足够表达清楚了
There was a problem hiding this comment.
pageContextPool 里面都是持有 page，这样命名没问题，如果使用 contextPool，很难理解里面是个啥，是 page，还是 executionContext，还是 jscontext
answershuto
reviewed
Dec 24, 2021
| @@ -0,0 +1,33 @@ | |||
| /* | |||
| * Copyright (C) 2020 Alibaba Inc. All rights reserved. | |||
wssgcg1213
approved these changes
Dec 30, 2021
answershuto
approved these changes
Dec 31, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
foundation::UICommandBuffer 曾经的实现会按照每个 contextId 创建一个单例，即不线程安全，也不太方便释放，也会导致没创建一个页面，都会泄漏一个 UICommandBuffer 对象。
Fix: 从生命周期的角度来看，UICommandBuffer 和 ExecutionContext 一样，所以应当让 ExecutionContext 持有 UICommandBuffer