UKM: fix ukm app recording criteria for PWA

PWA is treated same as general websites and thus it doesn't need
to check additionally if extensions sync is turned on for UKM
logging.

Bug: 1068404
Change-Id: I4fee2bdf53527a8f8693be27205e7fb4d68142b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152895Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Phillis Tang <phillis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760190}

components/ukm/app_source_url_recorder.cc

@@ -7,7 +7,6 @@
 #include "base/atomic_sequence_num.h"
 #include "components/crx_file/id_util.h"
 #include "services/metrics/public/cpp/delegating_ukm_recorder.h"
-#include "services/metrics/public/cpp/ukm_recorder.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 #include "url/gurl.h"
 
@@ -21,7 +20,7 @@ SourceId AssignNewAppId() {
 SourceId AppSourceUrlRecorder::GetSourceIdForChromeExtension(
     const std::string& id) {
   GURL url("chrome-extension://" + id);
-  return GetSourceIdForUrl(url);
+  return GetSourceIdForUrl(url, AppType::kExtension);
 }
 
 SourceId AppSourceUrlRecorder::GetSourceIdForArc(
@@ -29,14 +28,15 @@ SourceId AppSourceUrlRecorder::GetSourceIdForArc(
   const std::string package_name_hash =
       crx_file::id_util::GenerateId(package_name);
   GURL url("app://play/" + package_name_hash);
-  return GetSourceIdForUrl(url);
+  return GetSourceIdForUrl(url, AppType::kArc);
 }
 
 SourceId AppSourceUrlRecorder::GetSourceIdForPWA(const GURL& url) {
-  return GetSourceIdForUrl(url);
+  return GetSourceIdForUrl(url, AppType::kPWA);
 }
 
-SourceId AppSourceUrlRecorder::GetSourceIdForUrl(const GURL& url) {
+SourceId AppSourceUrlRecorder::GetSourceIdForUrl(const GURL& url,
+                                                 AppType app_type) {
   ukm::DelegatingUkmRecorder* const recorder =
       ukm::DelegatingUkmRecorder::Get();
   if (!recorder)
@@ -44,7 +44,7 @@ SourceId AppSourceUrlRecorder::GetSourceIdForUrl(const GURL& url) {
 
   const SourceId source_id = AssignNewAppId();
   if (base::FeatureList::IsEnabled(kUkmAppLogging)) {
-    recorder->UpdateAppURL(source_id, url);
+    recorder->UpdateAppURL(source_id, url, app_type);
   }
   return source_id;
 }

components/ukm/app_source_url_recorder.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_UKM_APP_SOURCE_URL_RECORDER_H_
 #define COMPONENTS_UKM_APP_SOURCE_URL_RECORDER_H_
 
+#include "services/metrics/public/cpp/ukm_recorder.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 
 #include "base/feature_list.h"
@@ -45,7 +46,7 @@ class AppSourceUrlRecorder {
   static SourceId GetSourceIdForPWA(const GURL& url);
 
   // For internal use only.
-  static SourceId GetSourceIdForUrl(const GURL& url);
+  static SourceId GetSourceIdForUrl(const GURL& url, const AppType);
 };
 
 }  // namespace ukm

components/ukm/ukm_recorder_impl.cc

@@ -540,8 +540,10 @@ void UkmRecorderImpl::UpdateSourceURL(SourceId source_id,
   RecordSource(std::make_unique<UkmSource>(source_id, sanitized_url));
 }
 
-void UkmRecorderImpl::UpdateAppURL(SourceId source_id, const GURL& url) {
-  if (!extensions_enabled_) {
+void UkmRecorderImpl::UpdateAppURL(SourceId source_id,
+                                   const GURL& url,
+                                   const AppType app_type) {
+  if (app_type != AppType::kPWA && !extensions_enabled_) {
     RecordDroppedSource(DroppedDataReason::EXTENSION_URLS_DISABLED);
     return;
   }

components/ukm/ukm_recorder_impl.h

@@ -117,7 +117,9 @@ class UkmRecorderImpl : public UkmRecorder {
   // UkmRecorder:
   void AddEntry(mojom::UkmEntryPtr entry) override;
   void UpdateSourceURL(SourceId source_id, const GURL& url) override;
-  void UpdateAppURL(SourceId source_id, const GURL& url) override;
+  void UpdateAppURL(SourceId source_id,
+                    const GURL& url,
+                    const AppType app_type) override;
   void RecordNavigation(
       SourceId source_id,
       const UkmSource::NavigationData& navigation_data) override;

services/metrics/public/cpp/delegating_ukm_recorder.cc

@@ -65,14 +65,16 @@ void DelegatingUkmRecorder::RecordNavigation(
   }
 }
 
-void DelegatingUkmRecorder::UpdateAppURL(SourceId source_id, const GURL& url) {
+void DelegatingUkmRecorder::UpdateAppURL(SourceId source_id,
+                                         const GURL& url,
+                                         const AppType app_type) {
   if (GetSourceIdType(source_id) != SourceIdType::APP_ID) {
     DLOG(FATAL) << "UpdateAppURL invoked for non-APP_ID SourceId";
     return;
   }
   base::AutoLock auto_lock(lock_);
   for (auto& iterator : delegates_)
-    iterator.second.UpdateAppURL(source_id, url);
+    iterator.second.UpdateAppURL(source_id, url, app_type);
 }
 
 void DelegatingUkmRecorder::AddEntry(mojom::UkmEntryPtr entry) {
@@ -115,13 +117,15 @@ void DelegatingUkmRecorder::Delegate::UpdateSourceURL(ukm::SourceId source_id,
 }
 
 void DelegatingUkmRecorder::Delegate::UpdateAppURL(ukm::SourceId source_id,
-                                                   const GURL& url) {
+                                                   const GURL& url,
+                                                   const AppType app_type) {
   if (task_runner_->RunsTasksInCurrentSequence()) {
-    ptr_->UpdateAppURL(source_id, url);
+    ptr_->UpdateAppURL(source_id, url, app_type);
     return;
   }
-  task_runner_->PostTask(FROM_HERE, base::BindOnce(&UkmRecorder::UpdateAppURL,
-                                                   ptr_, source_id, url));
+  task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&UkmRecorder::UpdateAppURL, ptr_, source_id,
+                                url, app_type));
 }
 
 void DelegatingUkmRecorder::Delegate::RecordNavigation(

services/metrics/public/cpp/delegating_ukm_recorder.h

@@ -49,7 +49,9 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder {
 
   // UkmRecorder:
   void UpdateSourceURL(SourceId source_id, const GURL& url) override;
-  void UpdateAppURL(SourceId source_id, const GURL& url) override;
+  void UpdateAppURL(SourceId source_id,
+                    const GURL& url,
+                    const AppType app_type) override;
   void RecordNavigation(
       SourceId source_id,
       const UkmSource::NavigationData& navigation_data) override;
@@ -64,7 +66,9 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder {
     ~Delegate();
 
     void UpdateSourceURL(SourceId source_id, const GURL& url);
-    void UpdateAppURL(SourceId source_id, const GURL& url);
+    void UpdateAppURL(SourceId source_id,
+                      const GURL& url,
+                      const AppType app_type);
     void RecordNavigation(SourceId source_id,
                           const UkmSource::NavigationData& navigation_data);
     void AddEntry(mojom::UkmEntryPtr entry);

services/metrics/public/cpp/mojo_ukm_recorder.cc

@@ -23,7 +23,9 @@ void MojoUkmRecorder::UpdateSourceURL(SourceId source_id, const GURL& url) {
   interface_->UpdateSourceURL(source_id, url.spec());
 }
 
-void MojoUkmRecorder::UpdateAppURL(SourceId source_id, const GURL& url) {
+void MojoUkmRecorder::UpdateAppURL(SourceId source_id,
+                                   const GURL& url,
+                                   const AppType app_type) {
   NOTREACHED();
 }
 

services/metrics/public/cpp/mojo_ukm_recorder.h

@@ -45,7 +45,9 @@ class METRICS_EXPORT MojoUkmRecorder : public UkmRecorder {
  private:
   // UkmRecorder:
   void UpdateSourceURL(SourceId source_id, const GURL& url) override;
-  void UpdateAppURL(SourceId source_id, const GURL& url) override;
+  void UpdateAppURL(SourceId source_id,
+                    const GURL& url,
+                    const AppType app_type) override;
   void RecordNavigation(
       SourceId source_id,
       const UkmSource::NavigationData& navigation_data) override;

services/metrics/public/cpp/ukm_recorder.cc

@@ -56,8 +56,10 @@ void UkmRecorder::RecordOtherURL(base::UkmSourceId source_id, const GURL& url) {
   UpdateSourceURL(source_id.ToInt64(), url);
 }
 
-void UkmRecorder::RecordAppURL(base::UkmSourceId source_id, const GURL& url) {
-  UpdateAppURL(source_id.ToInt64(), url);
+void UkmRecorder::RecordAppURL(base::UkmSourceId source_id,
+                               const GURL& url,
+                               const AppType app_type) {
+  UpdateAppURL(source_id.ToInt64(), url, app_type);
 }
 
 }  // namespace ukm

services/metrics/public/cpp/ukm_recorder.h

@@ -38,6 +38,12 @@ class DelegatingUkmRecorder;
 class TestRecordingHelper;
 class UkmBackgroundRecorderService;
 
+enum class AppType {
+  kArc,
+  kPWA,
+  kExtension,
+};
+
 namespace internal {
 class SourceUrlRecorderWebContentsObserver;
 }  // namespace internal
@@ -70,7 +76,9 @@ class METRICS_EXPORT UkmRecorder {
  protected:
   // Type-safe wrappers for Update<X> functions.
   void RecordOtherURL(base::UkmSourceId source_id, const GURL& url);
-  void RecordAppURL(base::UkmSourceId source_id, const GURL& url);
+  void RecordAppURL(base::UkmSourceId source_id,
+                    const GURL& url,
+                    const AppType app_type);
 
   // Gets new source Id for WEBAPK_ID type and updates the manifest url. This
   // method should only be called by WebApkUkmRecorder class.
@@ -106,7 +114,9 @@ class METRICS_EXPORT UkmRecorder {
 
   // Associates the SourceId with an app URL for APP_ID sources. This method
   // should only be called by AppSourceUrlRecorder and DelegatingUkmRecorder.
-  virtual void UpdateAppURL(SourceId source_id, const GURL& url) = 0;
+  virtual void UpdateAppURL(SourceId source_id,
+                            const GURL& url,
+                            const AppType app_type) = 0;
 
   // Associates navigation data with the UkmSource keyed by |source_id|. This
   // should only be called by SourceUrlRecorderWebContentsObserver, for