Skip to content

Commit

Permalink
Be more conservative with handlers and references
Browse files Browse the repository at this point in the history
Expiring message timers could end up leaking references and
executing work even after their conversation item was no longer
visible

Maybe fixes signalapp#6898

// FREEBIE
  • Loading branch information
moxie0 committed Sep 10, 2017
1 parent 6a10c69 commit f3d9432
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 77 deletions.
14 changes: 5 additions & 9 deletions src/org/thoughtcrime/securesms/ConversationListItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import android.graphics.drawable.RippleDrawable;
import android.os.Build.VERSION;
import android.os.Build.VERSION_CODES;
import android.os.Handler;
import android.support.annotation.DrawableRes;
import android.support.annotation.NonNull;
import android.util.AttributeSet;
Expand All @@ -42,6 +41,7 @@
import org.thoughtcrime.securesms.recipients.RecipientModifiedListener;
import org.thoughtcrime.securesms.util.DateUtils;
import org.thoughtcrime.securesms.util.ResUtil;
import org.thoughtcrime.securesms.util.Util;
import org.thoughtcrime.securesms.util.ViewUtil;

import java.util.Locale;
Expand Down Expand Up @@ -83,7 +83,6 @@ public class ConversationListItem extends RelativeLayout
private final @DrawableRes int readBackground;
private final @DrawableRes int unreadBackround;

private final Handler handler = new Handler();
private int distributionType;

