Reland "Re-enable PredictionManager browser tests on Win/Mac/CrOS"

This is a reland of 1a291df2

Original change's description:
> Re-enable PredictionManager browser tests on Win/Mac/CrOS
>
> And fix init issue on ChromeOS for sign in profiles
>
> Change-Id: I26b87d58f2eae2afecf67685690312a8aed70c60
> Bug: 1146151
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2625157
> Reviewed-by: Michael Crouse <mcrouse@chromium.org>
> Commit-Queue: Sophie Chang <sophiechang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#843176}

Bug: 1146151
Change-Id: Id9b0353b3f5e7d5ffc3d27f68aec612efad4e68e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626985Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843590}

chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
 
+#include "build/build_config.h"
 #include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
@@ -11,6 +12,10 @@
 #include "components/optimization_guide/core/optimization_guide_features.h"
 #include "content/public/browser/browser_context.h"
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#endif
+
 // static
 OptimizationGuideKeyedService*
 OptimizationGuideKeyedServiceFactory::GetForProfile(Profile* profile) {
@@ -38,6 +43,18 @@ OptimizationGuideKeyedServiceFactory::~OptimizationGuideKeyedServiceFactory() =
 
 KeyedService* OptimizationGuideKeyedServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  // Do not build the OptimizationGuideKeyedService if it's a sign-in profile
+  // since it basically is an ephemeral profile anyway and we cannot provide
+  // hints or models to it anyway. Additionally, sign in profiles do not go
+  // through the standard profile initialization flow, so a lot of things that
+  // are required are not available when the browser context for the signin
+  // profile is created.
+  Profile* profile = Profile::FromBrowserContext(context);
+  if (chromeos::ProfileHelper::IsSigninProfile(profile))
+    return nullptr;
+#endif
+
   return new OptimizationGuideKeyedService(context);
 }
 

chrome/browser/optimization_guide/prediction/prediction_manager_browsertest.cc

@@ -10,6 +10,7 @@
 #include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_run_loop_timeout.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/browser_process.h"
@@ -456,15 +457,8 @@ class PredictionManagerBrowserTest : public PredictionManagerBrowserTestBase {
   }
 };
 
