Ajax requests in installer cause cron to run prematurely

If the only issue here is the auto cron, then I don't see why we couldn't disable it (either off by default or via $conf) right up until it's run by the installer, then we wouldn't need any additional runtime code to handle that case. The cache stuff shouldn't be run, but all the drupal fake cache exists to prevent that and other cache_set() which could be called from somewhere other than drupal_page_footer()).

Seems better to me, but I think we should put the variable check inside system_run_automated_cron() itself. Otherwise looks RTBC.

I'll just add that we need a non hackish way to disable the auto cron during install. There is no way to do that now that I can see. Many installs will be performed by drush site-install and those scripts are quite capable of calling drush cron after installation if they choose.

I was thinking that we should call system_run_automated_cron() from the installer instead of directly calling drupal_cron_run() so that a profile could set cron_safe_threshold=0 and skip cron during install.

Instead of a new variable, isn't it possible to just use the existing 'install_task' variable here (check if it's equal to 'done' or not)?

Otherwise, I guess this patch makes sense since running cron more than once during the installer is pretty wasteful anyway, but geez, the fact that this is currently the only way to solve #831080: Dashboard is completely empty after installation is pretty scary. There is something going on in Drupal that none of us understand yet :)

The installer making ajax requests, which then cause caches (and etc.) to be populated which otherwise shouldn't has been known for a while now, this is why DrupalFakeCache actually tries to delete from cache tables despite never putting anything in them allegedly. Problem is that all the bugs that arise from it are very strange and hard to track down, last one I remember was the site name being wrong after install but fixing itself as soon as you refreshed the page.

I remember it well - I wrote that part of the DrupalFakeCache code :) The reason I think this is surprising is that the Dashboard module is already fully installed well before _block_rehash() gets called (I think several page requests before), and all those cache clearings in between still don't manage to fix whatever out-of-date data is causing this bug and allow its regions to be recognized...

This version looks fine to me.

I think with this patch, the install_task variable won't necessarily wind up 'done' at the end of the installation, because there is no actual guarantee that install_finished() is the last task that will run in all installation profiles...

Also, it seems that complication is only necessary to support this part of the patch:

-  drupal_cron_run();
+  // Profiles may choose to opt out of auto cron by calling
+  // variable_set('cron_safe_threshold', 0).
+  system_run_automated_cron();

But looking at that, I am thinking it's not actually a good idea anyway? If a profile just wants to set the 'cron_safe_threshold' variable to 0 for use in the site itself, it shouldn't be forced to get this change in installation behavior as a side effect. If profiles need a way to opt out of this (I'm not sure I understand what the use case is - but maybe a profile that creates so much stuff that the first cron run will take a really long time and it doesn't want the person installing the site to have to sit there waiting for it?) then I think we need another method. How about we leave that for another issue, since it wasn't part of the original issue anyway?

So if we strip that part out we basically have a one line patch (I added a code comment too, though). Maybe we can go with this, and leave allowing profiles to entirely opt out of cron to another issue?

The shortened comment sounds good to me. Here it is with something very similar to that.

Looks great.

Committed to HEAD, thanks! That one-liner is nice and elegant.

Agreed on pushing the disabling of this behaviour to a separate issue. Since D6 didn't have this capability, this smells like a D8 feature, for possible backport to D7 and D6 once it's in.