public ConversationListItem(Context context) {
Expand Down Expand Up @@ -236,13 +235,10 @@ private void setRippleColor(Recipient recipient) {

@Override
public void onModified(final Recipient recipient) {
handler.post(new Runnable() {
@Override
public void run() {
fromView.setText(recipient, read);
contactPhotoImage.setAvatar(recipient, true);
setRippleColor(recipient);
}
Util.runOnMain(() -> {
fromView.setText(recipient, read);
contactPhotoImage.setAvatar(recipient, true);
setRippleColor(recipient);
});
}

Expand Down
13 changes: 4 additions & 9 deletions src/org/thoughtcrime/securesms/MessageRecipientListItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import android.content.Context;
import android.os.AsyncTask;
import android.os.Handler;
import android.text.TextUtils;
import android.util.AttributeSet;
import android.view.View;
Expand All @@ -37,6 +36,7 @@
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientModifiedListener;
import org.thoughtcrime.securesms.sms.MessageSender;
import org.thoughtcrime.securesms.util.Util;

/**
* A simple view to show the recipients of a message
Expand All @@ -56,8 +56,6 @@ public class MessageRecipientListItem extends RelativeLayout
private Button resendButton;
private AvatarImageView contactPhotoImage;

private final Handler handler = new Handler();

public MessageRecipientListItem(Context context) {
super(context);
}
Expand Down Expand Up @@ -164,12 +162,9 @@ public void unbind() {

@Override
public void onModified(final Recipient recipient) {
handler.post(new Runnable() {
@Override
public void run() {
fromView.setText(recipient);
contactPhotoImage.setAvatar(recipient, false);
}
Util.runOnMain(() -> {
fromView.setText(recipient);
contactPhotoImage.setAvatar(recipient, false);
});
}

Expand Down
11 changes: 2 additions & 9 deletions src/org/thoughtcrime/securesms/RecipientPreferenceActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.thoughtcrime.securesms.util.DynamicNoActionBarTheme;
import org.thoughtcrime.securesms.util.DynamicTheme;
import org.thoughtcrime.securesms.util.IdentityUtil;
import org.thoughtcrime.securesms.util.Util;
import org.thoughtcrime.securesms.util.concurrent.ListenableFuture;
import org.whispersystems.libsignal.util.guava.Optional;

Expand Down Expand Up @@ -189,9 +190,6 @@ public static class RecipientPreferenceFragment
extends PreferenceFragment
implements RecipientModifiedListener
{

private final Handler handler = new Handler();

private Recipient recipient;
private BroadcastReceiver staleReceiver;
private MasterSecret masterSecret;
Expand Down Expand Up @@ -329,12 +327,7 @@ public void onFailure(ExecutionException e) {

@Override
public void onModified(final Recipient recipient) {
handler.post(new Runnable() {
@Override
public void run() {
setSummaries(recipient);
}
});
Util.runOnMain(() -> setSummaries(recipient));
}

private class RingtoneChangeListener implements Preference.OnPreferenceChangeListener {
Expand Down
11 changes: 4 additions & 7 deletions src/org/thoughtcrime/securesms/ShareListItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.thoughtcrime.securesms.database.model.ThreadRecord;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientModifiedListener;
import org.thoughtcrime.securesms.util.Util;

/**
* A simple view to show the recipients of an open conversation
Expand All @@ -45,7 +46,6 @@ public class ShareListItem extends RelativeLayout

private AvatarImageView contactPhotoImage;

private final Handler handler = new Handler();
private int distributionType;

public ShareListItem(Context context) {
Expand Down Expand Up @@ -104,12 +104,9 @@ public int getDistributionType() {

@Override
public void onModified(final Recipient recipient) {
handler.post(new Runnable() {
@Override
public void run() {
fromView.setText(recipient);
contactPhotoImage.setAvatar(recipient, false);
}
Util.runOnMain(() -> {
fromView.setText(recipient);
contactPhotoImage.setAvatar(recipient, false);
});
}
}
54 changes: 33 additions & 21 deletions src/org/thoughtcrime/securesms/components/ExpirationTimerView.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package org.thoughtcrime.securesms.components;

import android.content.Context;
import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.AttributeSet;

import org.thoughtcrime.securesms.util.Util;

import java.lang.ref.WeakReference;
import java.util.concurrent.TimeUnit;

public class ExpirationTimerView extends HourglassView {

private final Handler handler = new Handler();

private long startedAt;
private long expiresIn;

Expand Down Expand Up @@ -39,26 +40,11 @@ public void setExpirationTime(long startedAt, long expiresIn) {
public void startAnimation() {
synchronized (this) {
visible = true;
if (stopped == false) return;
else stopped = false;
if (!stopped) return;
else stopped = false;
}

handler.postDelayed(new Runnable() {
@Override
public void run() {
setPercentage(calculateProgress(startedAt, expiresIn));


synchronized (ExpirationTimerView.this) {
if (!visible) {
stopped = true;
return;
}
}

handler.postDelayed(this, calculateAnimationDelay(startedAt, expiresIn));
}
}, calculateAnimationDelay(this.startedAt, this.expiresIn));
Util.runOnMainDelayed(new AnimationUpdateRunnable(this), calculateAnimationDelay(this.startedAt, this.expiresIn));
}

public void stopAnimation() {
Expand All @@ -85,4 +71,30 @@ private long calculateAnimationDelay(long startedAt, long expiresIn) {
}
}

private static class AnimationUpdateRunnable implements Runnable {

private final WeakReference<ExpirationTimerView> expirationTimerViewReference;

private AnimationUpdateRunnable(@NonNull ExpirationTimerView expirationTimerView) {
this.expirationTimerViewReference = new WeakReference<>(expirationTimerView);
}

@Override
public void run() {
ExpirationTimerView timerView = expirationTimerViewReference.get();
if (timerView == null) return;

timerView.setExpirationTime(timerView.startedAt, timerView.expiresIn);

synchronized (timerView) {
if (!timerView.visible) {
timerView.stopped = true;
return;
}
}

Util.runOnMainDelayed(this, timerView.calculateAnimationDelay(timerView.startedAt, timerView.expiresIn));
}
}

}
9 changes: 3 additions & 6 deletions src/org/thoughtcrime/securesms/components/InputPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import android.content.Context;
import android.net.Uri;
import android.os.Build;
import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.view.ViewCompat;
Expand All @@ -25,11 +24,11 @@
import org.thoughtcrime.securesms.components.emoji.EmojiDrawer;
import org.thoughtcrime.securesms.components.emoji.EmojiToggle;
import org.thoughtcrime.securesms.util.TextSecurePreferences;
import org.thoughtcrime.securesms.util.Util;
import org.thoughtcrime.securesms.util.ViewUtil;
import org.thoughtcrime.securesms.util.concurrent.AssertedSuccessListener;
import org.thoughtcrime.securesms.util.concurrent.ListenableFuture;
import org.thoughtcrime.securesms.util.concurrent.SettableFuture;
import org.thoughtcrime.securesms.util.views.Stub;

import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
Expand Down Expand Up @@ -288,9 +287,7 @@ private float getOffset(float x) {
private static class RecordTime implements Runnable {

private final TextView recordTimeView;

private final AtomicLong startTime = new AtomicLong(0);
private final Handler handler = new Handler();

private RecordTime(TextView recordTimeView) {
this.recordTimeView = recordTimeView;
Expand All @@ -300,7 +297,7 @@ public void display() {
this.startTime.set(System.currentTimeMillis());
this.recordTimeView.setText(DateUtils.formatElapsedTime(0));
ViewUtil.fadeIn(this.recordTimeView, FADE_TIME);
handler.postDelayed(this, TimeUnit.SECONDS.toMillis(1));
Util.runOnMainDelayed(this, TimeUnit.SECONDS.toMillis(1));
}

public long hide() {
Expand All @@ -316,7 +313,7 @@ public void run() {
if (localStartTime > 0) {
long elapsedTime = System.currentTimeMillis() - localStartTime;
recordTimeView.setText(DateUtils.formatElapsedTime(TimeUnit.MILLISECONDS.toSeconds(elapsedTime)));
handler.postDelayed(this, TimeUnit.SECONDS.toMillis(1));
Util.runOnMainDelayed(this, TimeUnit.SECONDS.toMillis(1));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.thoughtcrime.securesms.components.webrtc;

import android.content.Context;
import android.os.Handler;
import android.os.Message;
import android.support.annotation.Nullable;
import android.util.AttributeSet;
import android.view.LayoutInflater;
Expand All @@ -11,8 +9,9 @@
import android.widget.RelativeLayout;
import android.widget.TextView;

import org.thoughtcrime.securesms.components.multiwaveview.MultiWaveView;
import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.components.multiwaveview.MultiWaveView;
import org.thoughtcrime.securesms.util.Util;

/**
* Displays the controls at the bottom of the in-call screen.
Expand All @@ -26,16 +25,6 @@ public class WebRtcIncomingCallOverlay extends RelativeLayout {
private MultiWaveView incomingCallWidget;
private TextView redphoneLabel;

private Handler handler = new Handler() {
@Override
public void handleMessage(Message message) {
if (incomingCallWidget.getVisibility() == View.VISIBLE) {
incomingCallWidget.ping();
handler.sendEmptyMessageDelayed(0, 1200);
}
}
};

public WebRtcIncomingCallOverlay(Context context) {
super(context);
initialize();
Expand Down Expand Up @@ -63,7 +52,15 @@ public void setIncomingCall() {
incomingCallWidget.setVisibility(View.VISIBLE);
redphoneLabel.setVisibility(View.VISIBLE);

handler.sendEmptyMessageDelayed(0, 500);
Util.runOnMainDelayed(new Runnable() {
@Override
public void run() {
if (incomingCallWidget.getVisibility() == View.VISIBLE) {
incomingCallWidget.ping();
Util.runOnMainDelayed(this, 1200);
}
}
}, 500);
}

public void setActiveCall() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ private enum CallState {
private boolean localVideoEnabled = false;
private boolean remoteVideoEnabled = false;
private boolean bluetoothAvailable = false;
private Handler serviceHandler = new Handler();

@Inject public SignalMessageSenderFactory messageSenderFactory;
@Inject public SignalServiceAccountManager accountManager;
Expand Down Expand Up @@ -611,7 +610,7 @@ private void handleBusyMessage(Intent intent) {
sendMessage(WebRtcViewModel.State.CALL_BUSY, recipient, localVideoEnabled, remoteVideoEnabled, bluetoothAvailable, microphoneEnabled);

audioManager.startOutgoingRinger(OutgoingRinger.Type.BUSY);
serviceHandler.postDelayed(new Runnable() {
Util.runOnMainDelayed(new Runnable() {
@Override
public void run() {
Intent intent = new Intent(WebRtcCallService.this, WebRtcCallService.class);
Expand Down
4 changes: 4 additions & 0 deletions src/org/thoughtcrime/securesms/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ public static void runOnMain(final @NonNull Runnable runnable) {
else handler.post(runnable);
}

public static void runOnMainDelayed(final @NonNull Runnable runnable, long delayMillis) {
handler.postDelayed(runnable, delayMillis);
}

public static void runOnMainSync(final @NonNull Runnable runnable) {
if (isMainThread()) {
runnable.run();
Expand Down

0 comments on commit f3d9432

Please sign in to comment.