Replace REQUEST_TIME with time service in cron service

The cron service gets a bunch of other services in its constructor. Can we inject time service in the same way (i.e. adding another constructor parameter)?

I am working on this at DrupalCon Vienna

As suggested by valthebald I have added the time service into the constructor.

Exactly what I was looking for, thank you @chenderson!

Adding issue summary, tagging, moving to RTBC

+++ b/core/core.services.yml
@@ -345,7 +345,7 @@ services:
-    arguments: ['@module_handler', '@lock', '@queue', '@state', '@account_switcher', '@logger.channel.cron', '@plugin.manager.queue_worker']
+    arguments: ['@module_handler', '@lock', '@queue', '@state', '@account_switcher', '@logger.channel.cron', '@plugin.manager.queue_worker', '@datetime.time']
     lazy: true

+++ b/core/lib/Drupal/Core/Cron.php
@@ -86,8 +94,10 @@ class Cron implements CronInterface {
-  public function __construct(ModuleHandlerInterface $module_handler, LockBackendInterface $lock, QueueFactory $queue_factory, StateInterface $state, AccountSwitcherInterface $account_switcher, LoggerInterface $logger, QueueWorkerManagerInterface $queue_manager) {
+  public function __construct(ModuleHandlerInterface $module_handler, LockBackendInterface $lock, QueueFactory $queue_factory, StateInterface $state, AccountSwitcherInterface $account_switcher, LoggerInterface $logger, QueueWorkerManagerInterface $queue_manager, TimeInterface $time) {
     $this->moduleHandler = $module_handler;
     $this->lock = $lock;
     $this->queueFactory = $queue_factory;
@@ -95,6 +105,7 @@ public function __construct(ModuleHandlerInterface $module_handler, LockBackendI

@@ -95,6 +105,7 @@ public function __construct(ModuleHandlerInterface $module_handler, LockBackendI
     $this->accountSwitcher = $account_switcher;
     $this->logger = $logger;
     $this->queueManager = $queue_manager;
+    $this->time = $time;
   }

We should make the $time parameter optional as otherwise code subclassing this class and overriding the constructor, would break.

@valthebald thanks :)

@dawehner I have made the $time parameter optional and have set the tests running.

Just wondering for future reference, is it required to run all the test variations for this type of change? Also I assume I needed change this back to 'Needs review' given the updated patch?

@chenderson - there is no need to add all possible test combinations, default one is enough.
@dawehner - according to https://www.drupal.org/node/2562903#classes, only interfaces should be considered public API, and CronInterface does not set any expectations on constructor parameters

Just an example: http://cgit.drupalcode.org/ultimate_cron/tree/src/UltimateCron.php?h=8.x...
I think avoiding BC breaks when we can easily is not really worth discussing :)

@dawehner: oki-doki! :)

testbot is happy, I dare move the issue back to RTBC

Re #17 Thank you valdthebald. Testbot maintainer here trying to figure out why we're racking up such high costs. RTBC automatic retesting is running 14 tests a day.

Committed d26fe82 and pushed to 8.5.x. Thanks!