Create web::features::UseWKWebViewLoading

web::features::UseWKWebViewLoading is a helper method that uses 3
related feature flags to control the new feature of using
WKWebView.loading for WebState::IsLoading:

1. kSlimNavigationManager is used because we only want to fix the
   failing test cases with slim-nav enabled;
2. kLogLoadStartedInDidStartNavigation is used because the CPM metric
   was logged in WebStateDidStartLoading. This flag moves the logging to
   WebStateDidStartNavigation, and if it's enabled then the CPM won't be
   affected by this feature;
3. kUseWKWebViewLoading is the flag for this feature, because we want
   to use it for experiment with kLogLoadStartedInDidStartNavigation
   enabled to make sure that this feature doesn't change CPM.

Bug: 991763
Change-Id: I99fe31317277163eb4768b2db1c9ef8f5d2dd844
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833470Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708135}

ios/chrome/browser/metrics/BUILD.gn

@@ -109,6 +109,7 @@ source_set("metrics") {
     "//ios/chrome/browser/web_state_list",
     "//ios/chrome/common",
     "//ios/public/provider/chrome/browser",
+    "//ios/web/common",
     "//ios/web/public/deprecated",
     "//url",
   ]
@@ -144,6 +145,7 @@ source_set("unit_tests") {
     "//ios/chrome/browser/browser_state:test_support",
     "//ios/chrome/browser/web:test_support",
     "//ios/chrome/test:test_support",
+    "//ios/web/common",
     "//ios/web/public:public",
     "//ios/web/public/test",
     "//ios/web/public/test/fakes",

ios/chrome/browser/metrics/features.cc

@@ -3,6 +3,3 @@
 // found in the LICENSE file.
 
 #include "ios/chrome/browser/metrics/features.h"
-
-const base::Feature kLogLoadStartedInDidStartNavigation{
-    "LogLoadStartedInDidStartNavigation", base::FEATURE_ENABLED_BY_DEFAULT};

ios/chrome/browser/metrics/features.h

@@ -7,7 +7,4 @@
 
 #include "base/feature_list.h"
 
-// Feature flag to move -LogLoadStarted() to WebStateDidStartNavigation().
-extern const base::Feature kLogLoadStartedInDidStartNavigation;
-
 #endif  // IOS_CHROME_BROWSER_METRICS_FEATURES_H_

ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider.mm

@@ -8,6 +8,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
 #include "ios/chrome/browser/metrics/features.h"
+#include "ios/web/common/features.h"
 #import "ios/web/public/navigation/navigation_context.h"
 #include "url/gurl.h"
 
@@ -65,7 +66,8 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartLoading(
     return;
 
   UMA_HISTOGRAM_BOOLEAN(kPageLoadCountLoadingStartedMetric, true);
-  if (!base::FeatureList::IsEnabled(kLogLoadStartedInDidStartNavigation))
+  if (!base::FeatureList::IsEnabled(
+          web::features::kLogLoadStartedInDidStartNavigation))
     helper_.LogLoadStarted();
 }
 
@@ -82,7 +84,8 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartNavigation(
   } else if (navigation_context->IsSameDocument()) {
     type = PageLoadCountNavigationType::SAME_DOCUMENT_WEB_NAVIGATION;
   } else {
-    if (base::FeatureList::IsEnabled(kLogLoadStartedInDidStartNavigation))
+    if (base::FeatureList::IsEnabled(
+            web::features::kLogLoadStartedInDidStartNavigation))
       helper_.LogLoadStarted();
   }
   UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMetric, type,

ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider_unittest.mm

@@ -10,6 +10,7 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/prefs/testing_pref_service.h"
 #include "ios/chrome/browser/metrics/features.h"
+#include "ios/web/common/features.h"
 #import "ios/web/public/test/fakes/fake_navigation_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -33,7 +34,8 @@ class IOSChromeStabilityMetricsProviderTest : public PlatformTest {
 
 TEST_F(IOSChromeStabilityMetricsProviderTest,
        DidStartLoadingEventShouldIncrementPageLoadCount) {
-  if (base::FeatureList::IsEnabled(kLogLoadStartedInDidStartNavigation))
+  if (base::FeatureList::IsEnabled(
+          web::features::kLogLoadStartedInDidStartNavigation))
     return;
   IOSChromeStabilityMetricsProvider provider(&prefs_);
 
@@ -66,7 +68,8 @@ TEST_F(IOSChromeStabilityMetricsProviderTest,
 
 TEST_F(IOSChromeStabilityMetricsProviderTest,
        DidStartNavigationEventShouldIncrementPageLoadCount) {
-  if (!base::FeatureList::IsEnabled(kLogLoadStartedInDidStartNavigation))
+  if (!base::FeatureList::IsEnabled(
+          web::features::kLogLoadStartedInDidStartNavigation))
     return;
 
   web::FakeNavigationContext context;
@@ -185,8 +188,10 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, WebNavigationShouldLogPageLoad) {
 
   metrics::SystemProfileProto system_profile;
   provider.ProvideStabilityMetrics(&system_profile);
-  int page_load_count =
-      base::FeatureList::IsEnabled(kLogLoadStartedInDidStartNavigation) ? 1 : 0;
+  int page_load_count = base::FeatureList::IsEnabled(
+                            web::features::kLogLoadStartedInDidStartNavigation)
+                            ? 1
+                            : 0;
   EXPECT_EQ(page_load_count, system_profile.stability().page_load_count());
 }
 

ios/web/common/features.h

@@ -39,6 +39,16 @@ extern const base::Feature kClearOldNavigationRecordsWorkaround;
 // Used to enable committed interstitials for SSL errors.
 extern const base::Feature kSSLCommittedInterstitials;
 
+// Used to enable using WKWebView.loading for WebState::IsLoading.
+extern const base::Feature kUseWKWebViewLoading;
+
+// Feature flag to move -LogLoadStarted() to WebStateDidStartNavigation().
+extern const base::Feature kLogLoadStartedInDidStartNavigation;
+
+// Use WKWebView.loading to update WebState::IsLoading.
+// TODO(crbug.com/1006012): Clean up this flag after experiment.
+bool UseWKWebViewLoading();
+
 }  // namespace features
 }  // namespace web
 

ios/web/common/features.mm

@@ -35,5 +35,25 @@ const base::Feature kClearOldNavigationRecordsWorkaround{
 const base::Feature kSSLCommittedInterstitials{
     "SSLCommittedInterstitials", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kUseWKWebViewLoading{"UseWKWebViewLoading",
+                                         base::FEATURE_DISABLED_BY_DEFAULT};
+
+const base::Feature kLogLoadStartedInDidStartNavigation{
+    "LogLoadStartedInDidStartNavigation", base::FEATURE_ENABLED_BY_DEFAULT};
+
+// The feature kUseWKWebViewLoading will change the CPM if
+// kLogLoadStartedInDidStartNavigation is not enabled, so
+// kLogLoadStartedInDidStartNavigation is required. The feature flag
+// kUseWKWebViewLoading itself is also required to make sure CPM is not changed
+// even with kLogLoadStartedInDidStartNavigation enabled. The flag
+// kSlimNavigationManager is used here because we only want to fix test failure
+// for slim-nav since it's going to be shipped soon.
+bool UseWKWebViewLoading() {
+  return base::FeatureList::IsEnabled(web::features::kSlimNavigationManager) &&
+         base::FeatureList::IsEnabled(web::features::kUseWKWebViewLoading) &&
+         base::FeatureList::IsEnabled(
+             web::features::kLogLoadStartedInDidStartNavigation);
+}
+
 }  // namespace features
 }  // namespace web