Skip to content

Fix reported freeze#8864

Merged
auto-submit[bot] merged 5 commits into
flutter:mainfrom
helin24:freeze
Mar 23, 2026
Merged

Fix reported freeze#8864
auto-submit[bot] merged 5 commits into
flutter:mainfrom
helin24:freeze

Conversation

@helin24
Copy link
Copy Markdown
Member

@helin24 helin24 commented Mar 19, 2026

This is to fix an internal freeze report that close can take a long time to finish.

com.teamdev.jxbrowser.browser.internal.BrowserImpl.close(BrowserImpl.java:779)
com.teamdev.jxbrowser.internal.rpc.UniversalServiceConnection.invoke(UniversalServiceConnection.java:183)
com.teamdev.jxbrowser.internal.rpc.transport.CommonThreadCallExecutor.execute(CommonThreadCallExecutor.java:29)
==> com.teamdev.jxbrowser.internal.rpc.transport.CommonThreadResponseConsumer.await(CommonThreadResponseConsumer.java:49) <==
java.util.concurrent.CountDownLatch.await(Unknown Source)

I don't really have a good way to test (e.g. making this part slow), other than to make sure that opening and closing the tool windows remains okay.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a UI freeze during shutdown by moving potentially long-running close() calls for JxBrowser components to a background thread. The changes in EmbeddedBrowser.java and WidgetPreviewPanel.java are appropriate as they occur during project/component disposal, not application exit.

However, the change in EmbeddedBrowserEngine.java for application shutdown is risky. It uses a fire-and-forget background task, which may not complete before the IDE exits, potentially leading to data corruption. I've left a comment with a suggestion to use the Disposable pattern, which is the standard and safer way to handle cleanup during application shutdown in IntelliJ plugins.

Comment thread src/io/flutter/jxbrowser/EmbeddedBrowserEngine.java Outdated
@helin24
Copy link
Copy Markdown
Member Author

helin24 commented Mar 20, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a potential UI freeze by moving potentially long-running close() operations for the embedded browser to a background thread. The changes in EmbeddedBrowserEngine, EmbeddedBrowser, and WidgetPreviewPanel are consistent in using safeExecuteOnPooledThread to avoid blocking the Event Dispatch Thread (EDT), which is a good practice for IntelliJ plugin development and aligns with the repository's style guide on threading. I have one suggestion to further improve the design by centralizing the asynchronous logic to avoid code duplication.

Comment thread src/io/flutter/view/EmbeddedBrowser.java Outdated
@helin24
Copy link
Copy Markdown
Member Author

helin24 commented Mar 20, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a potential UI freeze by moving the blocking JBrowser.close() calls to a background thread. The changes are well-aligned with IntelliJ Platform's threading best practices. Using Disposable for EmbeddedBrowserEngine is a good architectural improvement. I've pointed out a minor code duplication issue that could be refactored for better maintainability.

Comment thread src/io/flutter/jxbrowser/EmbeddedJxBrowser.java
@helin24 helin24 requested a review from pq March 20, 2026 20:18
@helin24 helin24 added the autosubmit When this label is applied to a PR, the PR will be submitted as soon as all checks are green. label Mar 23, 2026
@auto-submit auto-submit Bot merged commit 66bcb11 into flutter:main Mar 23, 2026
8 checks passed
@helin24 helin24 deleted the freeze branch March 23, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit When this label is applied to a PR, the PR will be submitted as soon as all checks are green.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants