New issue
Advanced search Search tips

Issue 869907 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 27
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

AsyncTask.execute() happens on SERIAL_EXECUTOR

Project Member Reported by smaier@chromium.org, Aug 1

Issue description

We should either make execute() default to THREAD_POOL_EXECUTOR, or remove it entirely and update the API with a new default.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74bf0b9652ccf8db5d7ec8cc07810d0ec606e1ea

commit 74bf0b9652ccf8db5d7ec8cc07810d0ec606e1ea
Author: Sam Maier <smaier@chromium.org>
Date: Wed Aug 01 18:12:59 2018

Android: moving Webview seed fetch to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I7dce5b4f28520a3d2a81491756f5aec5f6806fa9
Reviewed-on: https://chromium-review.googlesource.com/1158962
Reviewed-by: Paul Miller <paulmiller@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579862}
[modify] https://crrev.com/74bf0b9652ccf8db5d7ec8cc07810d0ec606e1ea/android_webview/java/src/org/chromium/android_webview/services/AwVariationsSeedFetcher.java

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc24975d6857d775dffe526c9c52ebd76a2e88f7

commit dc24975d6857d775dffe526c9c52ebd76a2e88f7
Author: Sam Maier <smaier@chromium.org>
Date: Wed Aug 01 19:06:53 2018

Android: moving download AsyncTasks to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like these callsites can use the THREAD_POOL_EXECUTOR instead,
since these uses don't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I0a2c9021be07447d9437c7e33606307ea3685fc5
Reviewed-on: https://chromium-review.googlesource.com/1159105
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579883}
[modify] https://crrev.com/dc24975d6857d775dffe526c9c52ebd76a2e88f7/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[modify] https://crrev.com/dc24975d6857d775dffe526c9c52ebd76a2e88f7/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java
[modify] https://crrev.com/dc24975d6857d775dffe526c9c52ebd76a2e88f7/chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27de0cdd441e5b9219dc0c957651ea192b225141

commit 27de0cdd441e5b9219dc0c957651ea192b225141
Author: Sam Maier <smaier@chromium.org>
Date: Wed Aug 01 20:10:30 2018

Android: moving download onclick execution to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Iad83ebccb137eec01ff32bc1f78d72047859efc8
Reviewed-on: https://chromium-review.googlesource.com/1158789
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579915}
[modify] https://crrev.com/27de0cdd441e5b9219dc0c957651ea192b225141/chrome/android/java/src/org/chromium/chrome/browser/infobar/DuplicateDownloadInfoBar.java

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f711c30ff35b20079c7b74b31e259b9bb7396a21

commit f711c30ff35b20079c7b74b31e259b9bb7396a21
Author: Sam Maier <smaier@chromium.org>
Date: Wed Aug 01 20:10:44 2018

Android: moving CalendarBuilder execution to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I2f5b412edaf9e6f9ec2e634af757f3b238987afe
Reviewed-on: https://chromium-review.googlesource.com/1159112
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579916}
[modify] https://crrev.com/f711c30ff35b20079c7b74b31e259b9bb7396a21/chrome/android/java/src/org/chromium/chrome/browser/download/home/list/CalendarFactory.java

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb312b588513b2d42c4f44c411537771df2f336c

commit bb312b588513b2d42c4f44c411537771df2f336c
Author: Sam Maier <smaier@chromium.org>
Date: Wed Aug 01 20:56:14 2018

Android: moving CardUnmaskPrompt Calendar creation to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Ied05df42799c3b643f61d2db6abaec65d53b110c
Reviewed-on: https://chromium-review.googlesource.com/1158992
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579933}
[modify] https://crrev.com/bb312b588513b2d42c4f44c411537771df2f336c/chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aac4a16570e97660456a60877ff71776c3106d7e

commit aac4a16570e97660456a60877ff71776c3106d7e
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 02 14:30:17 2018

Android: moving SSL cert request to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Ia90e016f9948ad87d577ddb8ac24dd27450f36e4
Reviewed-on: https://chromium-review.googlesource.com/1158988
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580167}
[modify] https://crrev.com/aac4a16570e97660456a60877ff71776c3106d7e/chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec88843a197b60125c1b2c88a3c11d623f3e665f

commit ec88843a197b60125c1b2c88a3c11d623f3e665f
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 02 14:31:21 2018

