Skip to content

Commit 5efb60d

Browse files
authored
Merge pull request #995 from wordpress-mobile/fix/remove-memory-leak-of-aztec-text
Clear references in AztecExceptionHandler.kt
2 parents f8b32ee + 5146442 commit 5efb60d

File tree

10 files changed

+47
-33
lines changed

10 files changed

+47
-33
lines changed

aztec/src/main/kotlin/org/wordpress/aztec/AztecExceptionHandler.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import org.wordpress.android.util.AppLog
66
import org.wordpress.aztec.exceptions.DynamicLayoutGetBlockIndexOutOfBoundsException
77
import org.wordpress.aztec.util.AztecLog
88
import java.lang.Thread.UncaughtExceptionHandler
9+
import java.lang.ref.WeakReference
910

10-
class AztecExceptionHandler(private val logHelper: ExceptionHandlerHelper?, private val visualEditor: AztecText) : UncaughtExceptionHandler {
11+
class AztecExceptionHandler(private var logHelper: WeakReference<ExceptionHandlerHelper>, private var visualEditor: WeakReference<AztecText>) : UncaughtExceptionHandler {
12+
constructor(logHelper: ExceptionHandlerHelper, visualEditor: AztecText): this(WeakReference(logHelper), WeakReference(visualEditor))
1113

1214
interface ExceptionHandlerHelper {
1315
fun shouldLog(ex: Throwable) : Boolean
@@ -25,7 +27,7 @@ class AztecExceptionHandler(private val logHelper: ExceptionHandlerHelper?, priv
2527
// Check if we should log the content or not
2628
var shouldLog = true
2729
try {
28-
shouldLog = logHelper?.shouldLog(ex) ?: true
30+
shouldLog = logHelper.get()?.shouldLog(ex) ?: true
2931
} catch (e: Throwable) {
3032
AppLog.w(AppLog.T.EDITOR, "There was an exception in the Logger Helper. Set the logging to true")
3133
}
@@ -34,12 +36,14 @@ class AztecExceptionHandler(private val logHelper: ExceptionHandlerHelper?, priv
3436
// Try to report the HTML code of the content, the spans details, but do not report exceptions that can occur logging the content
3537
try {
3638
AppLog.e(AppLog.T.EDITOR, "HTML content of Aztec Editor before the crash:")
37-
AppLog.e(AppLog.T.EDITOR, visualEditor.toPlainHtml(false))
39+
AppLog.e(AppLog.T.EDITOR, visualEditor.get()?.toPlainHtml(false) ?: "Editor was cleared")
3840
} catch (e: Throwable) {
3941
AppLog.e(AppLog.T.EDITOR, "Oops! There was an error logging the HTML code.")
4042
}
4143
try {
42-
AztecLog.logContentDetails(visualEditor)
44+
visualEditor.get()?.let {
45+
AztecLog.logContentDetails(it)
46+
}
4347
} catch (e: Throwable) {
4448
// nop
4549
}
@@ -58,14 +62,16 @@ class AztecExceptionHandler(private val logHelper: ExceptionHandlerHelper?, priv
5862
detected = true
5963
}
6064
if (detected) {
61-
visualEditor.externalLogger?.logException(DynamicLayoutGetBlockIndexOutOfBoundsException("Error #8828", ex))
65+
visualEditor.get()?.externalLogger?.logException(DynamicLayoutGetBlockIndexOutOfBoundsException("Error #8828", ex))
6266
}
6367
}
6468

6569
rootHandler?.uncaughtException(thread, ex)
6670
}
6771

6872
fun restoreDefaultHandler() {
73+
visualEditor.clear()
74+
logHelper.clear()
6975
Thread.setDefaultUncaughtExceptionHandler(rootHandler)
7076
}
7177
}

aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ import org.wordpress.aztec.watchers.event.text.BeforeTextChangedEventData
125125
import org.wordpress.aztec.watchers.event.text.OnTextChangedEventData
126126
import org.wordpress.aztec.watchers.event.text.TextWatcherEvent
127127
import org.xml.sax.Attributes
128+
import java.lang.ref.WeakReference
128129
import java.security.MessageDigest
129130
import java.security.NoSuchAlgorithmException
130131
import java.util.Arrays
@@ -1390,7 +1391,7 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
13901391
disableTextChangedListener()
13911392

13921393
builder.getSpans(0, builder.length, AztecDynamicImageSpan::class.java).forEach {
1393-
it.textView = this
1394+
it.textView = WeakReference(this)
13941395
}
13951396

13961397
val cursorPosition = consumeCursorPosition(builder)

aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecDynamicImageSpan.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import android.graphics.Rect
77
import android.graphics.drawable.Drawable
88
import android.text.style.DynamicDrawableSpan
99
import org.wordpress.aztec.AztecText
10+
import java.lang.ref.WeakReference
1011

1112
abstract class AztecDynamicImageSpan(val context: Context, protected var imageDrawable: Drawable?) : DynamicDrawableSpan() {
12-
var textView: AztecText? = null
13+
var textView: WeakReference<AztecText>? = null
1314
var aspectRatio: Double = 1.0
1415

1516
private var measuring = false
@@ -88,11 +89,11 @@ abstract class AztecDynamicImageSpan(val context: Context, protected var imageDr
8889
}
8990

9091
fun adjustBounds(start: Int): Rect {
91-
if (textView == null || textView?.widthMeasureSpec == 0) {
92+
if (textView == null || textView?.get()?.widthMeasureSpec == 0) {
9293
return Rect(imageDrawable?.bounds ?: Rect(0, 0, 0, 0))
9394
}
9495

95-
val layout = textView?.layout
96+
val layout = textView?.get()?.layout
9697

9798
if (measuring || layout == null) {
9899
// if we're in pre-layout phase, just return an empty rect
@@ -143,7 +144,7 @@ abstract class AztecDynamicImageSpan(val context: Context, protected var imageDr
143144

144145
if (imageDrawable != null) {
145146
var transY = top
146-
if (mVerticalAlignment == DynamicDrawableSpan.ALIGN_BASELINE) {
147+
if (mVerticalAlignment == ALIGN_BASELINE) {
147148
transY -= paint.fontMetricsInt.descent
148149
}
149150

aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecHorizontalRuleSpan.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import android.content.Context
44
import android.graphics.drawable.Drawable
55
import org.wordpress.aztec.AztecAttributes
66
import org.wordpress.aztec.AztecText
7+
import java.lang.ref.WeakReference
78

89
class AztecHorizontalRuleSpan(context: Context, drawable: Drawable, override var nestingLevel: Int,
910
override var attributes: AztecAttributes = AztecAttributes(), editor: AztecText? = null) :
1011
AztecDynamicImageSpan(context, drawable), IAztecFullWidthImageSpan, IAztecSpan {
1112
init {
12-
textView = editor
13+
textView = WeakReference(editor)
1314
}
1415

1516
override val TAG: String = "hr"

aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecMediaSpan.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import android.graphics.drawable.Drawable
88
import android.view.Gravity
99
import org.wordpress.aztec.AztecAttributes
1010
import org.wordpress.aztec.AztecText
11+
import java.lang.ref.WeakReference
1112
import java.util.ArrayList
1213

1314
abstract class AztecMediaSpan(context: Context, drawable: Drawable?, override var attributes: AztecAttributes = AztecAttributes(),
@@ -18,7 +19,7 @@ abstract class AztecMediaSpan(context: Context, drawable: Drawable?, override va
1819
private val overlays: ArrayList<Pair<Drawable?, Int>> = ArrayList()
1920

2021
init {
21-
textView = editor
22+
textView = editor?.let { WeakReference(editor) }
2223
}
2324

2425
fun setOverlay(index: Int, newDrawable: Drawable?, gravity: Int) {

media-placeholders/src/main/java/org/wordpress/aztec/placeholders/AztecPlaceholderSpan.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import org.wordpress.aztec.AztecText
88
import org.wordpress.aztec.spans.AztecMediaSpan
99
import org.wordpress.aztec.spans.IAztecFullWidthImageSpan
1010
import org.wordpress.aztec.spans.IAztecSpan
11+
import java.lang.ref.WeakReference
1112

1213
class AztecPlaceholderSpan(
1314
context: Context,
@@ -16,14 +17,14 @@ class AztecPlaceholderSpan(
1617
attributes: AztecAttributes = AztecAttributes(),
1718
onMediaDeletedListener: AztecText.OnMediaDeletedListener? = null,
1819
editor: AztecText? = null,
19-
private val adapter: PlaceholderManager.PlaceholderAdapter,
20+
private val adapter: WeakReference<PlaceholderManager.PlaceholderAdapter>,
2021
override val TAG: String) :
2122
AztecMediaSpan(context, drawable, attributes, onMediaDeletedListener, editor), IAztecFullWidthImageSpan, IAztecSpan {
2223
override fun onClick() {
2324

2425
}
2526

2627
override fun getMaxWidth(editorWidth: Int): Int {
27-
return runBlocking { adapter.calculateWidth(attributes, editorWidth) }
28+
return runBlocking { adapter.get()?.calculateWidth(attributes, editorWidth) ?: editorWidth }
2829
}
2930
}

media-placeholders/src/main/java/org/wordpress/aztec/placeholders/PlaceholderManager.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import org.wordpress.aztec.Html
2525
import org.wordpress.aztec.plugins.html2visual.IHtmlTagHandler
2626
import org.wordpress.aztec.spans.AztecMediaClickableSpan
2727
import org.xml.sax.Attributes
28+
import java.lang.ref.WeakReference
2829
import java.util.UUID
2930
import kotlin.coroutines.CoroutineContext
3031
import kotlin.math.min
@@ -90,7 +91,7 @@ class PlaceholderManager(
9091
val attrs = getAttributesForMedia(type, attributes)
9192
val drawable = buildPlaceholderDrawable(adapter, attrs)
9293
aztecText.insertMediaSpan(AztecPlaceholderSpan(aztecText.context, drawable, 0, attrs,
93-
this, aztecText, adapter, TAG = htmlTag))
94+
this, aztecText, WeakReference(adapter), TAG = htmlTag))
9495
insertContentOverSpanWithId(attrs.getValue(UUID_ATTRIBUTE))
9596
}
9697

@@ -260,7 +261,7 @@ class PlaceholderManager(
260261
nestingLevel = nestingLevel,
261262
attributes = aztecAttributes,
262263
onMediaDeletedListener = this,
263-
adapter = adapter,
264+
adapter = WeakReference(adapter),
264265
TAG = htmlTag
265266
)
266267
val clickableSpan = AztecMediaClickableSpan(span)

wordpress-comments/src/main/java/org/wordpress/aztec/plugins/wpcomments/spans/GutenbergInlineCommentSpan.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ import android.graphics.drawable.Drawable
55
import org.wordpress.aztec.AztecText
66
import org.wordpress.aztec.spans.AztecDynamicImageSpan
77
import org.wordpress.aztec.spans.IAztecFullWidthImageSpan
8+
import java.lang.ref.WeakReference
89

910
class GutenbergInlineCommentSpan @JvmOverloads constructor(val commentText: String, context: Context, drawable: Drawable, override var nestingLevel: Int, editor: AztecText? = null) :
1011
AztecDynamicImageSpan(context, drawable), IAztecFullWidthImageSpan {
1112

1213
init {
13-
textView = editor
14+
textView = WeakReference(editor)
1415
}
1516
}

wordpress-comments/src/main/java/org/wordpress/aztec/plugins/wpcomments/spans/WordPressCommentSpan.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ import android.graphics.drawable.Drawable
55
import org.wordpress.aztec.AztecText
66
import org.wordpress.aztec.spans.AztecDynamicImageSpan
77
import org.wordpress.aztec.spans.IAztecFullWidthImageSpan
8+
import java.lang.ref.WeakReference
89

910
class WordPressCommentSpan @JvmOverloads constructor(val commentText: String, context: Context, drawable: Drawable, override var nestingLevel: Int, editor: AztecText? = null) :
1011
AztecDynamicImageSpan(context, drawable), IAztecFullWidthImageSpan {
1112

1213
init {
13-
textView = editor
14+
textView = WeakReference(editor)
1415
}
1516

1617
companion object {

wordpress-shortcodes/src/main/java/org/wordpress/aztec/plugins/shortcodes/extensions/CaptionExtensions.kt

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,19 @@ fun AztecText.hasImageCaption(attributePredicate: AztecText.AttributePredicate):
5959
}
6060

6161
fun AztecImageSpan.getCaption(): String {
62-
textView?.text?.let {
63-
val wrapper = SpanWrapper(textView!!.text, this)
64-
textView!!.text.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.let {
62+
textView?.get()?.text?.let { editable ->
63+
val wrapper = SpanWrapper(editable, this)
64+
editable.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.let {
6565
return it.caption
6666
}
6767
}
6868
return ""
6969
}
7070

7171
fun AztecImageSpan.getCaptionAttributes(): AztecAttributes {
72-
textView?.text?.let {
73-
val wrapper = SpanWrapper(textView!!.text, this)
74-
textView!!.text.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.let {
72+
textView?.get()?.text?.let { editable ->
73+
val wrapper = SpanWrapper(editable, this)
74+
editable.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.let {
7575
return it.attributes
7676
}
7777
}
@@ -80,17 +80,17 @@ fun AztecImageSpan.getCaptionAttributes(): AztecAttributes {
8080

8181
@JvmOverloads
8282
fun AztecImageSpan.setCaption(value: String, attrs: AztecAttributes? = null) {
83-
textView?.text?.let {
84-
val wrapper = SpanWrapper(textView!!.text, this)
83+
textView?.get()?.text?.let { editable ->
84+
val wrapper = SpanWrapper(editable, this)
8585

86-
var captionSpan = textView?.text?.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java)?.firstOrNull()
86+
var captionSpan = editable.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java)?.firstOrNull()
8787
if (captionSpan == null) {
8888
captionSpan = createCaptionShortcodeSpan(
8989
AztecAttributes(),
9090
CaptionShortcodePlugin.HTML_TAG,
91-
IAztecNestable.getNestingLevelAt(textView!!.text, wrapper.start),
92-
textView!!)
93-
textView!!.text.setSpan(captionSpan, wrapper.start, wrapper.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
91+
IAztecNestable.getNestingLevelAt(editable, wrapper.start),
92+
textView?.get()!!)
93+
editable.setSpan(captionSpan, wrapper.start, wrapper.end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE)
9494
}
9595

9696
captionSpan.caption = value
@@ -114,8 +114,8 @@ fun AztecImageSpan.setCaption(value: String, attrs: AztecAttributes? = null) {
114114
}
115115

116116
fun AztecImageSpan.removeCaption() {
117-
textView?.text?.let {
118-
val wrapper = SpanWrapper(textView!!.text, this)
119-
textView!!.text.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.remove()
117+
textView?.get()?.text?.let { editable ->
118+
val wrapper = SpanWrapper(editable, this)
119+
editable.getSpans(wrapper.start, wrapper.end, CaptionShortcodeSpan::class.java).firstOrNull()?.remove()
120120
}
121121
}

0 commit comments

Comments
 (0)