-#if defined(OS_WIN) || defined(OS_MAC) || BUILDFLAG(IS_CHROMEOS_ASH)
-#define DISABLE_ON_WIN_MAC_CHROMEOS(x) DISABLED_##x
-#else
-#define DISABLE_ON_WIN_MAC_CHROMEOS(x) x
-#endif
-
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(FCPReachedSessionStatisticsUpdated)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       FCPReachedSessionStatisticsUpdated) {
   RegisterWithKeyedService();
   auto waiter = CreatePageLoadMetricsTestWaiter();
   waiter->AddPageExpectation(
@@ -478,9 +472,8 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(1u, session_fcp->GetNumberOfSamples());
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(NoFCPSessionStatisticsUnchanged)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       NoFCPSessionStatisticsUnchanged) {
   RegisterWithKeyedService();
   auto waiter = CreatePageLoadMetricsTestWaiter();
   waiter->AddPageExpectation(
@@ -501,9 +494,8 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(current_mean, session_fcp->GetMean());
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(ModelsAndFeaturesStoreInitialized)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       ModelsAndFeaturesStoreInitialized) {
   base::HistogramTester histogram_tester;
   content::NetworkConnectionChangeSimulator().SetConnectionType(
       network::mojom::ConnectionType::CONNECTION_2G);
@@ -526,9 +518,8 @@ IN_PROC_BROWSER_TEST_F(
       "OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 2, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(OnlyHostModelFeaturesInGetModelsResponse)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       OnlyHostModelFeaturesInGetModelsResponse) {
   base::HistogramTester histogram_tester;
 
   SetResponseType(PredictionModelsFetcherRemoteResponseType::
@@ -547,9 +538,8 @@ IN_PROC_BROWSER_TEST_F(
       "OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 0);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(OnlyPredictionModelsInGetModelsResponse)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       OnlyPredictionModelsInGetModelsResponse) {
   base::HistogramTester histogram_tester;
 
   SetResponseType(PredictionModelsFetcherRemoteResponseType::
@@ -574,9 +564,8 @@ IN_PROC_BROWSER_TEST_F(
       "OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 2, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(PredictionModelFetchFailed)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       PredictionModelFetchFailed) {
   SetResponseType(PredictionModelsFetcherRemoteResponseType::kUnsuccessful);
   base::HistogramTester histogram_tester;
 
@@ -602,9 +591,8 @@ IN_PROC_BROWSER_TEST_F(
       "OptimizationGuide.PredictionModelLoadedVersion.PainfulPageLoad", 0);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(HostModelFeaturesClearedOnHistoryClear)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       HostModelFeaturesClearedOnHistoryClear) {
   base::HistogramTester histogram_tester;
 
   RegisterWithKeyedService();
@@ -709,9 +697,8 @@ IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest, IncognitoCanStillRead) {
       1);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(IncognitoDoesntFetchModels)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserTest,
+                       IncognitoDoesntFetchModels) {
   SetResponseType(PredictionModelsFetcherRemoteResponseType::
                       kSuccessfulWithModelsAndFeatures);
   base::HistogramTester histogram_tester;
@@ -770,7 +757,7 @@ class PredictionManagerBrowserSameOriginTest
 // Regression test for https://crbug.com/1037945. Tests that the origin of the
 // previous navigation is computed correctly.
 IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserSameOriginTest,
-                       DISABLE_ON_WIN_MAC_CHROMEOS(IsSameOriginNavigation)) {
+                       IsSameOriginNavigation) {
   base::HistogramTester histogram_tester;
 
   RegisterWithKeyedService();
@@ -823,9 +810,8 @@ IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserSameOriginTest,
       "OptimizationGuide.PredictionManager.IsSameOrigin", true, 1);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerBrowserSameOriginTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(ShouldTargetNavigationAsync)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerBrowserSameOriginTest,
+                       ShouldTargetNavigationAsync) {
   base::HistogramTester histogram_tester;
 
   RegisterWithKeyedService();
@@ -901,8 +887,7 @@ class PredictionManagerNoUserPermissionsTest
 };
 
 IN_PROC_BROWSER_TEST_F(PredictionManagerNoUserPermissionsTest,
-                       DISABLE_ON_WIN_MAC_CHROMEOS(
-                           HostsAndFieldTrialsNotPassedWhenNoUserPermissions)) {
+                       HostsAndFieldTrialsNotPassedWhenNoUserPermissions) {
   base::HistogramTester histogram_tester;
 
   SetResponseType(PredictionModelsFetcherRemoteResponseType::
@@ -1006,10 +991,8 @@ class PredictionManagerModelDownloadingBrowserTest
   std::unique_ptr<ModelFileObserver> model_file_observer_;
 };
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerModelDownloadingBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(
-        TestDownloadUrlAcceptedByDownloadServiceButInvalid)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerModelDownloadingBrowserTest,
+                       TestDownloadUrlAcceptedByDownloadServiceButInvalid) {
   base::HistogramTester histogram_tester;
 
   SetResponseType(PredictionModelsFetcherRemoteResponseType::
@@ -1031,9 +1014,8 @@ IN_PROC_BROWSER_TEST_F(
       "OptimizationGuide.PredictionModelUpdateVersion.PainfulPageLoad", 0);
 }
 
-IN_PROC_BROWSER_TEST_F(
-    PredictionManagerModelDownloadingBrowserTest,
-    DISABLE_ON_WIN_MAC_CHROMEOS(TestSuccessfulModelFileFlow)) {
+IN_PROC_BROWSER_TEST_F(PredictionManagerModelDownloadingBrowserTest,
+                       TestSuccessfulModelFileFlow) {
   // TODO(crbug/1146151): Remove this switch once we can produce a signed model
   // file.
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
@@ -1058,8 +1040,13 @@ IN_PROC_BROWSER_TEST_F(
   // containing a download URL and then subsequently downloaded.
   RegisterModelFileObserverWithKeyedService();
 
-  // Wait until the observer receives the file.
+  // Wait until the observer receives the file. We increase the timeout to 60
+  // seconds here since the file is on the larger side.
+  {
+    base::test::ScopedRunLoopTimeout file_download_timeout(
+        FROM_HERE, base::TimeDelta::FromSeconds(60));
     run_loop->Run();
+  }
 
   histogram_tester.ExpectUniqueSample(
       "OptimizationGuide.PredictionModelDownloadManager.DownloadStatus",