Android: moving update check to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I09f794fe73c706019ce42f16b505d9a1ee1757ea
Reviewed-on: https://chromium-review.googlesource.com/1159268
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580169}
[modify] https://crrev.com/ec88843a197b60125c1b2c88a3c11d623f3e665f/chrome/android/java/src/org/chromium/chrome/browser/omaha/UpdateMenuItemHelper.java

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e7c56fc13f2920683971fb4986be5959b129ba0

commit 7e7c56fc13f2920683971fb4986be5959b129ba0
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 02 14:32:34 2018

Android: moving download notify invalidations to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I5280674dd8afefbbfbcadbf0b41acf2aaba0b8a1
Reviewed-on: https://chromium-review.googlesource.com/1159207
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580172}
[modify] https://crrev.com/7e7c56fc13f2920683971fb4986be5959b129ba0/chrome/android/java/src/org/chromium/chrome/browser/invalidation/DelayedInvalidationsController.java

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61eabef37504664a2bd70e0a80a21cbfaff6e019

commit 61eabef37504664a2bd70e0a80a21cbfaff6e019
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 03 15:42:07 2018

Android: make loadNextTab AsyncTask explicit on SERIAL_EXECUTOR

We might be changing the default execute(), so we want to make this an
explicit call to the SERIAL_EXECUTOR.

Bug:  869907 
Change-Id: Ifd7cff98b7714b664fdc6cb30bc0d0f7b1a3be4a
Reviewed-on: https://chromium-review.googlesource.com/1162076
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580559}
[modify] https://crrev.com/61eabef37504664a2bd70e0a80a21cbfaff6e019/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9f8f70b78dd0569811edbeb2f5e367f593d9e23

commit c9f8f70b78dd0569811edbeb2f5e367f593d9e23
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 03 15:45:21 2018

Android: moving CardEditor calendar creation to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Ib95ed2c1c4442f5ed8d2e7f74594c7a57aee1c3d
Reviewed-on: https://chromium-review.googlesource.com/1161087
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580560}
[modify] https://crrev.com/c9f8f70b78dd0569811edbeb2f5e367f593d9e23/chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/443da37914571675b257a483e9366a6da8b1b6e2

commit 443da37914571675b257a483e9366a6da8b1b6e2
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 03 20:17:37 2018

Android: chromoting OAuthToken AsyncTasks explicitly on SERIAL_EXECUTOR

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

Looking at this code, I don't think we can escape the SERIAL_EXECUTOR
here, since there appears to be non-thread-safe things happening in the
AsyncTasks, and no guarantee they won't get scheduled at the same time.

Bug:  869907 
Change-Id: I7c1e1a74e48c2d2e774d71e04d89a83c02562e5b
Reviewed-on: https://chromium-review.googlesource.com/1162662
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580636}
[modify] https://crrev.com/443da37914571675b257a483e9366a6da8b1b6e2/remoting/android/java/src/org/chromium/chromoting/OAuthTokenConsumer.java
[modify] https://crrev.com/443da37914571675b257a483e9366a6da8b1b6e2/remoting/android/java/src/org/chromium/chromoting/base/OAuthTokenFetcher.java

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b17eddd6a259f88b58ab3c73c70e4ea65c03c2e8

commit b17eddd6a259f88b58ab3c73c70e4ea65c03c2e8
Author: Sam Maier <smaier@chromium.org>
Date: Sun Aug 05 19:24:22 2018

Android: Webapp register and get splash screen AsyncTasks to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like these callsites can use the THREAD_POOL_EXECUTOR instead,
since these usages don't appear to rely on the concurrency guarantees
that SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Iaf227dc516d1b2c850b9b17ce5d43302ab6532b2
Reviewed-on: https://chromium-review.googlesource.com/1161968
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580780}
[modify] https://crrev.com/b17eddd6a259f88b58ab3c73c70e4ea65c03c2e8/base/test/android/junit/src/org/chromium/base/test/asynctask/BackgroundShadowAsyncTask.java
[modify] https://crrev.com/b17eddd6a259f88b58ab3c73c70e4ea65c03c2e8/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDataStorage.java
[modify] https://crrev.com/b17eddd6a259f88b58ab3c73c70e4ea65c03c2e8/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f32eb00d920ef6802155fa680e75ba3a0e6099a

commit 2f32eb00d920ef6802155fa680e75ba3a0e6099a
Author: Sam Maier <smaier@chromium.org>
Date: Tue Aug 07 14:24:49 2018

