I'm guessing this is an actual race condition in LocalNtpSource: When the "is Google" state changes on the UI thread, we send that information to the IO thread asynchronously. Now if an NTP gets opened while that message is in flight, we can end up in a broken "half-Google" state, which would explain 2) above. Maybe some slightly different order of events can explain 1) too.


Ideas to fix this:

a) Split LocalNtpSource into a Google and a non-Google variant, serving from different URLs. This might be a good idea anyway, since the current sharing is somewhat confusing and brittle.
Downsides: Some code duplication, slightly more complex URL rewrite logic, and there will still be similar racing problems with Google base URL changes, which are also propagated from UI to IO thread.

b) Move LocalNtpSource::StartDataRequest to the IO thread (via URLDataSource::TaskRunnerForRequestPath). Probably *also* a good idea anyway, since the current thread hopping is broken (and has led to trouble in the past), and is hard to get right so will always be brittle at best.
Downsides: This will make accessing KeyedServices like LogoService and OneGoogleBarService (which live on the UI thread) more of a hassle.

On further reflection, b) won't actually fix the race condition. Possible sequence of events after b) is implemented (all on the IO thread):
- is_google starts out true.
- local_ntp.html is served, containing checksum for config.js based on is_google==true.
- is_google gets set to false.
- config.js is served based on is_google==false.
- Checksum doesn't match.
- Boom.

I still think it's the right thing to do, it just won't fix this particular problem.

Since neither a) nor b) will actually fix this, adding more ideas:

c) Introduce versioning for config.js: While serving local_ntp.html and inserting the checksum, also add a version suffix to the path of config.js ("config-1.js"), and cache the generated config along with the version number. Then when the config file is requested, we know which one to serve.
Problem: The checksum *also* goes into the CSP, and URLDataSource doesn't provide a way to tie CSP requests (GetContentSecurityPolicyScriptSrc) to actual file requests. I don't know how to solve this.

d) Load config.js dynamically (i.e. create a script tag from local_ntp.js). Then thanks to 'strict-dynamic', no checksum for config.js will be required (neither in the html nor in the CPS). Profit!
Potential downsides: It'll make the overall loading process a bit more complicated. This might have a perf impact, though it should be small. It might also make the setup for tests a bit more tricky, though I think that should work out.

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

commit 773623d293ed991e20596c4cb00e58610af5f395
Author: Marc Treib <treib@chromium.org>
Date: Thu Dec 07 15:17:16 2017

Local NTP: Load config.js dynamically

With this, no checksum is required for config.js anymore, which fixes a
race condition between serving the checksum and serving the contents
themselves. It also lets us remove quite a lot of plumbing :)

Bug:  792490 ,  791969 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I609ef274325bddb49bcbe6e90d58d35b7a385920
Reviewed-on: https://chromium-review.googlesource.com/811185
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522428}
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/browser/resources/local_ntp/local_ntp.css
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/browser/resources/local_ntp/local_ntp.html
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/browser/search/local_ntp_source.cc
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/browser/search/local_ntp_source.h
[modify] https://crrev.com/773623d293ed991e20596c4cb00e58610af5f395/chrome/test/data/local_ntp/local_ntp_browsertest.html

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

commit 71d761879bc76e7902d315c55ec0b9492dab0058
Author: Marc Treib <treib@chromium.org>
Date: Fri Dec 08 08:48:41 2017

Local NTP tests: Wait for TemplateURLService to load before using it

This might have been a cause of flakiness in these tests.

Bug:  791969 
Change-Id: I58d66ef67cd708c18fb902310c63aec29ea057b9
Reviewed-on: https://chromium-review.googlesource.com/814516
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522739}
[modify] https://crrev.com/71d761879bc76e7902d315c55ec0b9492dab0058/chrome/browser/ui/search/local_ntp_test_utils.cc

This seems to have done it, no more flakes in the past 3 days.

The NextAction date has arrived: 2017-12-11