New issue
Advanced search Search tips

Issue 810049 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup components/task_scheduler_util/.

Project Member Reported by fdoray@chromium.org, Feb 7 2018

Issue description

The code in components/task_scheduler_util can be simplified a lot:

- We can rely on the metrics system to pass variation params to child processes.
- We no longer need support for |backward_compatibility|.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 8 2018

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

commit 9626f0d2b516fbfe230945e99a44f84cdef6f090
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Feb 08 19:06:29 2018

Add GetTaskSchedulerInitParams() overload that reads variation params by itself.

The existing overload of GetTaskSchedulerInitParams() took a
|variation_params| argument because the source of these variation params
was different in the browser process (variations::GetVariationParams)
and in renderer processes (command line).

Now that variation params are available in all processes via
base::GetFieldTrialParams(), we don't need a |variation_params|
argument in GetTaskSchedulerInitParams(). We can simply call
base::GetFieldTrialParams() from that function.

The overload with a |variation_params| is kept until all callers
have been migrated to the new overload.

Bug:  810049 
Change-Id: Iec3396ac8752d989d58d9d25daf6f9dc3136d121
Reviewed-on: https://chromium-review.googlesource.com/909076
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535466}
[modify] https://crrev.com/9626f0d2b516fbfe230945e99a44f84cdef6f090/components/task_scheduler_util/common/variations_util.cc
[modify] https://crrev.com/9626f0d2b516fbfe230945e99a44f84cdef6f090/components/task_scheduler_util/common/variations_util.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 13 2018

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

commit bc0d03c0feaf0d43e4597a226427dc8466747a3c
Author: Francois Doray <fdoray@chromium.org>
Date: Tue Feb 13 20:51:18 2018

Remove components/task_scheduler_util/browser.

It takes less code to call
task_scheduler_util::GetTaskSchedulerInitParams() directly from
ChromeContentBrowserClient::GetTaskSchedulerInitParams().

Bug:  810049 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I23d7f4ad202f968ffe3e877bc39c48508bc5ea76
Reviewed-on: https://chromium-review.googlesource.com/909077
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536465}
[modify] https://crrev.com/bc0d03c0feaf0d43e4597a226427dc8466747a3c/chrome/browser/BUILD.gn
[modify] https://crrev.com/bc0d03c0feaf0d43e4597a226427dc8466747a3c/chrome/browser/chrome_content_browser_client.cc
[delete] https://crrev.com/033fa13912a0236b9a0d76321e754b1cdb5a7258/components/task_scheduler_util/browser/BUILD.gn
[delete] https://crrev.com/033fa13912a0236b9a0d76321e754b1cdb5a7258/components/task_scheduler_util/browser/DEPS
[delete] https://crrev.com/033fa13912a0236b9a0d76321e754b1cdb5a7258/components/task_scheduler_util/browser/initialization.cc
[delete] https://crrev.com/033fa13912a0236b9a0d76321e754b1cdb5a7258/components/task_scheduler_util/browser/initialization.h
[modify] https://crrev.com/bc0d03c0feaf0d43e4597a226427dc8466747a3c/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/bc0d03c0feaf0d43e4597a226427dc8466747a3c/ios/chrome/app/startup/ios_chrome_main.mm
[modify] https://crrev.com/bc0d03c0feaf0d43e4597a226427dc8466747a3c/ios/chrome/browser/web/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 16 2018

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

commit 7f56fd822d1edb7400af10da9fd66788b6e91542
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Feb 16 14:49:45 2018

Do not get TaskScheduler field trial params from command line in renderers.

base::GetFieldTrialParams() used not to work in child processes. For
that reason, we built a system to pass scheduler field trial params
to renderers on the command line. Now that base::GetFieldTrialParams()
works fine in child processes
(https://cs.chromium.org/chromium/src/content/app/content_main_runner.cc?l=160&rcl=9030625190ec0a2b60be2af279bcb234f47d3f51),
we don't need to use that system anymore.

TBR=rohitrao@chromium.org

Bug:  810049 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic40653eece1a103a9c913cb286925061fed1121c
Reviewed-on: https://chromium-review.googlesource.com/909169
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537305}
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/chrome/renderer/BUILD.gn
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/chrome/renderer/DEPS
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/components/task_scheduler_util/common/variations_util.cc
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/components/task_scheduler_util/common/variations_util.h
[modify] https://crrev.com/7f56fd822d1edb7400af10da9fd66788b6e91542/ios/chrome/app/startup/ios_chrome_main.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 16 2018

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

commit 3879680b2cd32054612182b881ce7d1743a1416a
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Feb 16 17:20:20 2018

Remove components/task_scheduler_util/renderer.

This code is not used anymore.