Android: GCMDriver AsyncTasks moved to THREAD_POOL_EXECUTOR

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like these callsites can use the THREAD_POOL_EXECUTOR instead,
since these usages don't appear to rely on the concurrency guarantees
that SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: If2c63496e7851db697e313429baa10508aeb4753
Reviewed-on: https://chromium-review.googlesource.com/1162691
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581218}
[modify] https://crrev.com/2f32eb00d920ef6802155fa680e75ba3a0e6099a/components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b78848e30f5d883b4a17cdc236526a649dd9a708

commit b78848e30f5d883b4a17cdc236526a649dd9a708
Author: Sam Maier <smaier@chromium.org>
Date: Tue Aug 07 18:11:31 2018

Android: AccountsChangedReceiver and SigninHelper explicitly using SERIAL_EXECUTOR

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like these callsites are stuck with the SERIAL_EXECUTOR, since
they both call updateAccountRenameData() which is not thread safe.

Bug:  869907 
Change-Id: I23ac4430cd10acb61842ee1b37c36d8c36f0f8af
Reviewed-on: https://chromium-review.googlesource.com/1161092
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581283}
[modify] https://crrev.com/b78848e30f5d883b4a17cdc236526a649dd9a708/chrome/android/java/src/org/chromium/chrome/browser/services/AccountsChangedReceiver.java
[modify] https://crrev.com/b78848e30f5d883b4a17cdc236526a649dd9a708/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninHelper.java

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61985bcc09f291be58e05b3632375c8a3aa66c73

commit 61985bcc09f291be58e05b3632375c8a3aa66c73
Author: Sam Maier <smaier@chromium.org>
Date: Tue Aug 07 19:43:30 2018

Android: SmartSelectionRequests explicitly on SERIAL_EXECUTOR

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite is stuck on the SERIAL_EXECUTOR. We make
this requirement explicit.

Bug:  869907 
Change-Id: I651a27380b514600793a248babd41aa556983fb1
Reviewed-on: https://chromium-review.googlesource.com/1162714
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581307}
[modify] https://crrev.com/61985bcc09f291be58e05b3632375c8a3aa66c73/content/public/android/java/src/org/chromium/content/browser/selection/SmartSelectionProvider.java

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e819edb813b0d9fec26cb7687b5243fadc6babc6

commit e819edb813b0d9fec26cb7687b5243fadc6babc6
Author: Sam Maier <smaier@chromium.org>
Date: Tue Aug 07 19:45:25 2018

Android: making ShareHelper usage of AsyncTask explicit

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

We are using SERIAL_EXECUTOR in clearSharedImages and shareImage since
they don't appear to be thread safe, and they could potentially be
called multiple times.

We are using the UI thread in getShareableIconAndName since we
immediately call get() on it anyway. This negates any benefit for having
an AsyncTask.

Bug:  869907 , 729737
Change-Id: I9ea44efba2155bb2b643f6d4d8f2b38c8365f418
Reviewed-on: https://chromium-review.googlesource.com/1161218
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581308}
[modify] https://crrev.com/e819edb813b0d9fec26cb7687b5243fadc6babc6/chrome/android/java/src/org/chromium/chrome/browser/share/ShareHelper.java

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0

commit d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 09 14:25:03 2018

Android: moving MediaUrlResolver tasks to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

TBR= test file
Bug:  869907 
Change-Id: I5770f3f0211d2c10b28f2290973bbfd57bee53f0
Reviewed-on: https://chromium-review.googlesource.com/1159236
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>

[modify] https://crrev.com/d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0/chrome/android/java/src/org/chromium/chrome/browser/media/remote/DefaultMediaRouteController.java
[modify] https://crrev.com/d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0/chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5eb802f9e1b7c765f77ec4a61f7f448731c36811

commit 5eb802f9e1b7c765f77ec4a61f7f448731c36811
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 09 14:30:59 2018

Android: moving FileEnumWorkerTask to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I266476c44f16ade391dfef0e5a99d33494312598
Reviewed-on: https://chromium-review.googlesource.com/1161089
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>

[modify] https://crrev.com/5eb802f9e1b7c765f77ec4a61f7f448731c36811/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2af33f66d4079e4306fb698828d6d9d6d4fbe95

commit c2af33f66d4079e4306fb698828d6d9d6d4fbe95
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Aug 09 19:19:06 2018

Revert "Android: moving FileEnumWorkerTask to thread pool"

This reverts commit 5eb802f9e1b7c765f77ec4a61f7f448731c36811.

Reason for revert: Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.

