Only precompile new regex if flag is enabled

The new regex uses too many resources to be enabled for all users. To
gather experiment data, we will only use it if the flag is enabled. Only
then will we compile the regex and run it against the selected text to
get false positives and negatives data.

Bug: 1029755
Change-Id: I19811328ce445612eea25a5b3d2b518211454469
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1950522Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721928}

chrome/browser/sharing/click_to_call/click_to_call_metrics.cc

@@ -164,6 +164,9 @@ void LogClickToCallPhoneNumberSize(const std::string& number,
 
 void LogPhoneNumberDetectionMetrics(const std::string& selection_text,
                                     bool sent_to_device) {
+  if (!base::FeatureList::IsEnabled(kClickToCallDetectionV2))
+    return;
+
   base::PostTask(FROM_HERE,
                  {base::ThreadPool(), base::TaskPriority::BEST_EFFORT,
                   base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},

chrome/browser/sharing/click_to_call/phone_number_regex.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/sharing/click_to_call/phone_number_regex.h"
 
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/feature_list.h"
+#include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/task/post_task.h"
 #include "base/time/time.h"
@@ -132,9 +134,15 @@ const char kPhoneNumberRegexPatternLowConfidenceModified[] =
 void PrecompilePhoneNumberRegexes() {
   SCOPED_UMA_HISTOGRAM_TIMER("Sharing.ClickToCallPhoneNumberPrecompileTime");
   static const char kExampleInput[] = "+01(2)34-5678 9012";
+  std::vector<PhoneNumberRegexVariant> variants = {
+      PhoneNumberRegexVariant::kSimple};
+
+  // Only precompile the low confidence regex when the flag is enabled.
+  if (base::FeatureList::IsEnabled(kClickToCallDetectionV2))
+    variants.push_back(PhoneNumberRegexVariant::kLowConfidenceModified);
+
   std::string parsed;
-  for (auto variant : {PhoneNumberRegexVariant::kSimple,
-                       PhoneNumberRegexVariant::kLowConfidenceModified}) {
+  for (auto variant : variants) {
     // Run RE2::PartialMatch over some example input to speed up future queries.
     re2::RE2::PartialMatch(kExampleInput, GetPhoneNumberRegex(variant),
                            &parsed);
@@ -152,6 +160,7 @@ const re2::RE2& GetPhoneNumberRegex(PhoneNumberRegexVariant variant) {
     case PhoneNumberRegexVariant::kSimple:
       return *kRegexSimple;
     case PhoneNumberRegexVariant::kLowConfidenceModified:
+      DCHECK(base::FeatureList::IsEnabled(kClickToCallDetectionV2));
       return *kRegexLowConfidenceModified;
   }
 }

chrome/browser/ui/views/sharing/click_to_call_browsertest.cc

@@ -58,14 +58,10 @@ enum class ClickToCallPolicy {
 
 }  // namespace
 
-// Browser tests for the Click To Call feature.
-class ClickToCallBrowserTest : public SharingBrowserTest {
+// Base browser tests for the Click To Call feature.
+class BaseClickToCallBrowserTest : public SharingBrowserTest {
  public:
-  ClickToCallBrowserTest() {
-    feature_list_.InitAndEnableFeature(kClickToCallUI);
-  }
-
-  ~ClickToCallBrowserTest() override {}
+  ~BaseClickToCallBrowserTest() override {}
 
   std::string GetTestPageURL() const override {
     return std::string(kTestPageURL);
@@ -86,9 +82,17 @@ class ClickToCallBrowserTest : public SharingBrowserTest {
   std::string HistogramName(const char* suffix) {
     return base::StrCat({"Sharing.ClickToCall", suffix});
   }
+};
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(ClickToCallBrowserTest);
+// Browser tests for the Click To Call feature.
+class ClickToCallBrowserTest : public BaseClickToCallBrowserTest {
+ public:
+  ClickToCallBrowserTest() {
+    feature_list_.InitAndEnableFeature(kClickToCallUI);
+  }
+
+ protected:
+  base::test::ScopedFeatureList feature_list_;
 };
 
 // TODO(himanshujaju): Add UI checks.
@@ -309,14 +313,8 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest,
       {HistogramName("PhoneNumberDigits.RightClickSelection.Showing"), 1},
       {HistogramName("PhoneNumberLength"), 1},
       {HistogramName("PhoneNumberLength.RightClickSelection.Showing"), 1},
-      {HistogramName(
-           "PhoneNumberRegexVariantResult.LowConfidenceModified.Showing"),
-       1},
-      {HistogramName("ContextMenuPhoneNumberParsingDelay"), 2},
+      {HistogramName("ContextMenuPhoneNumberParsingDelay"), 1},
       {HistogramName("ContextMenuPhoneNumberParsingDelay.Simple"), 1},
-      {HistogramName(
-           "ContextMenuPhoneNumberParsingDelay.LowConfidenceModified"),
-       1},
   };
   EXPECT_THAT(histograms.GetTotalCountsForPrefix(HistogramName("")),
               testing::ContainerEq(expected_counts));
@@ -331,10 +329,6 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest,
   histograms.ExpectUniqueSample(
       HistogramName("PhoneNumberLength.RightClickSelection.Showing"),
       /*sample=*/9, /*count=*/1);
-  histograms.ExpectUniqueSample(
-      HistogramName(
-          "PhoneNumberRegexVariantResult.LowConfidenceModified.Showing"),
-      /*sample=*/PhoneNumberRegexVariantResult::kBothMatch, /*count=*/1);
 
   // Send the number to the device in the context menu.
   menu->ExecuteCommand(IDC_CONTENT_CONTEXT_SHARING_CLICK_TO_CALL_SINGLE_DEVICE,
@@ -348,9 +342,6 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest,
       {HistogramName("PhoneNumberDigits.RightClickSelection.Sending"), 1},
       {HistogramName("PhoneNumberLength"), 2},
       {HistogramName("PhoneNumberLength.RightClickSelection.Sending"), 1},
-      {HistogramName(
-           "PhoneNumberRegexVariantResult.LowConfidenceModified.Sending"),
-       1},
   });
   expected_counts[HistogramName("PhoneNumberDigits")] = 2;
   expected_counts[HistogramName("PhoneNumberLength")] = 2;
@@ -372,11 +363,6 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest,
       HistogramName("PhoneNumberLength.RightClickSelection.Sending"),
       /*sample=*/9,
       /*count=*/1);
-  histograms.ExpectUniqueSample(
-      HistogramName(
-          "PhoneNumberRegexVariantResult.LowConfidenceModified.Sending"),
-      /*sample=*/PhoneNumberRegexVariantResult::kBothMatch,
-      /*count=*/1);
 }
 
 IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest, ContextMenu_UKM) {
@@ -621,3 +607,54 @@ INSTANTIATE_TEST_SUITE_P(All,
                          ::testing::Values(ClickToCallPolicy::kNotConfigured,
                                            ClickToCallPolicy::kFalse,
                                            ClickToCallPolicy::kTrue));
+
+class ClickToCallBrowserTestDetectionV2 : public BaseClickToCallBrowserTest {
+ public:
+  ClickToCallBrowserTestDetectionV2() {
+    feature_list_.InitWithFeatures({kClickToCallUI, kClickToCallDetectionV2},
+                                   {});
+  }
+
+ protected:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTestDetectionV2,
+                       ContextMenu_HighlightedText_Histograms) {
+  base::HistogramTester histograms;
+  Init(sync_pb::SharingSpecificFields::CLICK_TO_CALL,
+       sync_pb::SharingSpecificFields::UNKNOWN);
+
+  // Trigger a context menu for a selection with 8 digits and 9 characters.
+  std::unique_ptr<TestRenderViewContextMenu> menu =
+      InitContextMenu(GURL(kNonTelUrl), kLinkText, "1234-5678");
+  // RegexVariantResult is logged on a thread pool.
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+
+  histograms.ExpectTotalCount(
+      HistogramName("ContextMenuPhoneNumberParsingDelay"), 2);
+  histograms.ExpectTotalCount(
+      HistogramName("ContextMenuPhoneNumberParsingDelay.LowConfidenceModified"),
+      1);
+  histograms.ExpectUniqueSample(
+      HistogramName(
+          "PhoneNumberRegexVariantResult.LowConfidenceModified.Showing"),
+      /*sample=*/PhoneNumberRegexVariantResult::kBothMatch, /*count=*/1);
+
+  // Send the number to the device in the context menu.
+  menu->ExecuteCommand(IDC_CONTENT_CONTEXT_SHARING_CLICK_TO_CALL_SINGLE_DEVICE,
+                       0);
+  // RegexVariantResult is logged on a thread pool.
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+
+  histograms.ExpectTotalCount(
+      HistogramName("ContextMenuPhoneNumberParsingDelay"), 2);
+  histograms.ExpectTotalCount(
+      HistogramName("ContextMenuPhoneNumberParsingDelay.LowConfidenceModified"),
+      1);
+  histograms.ExpectUniqueSample(
+      HistogramName(
+          "PhoneNumberRegexVariantResult.LowConfidenceModified.Sending"),
+      /*sample=*/PhoneNumberRegexVariantResult::kBothMatch,
+      /*count=*/1);
+}