[newUiOnboarding] Fix Alarm leak when reopening the popup

The `onClosed()` can happen in the process of disposing the step disposable. At this moment `disposable.isDisposed` is false, but if we take it as a parent for the Alarm - the Alarm won't be disposed at all.

GitOrigin-RevId: 5df712badf43d0ac011900e803ce771ec537391f
This commit is contained in:
Konstantin Hudyakov
2023-10-23 15:32:21 +03:00
committed by intellij-monorepo-bot
parent b44dbadd62
commit 2c0600536a

View File

@@ -1,9 +1,11 @@
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.platform.ide.newUiOnboarding
import com.intellij.concurrency.currentThreadContext
import com.intellij.ide.DataManager
import com.intellij.ide.actions.DistractionFreeModeController
import com.intellij.ide.util.PropertiesComponent
import com.intellij.openapi.Disposable
import com.intellij.openapi.actionSystem.ActionPlaces
import com.intellij.openapi.actionSystem.AnActionEvent
import com.intellij.openapi.actionSystem.Toggleable
@@ -11,12 +13,10 @@ import com.intellij.openapi.actionSystem.ex.ActionUtil
import com.intellij.openapi.actionSystem.impl.ActionButton
import com.intellij.openapi.application.EDT
import com.intellij.openapi.diagnostic.thisLogger
import com.intellij.openapi.progress.runBlockingCancellable
import com.intellij.openapi.project.Project
import com.intellij.openapi.ui.popup.JBPopup
import com.intellij.openapi.ui.popup.JBPopupListener
import com.intellij.openapi.ui.popup.LightweightWindowEvent
import com.intellij.openapi.util.CheckedDisposable
import com.intellij.openapi.util.Disposer
import com.intellij.openapi.util.registry.Registry
import com.intellij.openapi.util.text.TextWithMnemonic
@@ -29,14 +29,11 @@ import com.intellij.ui.WebAnimationUtils
import com.intellij.ui.awt.RelativePoint
import com.intellij.ui.popup.AbstractPopup
import com.intellij.ui.popup.WizardPopup
import com.intellij.util.Alarm
import com.intellij.util.Alarm.ThreadToUse
import com.intellij.util.SlowOperations
import com.intellij.util.ui.JBUI
import com.intellij.util.ui.StartupUiUtil
import com.intellij.util.ui.UIUtil
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import kotlinx.coroutines.*
import org.jetbrains.annotations.ApiStatus
import org.jetbrains.annotations.Nls
import java.awt.Component
@@ -93,7 +90,7 @@ object NewUiOnboardingUtil {
return component as? T
}
suspend fun showToolbarComboButtonPopup(button: ToolbarComboButton, action: ExpandableComboAction, disposable: CheckedDisposable): JBPopup? {
suspend fun showToolbarComboButtonPopup(button: ToolbarComboButton, action: ExpandableComboAction, disposable: Disposable): JBPopup? {
return showNonClosablePopup(
disposable,
createPopup = {
@@ -116,11 +113,15 @@ object NewUiOnboardingUtil {
)
}
suspend fun showNonClosablePopup(disposable: CheckedDisposable,
createPopup: suspend () -> JBPopup?,
showPopup: (JBPopup) -> Unit): JBPopup? {
suspend fun showNonClosablePopup(disposable: Disposable,
createPopup: suspend () -> JBPopup?,
showPopup: (JBPopup) -> Unit): JBPopup? {
val popup = createPopup() ?: return null
Disposer.register(disposable) { popup.closeOk(null) }
// Can't provide parent coroutine scope here, but need it to reopen the popup.
// All created scopes will be closed when onboarding step dispose.
val coroutineScope = CoroutineScope(currentThreadContext())
Disposer.register(disposable) { coroutineScope.cancel() }
if (popup is WizardPopup) {
// do not install PopupDispatcher, otherwise some cancel settings that are installed below won't work.
@@ -136,15 +137,11 @@ object NewUiOnboardingUtil {
popup.addListener(object : JBPopupListener {
override fun onClosed(event: LightweightWindowEvent) {
Alarm(ThreadToUse.POOLED_THREAD, disposable).addRequest(Runnable {
if (!disposable.isDisposed) {
runBlockingCancellable {
withContext(Dispatchers.EDT) {
showNonClosablePopup(disposable, createPopup, showPopup)
}
}
}
}, 500)
coroutineScope.launch(Dispatchers.EDT) {
delay(500)
ensureActive()
showNonClosablePopup(disposable, createPopup, showPopup)
}
}
})
showPopup(popup)