Original change's description:
> Android: moving FileEnumWorkerTask to thread pool
> 
> Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> exector is good for preventing concurrency errors since it guarantees
> serial execution, but bad for performance since the entire app shares
> this single queue.
> 
> It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> since this use doesn't appear to rely on the concurrency guarantees that
> SERIAL_EXECUTOR provides.
> 
> Bug:  869907 
> Change-Id: I266476c44f16ade391dfef0e5a99d33494312598
> Reviewed-on: https://chromium-review.googlesource.com/1161089
> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>

TBR=finnur@chromium.org,smaier@chromium.org

Change-Id: I890bfff82b7471f54948c03dea7d2820224e978f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  869907 
Reviewed-on: https://chromium-review.googlesource.com/1169851
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/c2af33f66d4079e4306fb698828d6d9d6d4fbe95/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/108f9e6cb07bdf3327b8b62e3ad43703b854662d

commit 108f9e6cb07bdf3327b8b62e3ad43703b854662d
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Aug 09 19:25:26 2018

Revert "Android: moving MediaUrlResolver tasks to thread pool"

This reverts commit d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0.

Reason for revert:Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.

Original change's description:
> Android: moving MediaUrlResolver tasks to thread pool
> 
> Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> exector is good for preventing concurrency errors since it guarantees
> serial execution, but bad for performance since the entire app shares
> this single queue.
> 
> It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> since this use doesn't appear to rely on the concurrency guarantees that
> SERIAL_EXECUTOR provides.
> 
> TBR= test file
> Bug:  869907 
> Change-Id: I5770f3f0211d2c10b28f2290973bbfd57bee53f0
> Reviewed-on: https://chromium-review.googlesource.com/1159236
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>

TBR=mlamouri@chromium.org,smaier@chromium.org

Change-Id: I5cd823a033bb6443007734d4faf0e1ddf98de648
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  869907 
Reviewed-on: https://chromium-review.googlesource.com/1169854
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/108f9e6cb07bdf3327b8b62e3ad43703b854662d/chrome/android/java/src/org/chromium/chrome/browser/media/remote/DefaultMediaRouteController.java
[modify] https://crrev.com/108f9e6cb07bdf3327b8b62e3ad43703b854662d/chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87fa0656b765934b0cfbe762fca2e326d4fe4ca4

commit 87fa0656b765934b0cfbe762fca2e326d4fe4ca4
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 10 18:13:28 2018

Android: MockSyncContentResolverDelegate explicit executeOnExecutor

We would like to get rid of all usages of execute() since it is unclear
whether it goes to SERIAL_EXECUTOR or THREAD_POOL_EXECUTOR (many assume
THREAD_POOL_EXECUTOR, but it actually goes to SERIAL_EXECUTOR). This is
just cleaning up this test usage.

Bug:  869907 
Change-Id: I7564a0cc743f803b07aa04651337c857b7eb6d87
Reviewed-on: https://chromium-review.googlesource.com/1162704
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582249}
[modify] https://crrev.com/87fa0656b765934b0cfbe762fca2e326d4fe4ca4/components/sync/test/android/javatests/src/org/chromium/components/sync/test/util/MockSyncContentResolverDelegate.java

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1b5ee020fc4770291e328a91652f948f6b53206

commit e1b5ee020fc4770291e328a91652f948f6b53206
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 10 21:56:46 2018

Reland "Android: moving FileEnumWorkerTask to thread pool"

This reverts commit c2af33f66d4079e4306fb698828d6d9d6d4fbe95.

Reason for revert: Just re landing now that gerrit is OK

Original change's description:
> Revert "Android: moving FileEnumWorkerTask to thread pool"
>
> This reverts commit 5eb802f9e1b7c765f77ec4a61f7f448731c36811.
>
> Reason for revert: Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.
>
> Original change's description:
> > Android: moving FileEnumWorkerTask to thread pool
> >
> > Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> > exector is good for preventing concurrency errors since it guarantees
> > serial execution, but bad for performance since the entire app shares
> > this single queue.
> >
> > It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> > since this use doesn't appear to rely on the concurrency guarantees that
> > SERIAL_EXECUTOR provides.
> >
> > Bug:  869907 
> > Change-Id: I266476c44f16ade391dfef0e5a99d33494312598
> > Reviewed-on: https://chromium-review.googlesource.com/1161089
> > Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
> > Commit-Queue: Sam Maier <smaier@chromium.org>
>
> TBR=finnur@chromium.org,smaier@chromium.org
>
> Change-Id: I890bfff82b7471f54948c03dea7d2820224e978f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  869907 
> Reviewed-on: https://chromium-review.googlesource.com/1169851
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