Bug:  810049 
Change-Id: I029652468ec67d8b238a89e25b8cb9193435b4dd
Reviewed-on: https://chromium-review.googlesource.com/909289
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537337}
[delete] https://crrev.com/ece52f425ffafdbf92b5483a0bf5cfb8e3875650/components/task_scheduler_util/renderer/BUILD.gn
[delete] https://crrev.com/ece52f425ffafdbf92b5483a0bf5cfb8e3875650/components/task_scheduler_util/renderer/initialization.cc
[delete] https://crrev.com/ece52f425ffafdbf92b5483a0bf5cfb8e3875650/components/task_scheduler_util/renderer/initialization.h

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 17 2018

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

commit 94bb483704f2151abdc73483e1d29e7e6ae57d4f
Author: Francois Doray <fdoray@chromium.org>
Date: Sat Feb 17 03:26:50 2018

Remove task_scheduler_util::(Add|Get)VariationsParams(To|From)CommandLine.

These functions were used to pass scheduler field trial params to
renderer processes. They are no longer used since
https://chromium-review.googlesource.com/c/chromium/src/+/909169

Bug:  810049 
Change-Id: I480b7e56b70f5c24e7a656cc18673ba31eac6948
Reviewed-on: https://chromium-review.googlesource.com/909171
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537520}
[modify] https://crrev.com/94bb483704f2151abdc73483e1d29e7e6ae57d4f/components/task_scheduler_util/common/variations_util.cc
[modify] https://crrev.com/94bb483704f2151abdc73483e1d29e7e6ae57d4f/components/task_scheduler_util/common/variations_util.h
[modify] https://crrev.com/94bb483704f2151abdc73483e1d29e7e6ae57d4f/components/task_scheduler_util/common/variations_util_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 27 2018

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

commit d70dd063b6346f5d2216b55b3e2f47cfaaffc14c
Author: Francois Doray <fdoray@chromium.org>
Date: Tue Feb 27 17:29:56 2018

Remove GetTaskSchedulerInitParams() overload that takes variations params as argument.

This overload is no longer used. All callers use the overload that
calls base::GetFieldTrialParams() directly.

Bug:  810049 
Change-Id: I0cde22bd6701b8b698b90aa7b39a10666fe7d32f
Reviewed-on: https://chromium-review.googlesource.com/909296
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539484}
[modify] https://crrev.com/d70dd063b6346f5d2216b55b3e2f47cfaaffc14c/components/task_scheduler_util/common/BUILD.gn
[modify] https://crrev.com/d70dd063b6346f5d2216b55b3e2f47cfaaffc14c/components/task_scheduler_util/common/DEPS
[modify] https://crrev.com/d70dd063b6346f5d2216b55b3e2f47cfaaffc14c/components/task_scheduler_util/common/variations_util.cc
[modify] https://crrev.com/d70dd063b6346f5d2216b55b3e2f47cfaaffc14c/components/task_scheduler_util/common/variations_util.h
[modify] https://crrev.com/d70dd063b6346f5d2216b55b3e2f47cfaaffc14c/components/task_scheduler_util/common/variations_util_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 13 2018

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

commit 02af3e83119f7b696ab785776ef967753a32cf93
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Apr 13 13:04:22 2018

Move components/task_scheduler_util/common/ to components/task_scheduler_util/.

The browser/ and renderer/ subdirectories no longer exist in
components/task_scheduler_util/. There is no good reason to keep
the common/ subdirectories, everything can be moved directly to
components/task_scheduler_util/.

This is the last phase of the clean up planned in
 https://crbug.com/810049 

TBR=asvitkine@chromium.org

Bug:  810049 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If5d000edfd1bd689950ab8f8a6992ecbfbb93c78
Reviewed-on: https://chromium-review.googlesource.com/1005788
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550604}
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/browser/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/DEPS
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/BUILD.gn
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/BUILD.gn
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/DEPS
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util.cc
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util.h
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util_unittest.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/ios/chrome/app/startup/ios_chrome_main.mm

Comment 9 by fdoray@chromium.org, Apr 13 2018

Status: Fixed (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 17 2018

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

commit 02af3e83119f7b696ab785776ef967753a32cf93
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Apr 13 13:04:22 2018

Move components/task_scheduler_util/common/ to components/task_scheduler_util/.

The browser/ and renderer/ subdirectories no longer exist in
components/task_scheduler_util/. There is no good reason to keep
the common/ subdirectories, everything can be moved directly to
components/task_scheduler_util/.

This is the last phase of the clean up planned in
 https://crbug.com/810049 

TBR=asvitkine@chromium.org

Bug:  810049 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If5d000edfd1bd689950ab8f8a6992ecbfbb93c78
Reviewed-on: https://chromium-review.googlesource.com/1005788
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550604}
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/browser/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/DEPS
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/BUILD.gn
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/BUILD.gn
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/DEPS
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util.cc
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util.h
[rename] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/components/task_scheduler_util/variations_util_unittest.cc
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/ios/chrome/app/startup/BUILD.gn
[modify] https://crrev.com/02af3e83119f7b696ab785776ef967753a32cf93/ios/chrome/app/startup/ios_chrome_main.mm

Sign in to add a comment