GitLab

This is a reland of 1b420fdb

The flakiness in the original CL with ResourceLoadingHintsBrowserTest
and PreviewsNoScriptBrowserTest has been fixed.
RetryForHistogramUntilCountReached() now flushes the task scheduler
and keeps trying until the test times out.

Original change's description:
> Eliminate unnecessary proto::Configuration copies
>
> The previous logic in OptimizationGuideService created an
> optimization_guide::proto::Configuration object on a background thread
> and then used task posting to send it on to
> PreviewsHints::CreateFromConfig(), where it was used to create a
> PreviewsHints object on another background thread. In all, there were
> two tasks posted that included the config object as a parameter, one on
> the background thread and one on the UI thread. Including it as a
> parameter in a task requires a full copy of the protobuffer. This means
> that a full copy of the config protobuf, which can be as large as 1.8MB
> (the size of the current Brazil one on Canary), was occurring on the UI
> thread.
>
> In local performance measurements, making a single copy of a 600KB
> version of the protobuf took several milliseconds (it amounted to
> roughly 60% of the time taken by the initial component string parsing
> and 60% of the time taken by PreviewHints::CreateFromConfig()). Given
> that we were incurring the cost of two copies, one of which was on an
> extremely high priority thread, it makes sense to change the logic to
> eliminate the need for the copies.
>
> The logic has been altered so that OptimizationGuideService no longer
> immediately processes the component, but instead notifies the
> observers that it is available and allows them to trigger the
> processing. This eliminates both copies of the configuration protobuf,
> as it is now created where it is used.
>
> Additionally, OptimizationGuideServiceObservers are now immediately
> notified of the hints component when they register if one is already
> available. This will enable the PreviewsOptimizationGuide to wait until
> the HintCacheLevelDBStore is fully initialized before registering for
> the component, and in the future will potentially allow it to avoid
> processing the configuration altogether when the store already has the
> latest version cached.
>
> New unittests have been added and older ones have been updated to
> accommodate the new logic.
>
> The related browser tests have also been modified to be more robust,
> where they now wait for a signal from a local histogram indicating
> that hint processing is complete.

Bug: 908985, 910251
Change-Id: I90407db4c19dac29e10f756a6de87294a9ab683b
Reviewed-on: https://chromium-review.googlesource.com/c/1355256
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612408}