TBR=finnur@chromium.org,tandrii@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.
TBR=Gerrit reland

Bug:  869907 
Change-Id: I4e07a7d43ae7f0af01a248f4c41d42699d041f30
Reviewed-on: https://chromium-review.googlesource.com/1171427
Reviewed-by: Sam Maier <smaier@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582352}
[modify] https://crrev.com/e1b5ee020fc4770291e328a91652f948f6b53206/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/PickerCategoryView.java

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9102edc99321bd860523f03cfd63aa153738c9a

commit e9102edc99321bd860523f03cfd63aa153738c9a
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 10 22:24:21 2018

Reland "Android: moving MediaUrlResolver tasks to thread pool"

This reverts commit 108f9e6cb07bdf3327b8b62e3ad43703b854662d.

Reason for revert: Just re landing now that gerrit is OK

Original change's description:
> Revert "Android: moving MediaUrlResolver tasks to thread pool"
> 
> This reverts commit d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0.
> 
> Reason for revert:Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.
> 
> Original change's description:
> > Android: moving MediaUrlResolver tasks to thread pool
> > 
> > Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> > exector is good for preventing concurrency errors since it guarantees
> > serial execution, but bad for performance since the entire app shares
> > this single queue.
> > 
> > It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> > since this use doesn't appear to rely on the concurrency guarantees that
> > SERIAL_EXECUTOR provides.
> > 
> > TBR= test file
> > Bug:  869907 
> > Change-Id: I5770f3f0211d2c10b28f2290973bbfd57bee53f0
> > Reviewed-on: https://chromium-review.googlesource.com/1159236
> > Commit-Queue: Sam Maier <smaier@chromium.org>
> > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> 
> TBR=mlamouri@chromium.org,smaier@chromium.org
> 
> Change-Id: I5cd823a033bb6443007734d4faf0e1ddf98de648
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  869907 
> Reviewed-on: https://chromium-review.googlesource.com/1169854
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

TBR=mlamouri@chromium.org,tandrii@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  869907 
Change-Id: I31b02f5b543f2fdb8b0b089f241d79ceb720a626
Reviewed-on: https://chromium-review.googlesource.com/1171702
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582358}
[modify] https://crrev.com/e9102edc99321bd860523f03cfd63aa153738c9a/chrome/android/java/src/org/chromium/chrome/browser/media/remote/DefaultMediaRouteController.java
[modify] https://crrev.com/e9102edc99321bd860523f03cfd63aa153738c9a/chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb4b11daff5beed6a3fda334a13cd2a1b51dfca6

commit bb4b11daff5beed6a3fda334a13cd2a1b51dfca6
Author: Sam Maier <smaier@chromium.org>
Date: Mon Aug 13 22:39:27 2018

Android: moving ReadBookmarksTask to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: Ied2b63aaf8256033ada85c7d3367db0c4082b454
Reviewed-on: https://chromium-review.googlesource.com/1161084
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582723}
[modify] https://crrev.com/bb4b11daff5beed6a3fda334a13cd2a1b51dfca6/chrome/android/java/src/org/chromium/chrome/browser/partnerbookmarks/PartnerBookmarksReader.java

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dcd8cc71f77c79cc3ffa059497ef43776bcc020

commit 4dcd8cc71f77c79cc3ffa059497ef43776bcc020
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 16 13:21:51 2018

Changing cast AsyncTask.execute usages to thread pool

Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
exector is good for preventing concurrency errors since it guarantees
serial execution, but bad for performance since the entire app shares
this single queue.

We are hoping to get rid of execute() entirely, since it is often
assumed to be a parallel operation.

It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
since this use doesn't appear to rely on the concurrency guarantees that
SERIAL_EXECUTOR provides.

Bug:  869907 
Change-Id: I51f22e371ec1e0d0e5114f4b211c934e214f5784
Reviewed-on: https://chromium-review.googlesource.com/1174900
Reviewed-by: Simeon Anfinrud <sanfin@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583624}
[modify] https://crrev.com/4dcd8cc71f77c79cc3ffa059497ef43776bcc020/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/AsyncTaskRunner.java
[modify] https://crrev.com/4dcd8cc71f77c79cc3ffa059497ef43776bcc020/chromecast/browser/android/apk/src/org/chromium/chromecast/shell/ExternalServiceDeviceLogcatProvider.java
[modify] https://crrev.com/4dcd8cc71f77c79cc3ffa059497ef43776bcc020/chromecast/browser/android/junit/src/org/chromium/chromecast/shell/AsyncTaskRunnerTest.java

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be62a5bad9df35e97eab3fcdba50990c549e2304

