From 4ad32fe2dc7b27adc7a811223c2dc2bdf9e50afb Mon Sep 17 00:00:00 2001 From: zachh Date: Thu, 13 Jul 2017 16:22:50 -0700 Subject: Use a shared thread pool for spam transport. Before, we were creating a custom thread pool for spam transport, which isn't necessary. Instead, we can just use an application wide thread pool. Also fixed some instances where we were not clearing the thread stats tag after opening the socket. Test: manually launched app with data cleared and observed no crash PiperOrigin-RevId: 161882076 Change-Id: I39bdd31cf5fa8a974d7535e861ec0716c85986f3 --- .../contacts/common/ContactPhotoManagerImpl.java | 27 ++++++++++++---------- .../concurrent/DefaultDialerExecutorFactory.java | 12 +--------- .../dialer/common/concurrent/DialerExecutors.java | 27 ++++++++++++++++++++++ .../android/voicemail/impl/mail/MailTransport.java | 2 ++ 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/java/com/android/contacts/common/ContactPhotoManagerImpl.java b/java/com/android/contacts/common/ContactPhotoManagerImpl.java index 28ecf3421..ea317953e 100644 --- a/java/com/android/contacts/common/ContactPhotoManagerImpl.java +++ b/java/com/android/contacts/common/ContactPhotoManagerImpl.java @@ -1220,20 +1220,23 @@ class ContactPhotoManagerImpl extends ContactPhotoManager implements Callback { InputStream is = null; if (scheme.equals("http") || scheme.equals("https")) { TrafficStats.setThreadStatsTag(TrafficStatsTags.CONTACT_PHOTO_DOWNLOAD_TAG); - final HttpURLConnection connection = - (HttpURLConnection) new URL(uri.toString()).openConnection(); - - // Include the user agent if it is specified. - if (!TextUtils.isEmpty(mUserAgent)) { - connection.setRequestProperty("User-Agent", mUserAgent); - } try { - is = connection.getInputStream(); - } catch (IOException e) { - connection.disconnect(); - is = null; + final HttpURLConnection connection = + (HttpURLConnection) new URL(uri.toString()).openConnection(); + + // Include the user agent if it is specified. + if (!TextUtils.isEmpty(mUserAgent)) { + connection.setRequestProperty("User-Agent", mUserAgent); + } + try { + is = connection.getInputStream(); + } catch (IOException e) { + connection.disconnect(); + is = null; + } + } finally { + TrafficStats.clearThreadStatsTag(); } - TrafficStats.clearThreadStatsTag(); } else { is = mResolver.openInputStream(uri); } diff --git a/java/com/android/dialer/common/concurrent/DefaultDialerExecutorFactory.java b/java/com/android/dialer/common/concurrent/DefaultDialerExecutorFactory.java index 82e517d9a..a87bbceb4 100644 --- a/java/com/android/dialer/common/concurrent/DefaultDialerExecutorFactory.java +++ b/java/com/android/dialer/common/concurrent/DefaultDialerExecutorFactory.java @@ -166,17 +166,7 @@ public class DefaultDialerExecutorFactory implements DialerExecutorFactory { }); private static final Executor defaultParallelExecutor = - Executors.newFixedThreadPool( - 5, - new ThreadFactory() { - @Override - public Thread newThread(Runnable runnable) { - LogUtil.i("NonUiTaskBuilder.newThread", "creating parallel thread"); - Thread thread = new Thread(runnable, "NonUiTaskBuilder-Parallel"); - thread.setPriority(4); // Corresponds to Process.THREAD_PRIORITY_BACKGROUND - return thread; - } - }); + DialerExecutors.getLowPriorityThreadPool(); NonUiTaskBuilder(Worker worker) { this(worker, defaultSerialExecutorService, defaultParallelExecutor); diff --git a/java/com/android/dialer/common/concurrent/DialerExecutors.java b/java/com/android/dialer/common/concurrent/DialerExecutors.java index 148d8660c..81b3c5cb3 100644 --- a/java/com/android/dialer/common/concurrent/DialerExecutors.java +++ b/java/com/android/dialer/common/concurrent/DialerExecutors.java @@ -19,7 +19,11 @@ package com.android.dialer.common.concurrent; import android.app.FragmentManager; import android.support.annotation.NonNull; import com.android.dialer.common.Assert; +import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.DialerExecutor.Worker; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; /** * Factory methods for creating {@link DialerExecutor} objects for doing background work. @@ -131,4 +135,27 @@ public final class DialerExecutors { @NonNull Worker worker) { return new DefaultDialerExecutorFactory().createNonUiTaskBuilder(Assert.isNotNull(worker)); } + + private static final Executor lowPriorityThreadPool = + Executors.newFixedThreadPool( + 5, + new ThreadFactory() { + @Override + public Thread newThread(Runnable runnable) { + LogUtil.i("DialerExecutors.newThread", "creating low priority thread"); + Thread thread = new Thread(runnable, "DialerExecutors-LowPriority"); + thread.setPriority(4); // Corresponds to Process.THREAD_PRIORITY_BACKGROUND + return thread; + } + }); + + /** + * An application-wide thread pool used for low priority (non-UI) tasks. + * + *

This exists to prevent each individual dialer component from having to create its own + * threads/pools, which would result in the application having more threads than really necessary. + */ + public static Executor getLowPriorityThreadPool() { + return lowPriorityThreadPool; + } } diff --git a/java/com/android/voicemail/impl/mail/MailTransport.java b/java/com/android/voicemail/impl/mail/MailTransport.java index 00339f03d..c35e41450 100644 --- a/java/com/android/voicemail/impl/mail/MailTransport.java +++ b/java/com/android/voicemail/impl/mail/MailTransport.java @@ -195,6 +195,8 @@ public class MailTransport { } catch (IOException ioe) { LogUtils.d(TAG, ioe.toString()); throw new MessagingException(MessagingException.IOERROR, ioe.toString()); + } finally { + TrafficStats.clearThreadStatsTag(); } } -- cgit v1.2.3