feat: add JCEF reload action #8711
Conversation
QianKuang8
requested review from
Patrick-Erichsen
and removed request for
a team
dosubot
bot
added
the
size:M
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
dosubot
bot
added
size:L
QianKuang8
force-pushed
the
qian/jcef-reload
branch
from
3fa9081 to
f677852
Compare
| private val myJBCefClient: JBCefClient = JBCefApp.getInstance().createClient().apply { | ||
| setProperty(JBCefClient.Properties.JS_QUERY_POOL_SIZE, 200) | ||
| } | ||
| private val browser: JBCefBrowser = JBCefBrowser.createBuilder().setOffScreenRendering(true).setClient(myJBCefClient).build() |
There was a problem hiding this comment.
how come were changing this logic?
There was a problem hiding this comment.
Commit e998ae8 was added to try address the issue mentioned in #8085 (comment)
[5473397] WARN - #c.i.u.j.JBCefJSQuery - Set the property JBCefClient.Properties.JS_QUERY_POOL_SIZE to use JBCefJSQuery after the browser has been created
If you have concerns about the fix, I can revert that commit so we can focus on the reload action in this PR.
Patrick-Erichsen
left a comment
Patrick-Erichsen
left a comment
There was a problem hiding this comment.
Can you explain the changes here? I'm weary to modify the core browser logic and potentially making things worse than they already are.
…r the browser has been created
…ery after the browser has been created" This reverts commit e998ae8.
QianKuang8
force-pushed
the
qian/jcef-reload
branch
from
a097b35 to
3d71e5e
Compare
dosubot
bot
added
size:M
QianKuang8
force-pushed
the
qian/jcef-reload
branch
from
3d71e5e to
8bfc78d
Compare
|
@QianKuang8 I see you pushed some more commits, appreciate you moving this forward but a quick bump on my comment above:
|
QianKuang8
left a comment
QianKuang8
left a comment
There was a problem hiding this comment.
Sorry, previously I didn't understand your comment.
Add some explanation on my change.
Also, currently the reload action still not work when freeze. And I have worked on this for few days. I would update the PR if I found a way to make it work.
| } | ||
| } | ||
|
|
||
| class ReloadBrowserAction: ContinueToolbarAction() { |
There was a problem hiding this comment.
An action to trigger the browser reload
| <reference ref="continue.newContinueSession"/> | ||
| <reference ref="continue.viewHistory"/> | ||
| <reference ref="continue.openConfigPage"/> | ||
| <reference ref="continue.reloadPage" /> |
| fun Project.getBrowser(): ContinueBrowser? { | ||
| if (isDisposed) | ||
| return null | ||
| return service<ContinueBrowserService>().browser |
There was a problem hiding this comment.
Other component still use this function to get browser. No changes here.
| * Reloads the browser by disposing the current one and creating a new one. | ||
| * This method is intended for use when browser is frozen (unresponsive). | ||
| */ | ||
| fun reload() { |
There was a problem hiding this comment.
Only the new reload action use this function.
There was a problem hiding this comment.
What the function do
- dispose the old browser
- create a new browser
| override fun dispose() { | ||
| if (browser != null) | ||
| Disposer.dispose(browser) | ||
| browser?.let { Disposer.dispose(it) } | ||
| browser = null | ||
| } |
There was a problem hiding this comment.
Will set the browser to null after dispose
| private var browser: ContinueBrowser? = null | ||
|
|
||
| init { | ||
| load() |
There was a problem hiding this comment.
Use the init block to set the value of browser
| return null | ||
| } | ||
| val newBrowser = ContinueBrowser(project) | ||
| Disposer.register(this, newBrowser) |
There was a problem hiding this comment.
Browser would be disposed when the BrowserService is disposed after the registration
| browser = null | ||
| } | ||
|
|
||
| private fun load(): ContinueBrowser? { |
There was a problem hiding this comment.
Most of the logic is same to
private val browser: ContinueBrowser? =
if (JBCefApp.isSupported())
ContinueBrowser(project)
else null
|
Got it, thank you for the detailed comments! This makes more sense, seems like a good patch until we fix the underlying freeze issues. |
2 participants
Description
Discussion in #8085
Add a reload button and an action
With manual test, it works well in normal case. Not 100% sure whether it would work when UI freezes.
video.mp4
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Adds a Reload button for the Continue tool window that rebuilds the JCEF browser and tool window content to recover from freezes or crashes.
Written for commit 8bfc78d. Summary will update automatically on new commits.