commit be62a5bad9df35e97eab3fcdba50990c549e2304
Author: Sam Maier <smaier@chromium.org>
Date: Fri Aug 17 21:16:18 2018

Android: removing default execute() on AsyncTask

execute() is a confusing function, with a hidden implicit meaning that
changed in API 11. It would be better if everyone had to use the more
explicit executeOnExecutor() so their preference (serial or thread pool)
is called out.

Bug:  869907 
Change-Id: I79be6d2b3114cb0520df1104ec5849f7c070c99e
Reviewed-on: https://chromium-review.googlesource.com/1172446
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584189}
[modify] https://crrev.com/be62a5bad9df35e97eab3fcdba50990c549e2304/base/test/android/junit/src/org/chromium/base/test/asynctask/BackgroundShadowAsyncTask.java
[modify] https://crrev.com/be62a5bad9df35e97eab3fcdba50990c549e2304/base/test/android/junit/src/org/chromium/base/test/asynctask/CustomShadowAsyncTask.java
[modify] https://crrev.com/be62a5bad9df35e97eab3fcdba50990c549e2304/third_party/android_async_task/java/src/org/chromium/base/AsyncTask.java
[modify] https://crrev.com/be62a5bad9df35e97eab3fcdba50990c549e2304/third_party/robolectric/custom_asynctask/java/src/org/chromium/base/test/asynctask/ShadowAsyncTask.java

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da8490d633ae9609dc32fa9afdc1a5a83ce142e2

commit da8490d633ae9609dc32fa9afdc1a5a83ce142e2
Author: Matthew Jones <mdjones@chromium.org>
Date: Fri Aug 17 22:03:15 2018

Revert "Android: removing default execute() on AsyncTask"

This reverts commit be62a5bad9df35e97eab3fcdba50990c549e2304.

Reason for revert: Causes compile failure on many bots:

 ../../components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:625: error: cannot find symbol
        new BenchmarkTask().execute();

https://ci.chromium.org/buildbot/chromium.android/Android%20Cronet%20Marshmallow%2064bit%20Perf/24049

Original change's description:
> Android: removing default execute() on AsyncTask
> 
> execute() is a confusing function, with a hidden implicit meaning that
> changed in API 11. It would be better if everyone had to use the more
> explicit executeOnExecutor() so their preference (serial or thread pool)
> is called out.
> 
> Bug:  869907 
> Change-Id: I79be6d2b3114cb0520df1104ec5849f7c070c99e
> Reviewed-on: https://chromium-review.googlesource.com/1172446
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584189}

TBR=agrieve@chromium.org,smaier@chromium.org

Change-Id: I9184afddb3028649c88f9fdf80ea46c4f7cc7806
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  869907 
Reviewed-on: https://chromium-review.googlesource.com/1179352
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584211}
[modify] https://crrev.com/da8490d633ae9609dc32fa9afdc1a5a83ce142e2/base/test/android/junit/src/org/chromium/base/test/asynctask/BackgroundShadowAsyncTask.java
[modify] https://crrev.com/da8490d633ae9609dc32fa9afdc1a5a83ce142e2/base/test/android/junit/src/org/chromium/base/test/asynctask/CustomShadowAsyncTask.java
[modify] https://crrev.com/da8490d633ae9609dc32fa9afdc1a5a83ce142e2/third_party/android_async_task/java/src/org/chromium/base/AsyncTask.java
[modify] https://crrev.com/da8490d633ae9609dc32fa9afdc1a5a83ce142e2/third_party/robolectric/custom_asynctask/java/src/org/chromium/base/test/asynctask/ShadowAsyncTask.java

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d07131525cf091e5d6278cb2cbba98f6470c022

commit 4d07131525cf091e5d6278cb2cbba98f6470c022
Author: Sam Maier <smaier@chromium.org>
Date: Mon Aug 20 17:38:30 2018

Android: cronet perf tests explicitly run on SERIAL_EXECUTOR

We would like to get rid of all usages of execute() since it is unclear
whether it goes to SERIAL_EXECUTOR or THREAD_POOL_EXECUTOR (many assume
THREAD_POOL_EXECUTOR, but it actually goes to SERIAL_EXECUTOR). This is
just cleaning up this test usage.

Bug:  869907 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Id783526b0daead7f413f55bb443e9d9792142667
Reviewed-on: https://chromium-review.googlesource.com/1181540
Commit-Queue: Sam Maier <smaier@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584487}
[modify] https://crrev.com/4d07131525cf091e5d6278cb2cbba98f6470c022/components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81262eaba2b7dcf252516cfbcccea750952b3c95

commit 81262eaba2b7dcf252516cfbcccea750952b3c95
Author: Sam Maier <smaier@chromium.org>
Date: Thu Aug 23 16:16:29 2018

Reland "Android: removing default execute() on AsyncTask"

This is a reland of be62a5bad9df35e97eab3fcdba50990c549e2304

Original change's description:
> Android: removing default execute() on AsyncTask
>
> execute() is a confusing function, with a hidden implicit meaning that
> changed in API 11. It would be better if everyone had to use the more
> explicit executeOnExecutor() so their preference (serial or thread pool)
> is called out.
>
> Bug:  869907 
> Change-Id: I79be6d2b3114cb0520df1104ec5849f7c070c99e
> Reviewed-on: https://chromium-review.googlesource.com/1172446
> Reviewed-by: agrieve <agrieve@chromium.org>
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584189}
TBR=smaier@ reland now that fix is in https://chromium-review.googlesource.com/c/chromium/src/+/1172446

Bug:  869907 
Change-Id: I90613192c8655ee08dba939785c79e4aa254c08c
Reviewed-on: https://chromium-review.googlesource.com/1181803
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585506}
[modify] https://crrev.com/81262eaba2b7dcf252516cfbcccea750952b3c95/base/test/android/junit/src/org/chromium/base/test/asynctask/BackgroundShadowAsyncTask.java
[modify] https://crrev.com/81262eaba2b7dcf252516cfbcccea750952b3c95/base/test/android/junit/src/org/chromium/base/test/asynctask/CustomShadowAsyncTask.java
[modify] https://crrev.com/81262eaba2b7dcf252516cfbcccea750952b3c95/third_party/android_async_task/java/src/org/chromium/base/AsyncTask.java
[modify] https://crrev.com/81262eaba2b7dcf252516cfbcccea750952b3c95/third_party/robolectric/custom_asynctask/java/src/org/chromium/base/test/asynctask/ShadowAsyncTask.java

Status: Fixed (was: Started)
Project Member

Comment 31 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/272eecdb1623c84e0d56e4ab1ac5eacf9bb7939f

commit 272eecdb1623c84e0d56e4ab1ac5eacf9bb7939f
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Tue Sep 04 23:51:44 2018

Revert "Reland "Android: moving MediaUrlResolver tasks to thread pool""

This reverts commit e9102edc99321bd860523f03cfd63aa153738c9a.

Reason for revert: This code is going away in M71, but is causing crashes in M70. It is not worth implementing a non-trivial fix just for M70. See 873941

Original change's description:
> Reland "Android: moving MediaUrlResolver tasks to thread pool"
> 
> This reverts commit 108f9e6cb07bdf3327b8b62e3ad43703b854662d.
> 
> Reason for revert: Just re landing now that gerrit is OK
> 
> Original change's description:
> > Revert "Android: moving MediaUrlResolver tasks to thread pool"
> > 
> > This reverts commit d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0.
> > 
> > Reason for revert:Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.
> > 
> > Original change's description:
> > > Android: moving MediaUrlResolver tasks to thread pool
> > > 
> > > Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> > > exector is good for preventing concurrency errors since it guarantees
> > > serial execution, but bad for performance since the entire app shares
> > > this single queue.
> > > 
> > > It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> > > since this use doesn't appear to rely on the concurrency guarantees that
> > > SERIAL_EXECUTOR provides.
> > > 
> > > TBR= test file
> > > Bug:  869907 
> > > Change-Id: I5770f3f0211d2c10b28f2290973bbfd57bee53f0
> > > Reviewed-on: https://chromium-review.googlesource.com/1159236
> > > Commit-Queue: Sam Maier <smaier@chromium.org>
> > > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> > 
> > TBR=mlamouri@chromium.org,smaier@chromium.org
> > 
> > Change-Id: I5cd823a033bb6443007734d4faf0e1ddf98de648
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug:  869907 
> > Reviewed-on: https://chromium-review.googlesource.com/1169854
> > Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> 
> TBR=mlamouri@chromium.org,tandrii@chromium.org,smaier@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  869907 
> Change-Id: I31b02f5b543f2fdb8b0b089f241d79ceb720a626
> Reviewed-on: https://chromium-review.googlesource.com/1171702
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582358}

TBR=mlamouri@chromium.org,tandrii@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  869907 , 873941
Change-Id: I44847161ac09f49b81cb8186880830c636cfdb33
Reviewed-on: https://chromium-review.googlesource.com/1205450
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588699}
[modify] https://crrev.com/272eecdb1623c84e0d56e4ab1ac5eacf9bb7939f/chrome/android/java/src/org/chromium/chrome/browser/media/remote/DefaultMediaRouteController.java
[modify] https://crrev.com/272eecdb1623c84e0d56e4ab1ac5eacf9bb7939f/chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java

Project Member

Comment 32 by bugdroid1@chromium.org, Sep 10

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa50cabe65fed56697e8be736f046c5fec5f4464

commit aa50cabe65fed56697e8be736f046c5fec5f4464
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Mon Sep 10 18:45:42 2018

Revert "Reland "Android: moving MediaUrlResolver tasks to thread pool""

This reverts commit e9102edc99321bd860523f03cfd63aa153738c9a.

Reason for revert: This code is going away in M71, but is causing crashes in M70. It is not worth implementing a non-trivial fix just for M70. See 873941

Original change's description:
> Reland "Android: moving MediaUrlResolver tasks to thread pool"
> 
> This reverts commit 108f9e6cb07bdf3327b8b62e3ad43703b854662d.
> 
> Reason for revert: Just re landing now that gerrit is OK
> 
> Original change's description:
> > Revert "Android: moving MediaUrlResolver tasks to thread pool"
> > 
> > This reverts commit d20f6cb34484fbd5e7ca2c651974d71c62e2f4d0.
> > 
> > Reason for revert:Due to Gerrit outage  http://crbug.com/872722 , we are reverting this CL. Please, re-land it after all clear is given. If you have questions, please ask on the bug. Sorry for the inconvenience.
> > 
> > Original change's description:
> > > Android: moving MediaUrlResolver tasks to thread pool
> > > 
> > > Currently, AsyncTask.execute() defaults to the SERIAL_EXECUTOR. This
> > > exector is good for preventing concurrency errors since it guarantees
> > > serial execution, but bad for performance since the entire app shares
> > > this single queue.
> > > 
> > > It looks like this callsite can use the THREAD_POOL_EXECUTOR instead,
> > > since this use doesn't appear to rely on the concurrency guarantees that
> > > SERIAL_EXECUTOR provides.
> > > 
> > > TBR= test file
> > > Bug:  869907 
> > > Change-Id: I5770f3f0211d2c10b28f2290973bbfd57bee53f0
> > > Reviewed-on: https://chromium-review.googlesource.com/1159236
> > > Commit-Queue: Sam Maier <smaier@chromium.org>
> > > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> > 
> > TBR=mlamouri@chromium.org,smaier@chromium.org
> > 
> > Change-Id: I5cd823a033bb6443007734d4faf0e1ddf98de648
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug:  869907 
> > Reviewed-on: https://chromium-review.googlesource.com/1169854
> > Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> 
> TBR=mlamouri@chromium.org,tandrii@chromium.org,smaier@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  869907 
> Change-Id: I31b02f5b543f2fdb8b0b089f241d79ceb720a626
> Reviewed-on: https://chromium-review.googlesource.com/1171702
> Commit-Queue: Sam Maier <smaier@chromium.org>
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582358}

TBR=mlamouri@chromium.org,tandrii@chromium.org,smaier@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  869907 , 873941
Change-Id: I44847161ac09f49b81cb8186880830c636cfdb33
Reviewed-on: https://chromium-review.googlesource.com/1205450
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588699}(cherry picked from commit 272eecdb1623c84e0d56e4ab1ac5eacf9bb7939f)
Reviewed-on: https://chromium-review.googlesource.com/1217302
Cr-Commit-Position: refs/branch-heads/3538@{#236}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/aa50cabe65fed56697e8be736f046c5fec5f4464/chrome/android/java/src/org/chromium/chrome/browser/media/remote/DefaultMediaRouteController.java
[modify] https://crrev.com/aa50cabe65fed56697e8be736f046c5fec5f4464/chrome/android/junit/src/org/chromium/chrome/browser/media/remote/MediaUrlResolverTest.java

Sign in to add a comment