Revert of Profile_Metrics integration with Keystone (patchset #20 id:530001 of...

Revert of Profile_Metrics integration with Keystone (patchset #20 id:530001 of https://codereview.chromium.org/593243002/)

Reason for revert:
crbug.com/463900. Mac crashed, like, all the time.

Original issue's description:
> Profile_Metrics integration with Keystone
>
> Chromium side of the implementation of tracking profile stats (number of profiles, number of signed in profiles) through the Omaha channel for Mac. Design doc: https://docs.google.com/a/google.com/document/d/15Ou8VCYNCZvke8Bw9bF3vYqD67voJBPyx_GBR8ONcCw/edit?disco=AAAAAKU7Zzg (Googler Only)
>
> BUG=409895
>
> Committed: https://crrev.com/0bcaa8e521eae39a81b4754f8b4ac9de1f0463d5
> Cr-Commit-Position: refs/heads/master@{#318729}

TBR=bcwhite@chromium.org,borisv@chromium.org,mark@chromium.org,grt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=409895

Review URL: https://codereview.chromium.org/974423002

Cr-Commit-Position: refs/heads/master@{#319125}

chrome/browser/mac/keystone_glue.h

@@ -87,7 +87,6 @@ enum BrandFileType {
   // And the Keystone registration itself, with the active timer
   KSRegistration* registration_;  // strong
   NSTimer* timer_;  // strong
-  Class ksUnsignedReportingAttributeClass_;
 
   // The most recent kAutoupdateStatusNotification notification posted.
   base::scoped_nsobject<NSNotification> recentNotification_;
@@ -102,10 +101,6 @@ enum BrandFileType {
 
   // YES if an update was ever successfully installed by -installUpdate.
   BOOL updateSuccessfullyInstalled_;
-
-  // Profile count information.
-  uint32_t numProfiles_;
-  uint32_t numSignedInProfiles_;
 }
 
 // Return the default Keystone Glue object.
@@ -177,10 +172,6 @@ enum BrandFileType {
 // at the installed copy.
 - (void)setAppPath:(NSString*)appPath;
 
-// Sets the total number of profiles and the number of signed in profiles.
-- (void)updateProfileCountsWithNumProfiles:(uint32_t)profiles
-                       numSignedInProfiles:(uint32_t)signedInProfiles;
-
 @end  // @interface KeystoneGlue
 
 @interface KeystoneGlue(ExposedForTesting)

chrome/browser/mac/keystone_glue.mm

@@ -119,9 +119,6 @@ class PerformBridge : public base::RefCountedThreadSafe<PerformBridge> {
 // Called when Keystone registration completes.
 - (void)registrationComplete:(NSNotification*)notification;
 
-// Set the registration active and pass profile count parameters.
-- (void)setRegistrationActive;
-
 // Called periodically to announce activity by pinging the Keystone server.
 - (void)markActive:(NSTimer*)timer;
 
@@ -448,8 +445,6 @@ NSString* const kVersionKey = @"KSVersion";
     return NO;
 
   registration_ = [ksr retain];
-  ksUnsignedReportingAttributeClass_ =
-      [ksrBundle classNamed:@"KSUnsignedReportingAttribute"];
   return YES;
 }
 
@@ -496,47 +491,6 @@ NSString* const kVersionKey = @"KSVersion";
              nil];
 }
 
-- (void)setRegistrationActive {
-  if (!registration_)
-    return;
-
-  // Should never have zero profiles. Do not report this value.
-  if (!numProfiles_) {
-    [registration_ setActive];
-    return;
-  }
-
-  NSError* reportingError = nil;
-
-  KSReportingAttribute* numAccountsAttr =
-      [ksUnsignedReportingAttributeClass_
-          reportingAttributeWithValue:numProfiles_
-                                 name:@"_NumAccounts"
-                      aggregationType:kKSReportingAggregationSum
-                                error:&reportingError];
-  if (reportingError != nil)
-    VLOG(1) << [reportingError localizedDescription];
-  reportingError = nil;
-
-  KSReportingAttribute* numSignedInAccountsAttr =
-      [ksUnsignedReportingAttributeClass_
-          reportingAttributeWithValue:numSignedInProfiles_
-                                 name:@"_NumSignedIn"
-                      aggregationType:kKSReportingAggregationSum
-                                error:&reportingError];
-  if (reportingError != nil)
-    VLOG(1) << [reportingError localizedDescription];
-  reportingError = nil;
-
-  NSArray* profileCountsInformation =
-      [NSArray arrayWithObjects:numAccountsAttr, numSignedInAccountsAttr, nil];
-
-  if (![registration_ setActiveWithReportingAttributes:profileCountsInformation
-                                                 error:&reportingError]) {
-    VLOG(1) << [reportingError localizedDescription];
-  }
-}
-
 - (void)registerWithKeystone {
   [self updateStatus:kAutoupdateRegistering version:nil];
 
@@ -558,13 +512,13 @@ NSString* const kVersionKey = @"KSVersion";
   // posted, and -registrationComplete: will be called.
 
   // Mark an active RIGHT NOW; don't wait an hour for the first one.
-  [self setRegistrationActive];
+  [registration_ setActive];
 
   // Set up hourly activity pings.
   timer_ = [NSTimer scheduledTimerWithTimeInterval:60 * 60  // One hour
                                             target:self
                                           selector:@selector(markActive:)
-                                          userInfo:nil
+                                          userInfo:registration_
                                            repeats:YES];
 }
 
@@ -587,7 +541,8 @@ NSString* const kVersionKey = @"KSVersion";
 }
 
 - (void)markActive:(NSTimer*)timer {
-  [self setRegistrationActive];
+  KSRegistration* ksr = [timer userInfo];
+  [ksr setActive];
 }
 
 - (void)checkForUpdate {
@@ -1094,13 +1049,6 @@ NSString* const kVersionKey = @"KSVersion";
   return tagSuffix;
 }
 
-
-- (void)updateProfileCountsWithNumProfiles:(uint32_t)profiles
-                       numSignedInProfiles:(uint32_t)signedInProfiles {
-  numProfiles_ = profiles;
-  numSignedInProfiles_ = signedInProfiles;
-}
-
 @end  // @implementation KeystoneGlue
 
 namespace {

chrome/browser/mac/keystone_glue_unittest.mm

@@ -39,11 +39,6 @@ namespace ksr = keystone_registration;
   return NO;
 }
 
-- (BOOL)setActiveWithReportingAttributes:(NSArray*)reportingAttributes
-                                   error:(NSError**)error {
-  return NO;
-}
-
 - (void)checkForUpdateWasUserInitiated:(BOOL)userInitiated {
 }
 

chrome/browser/mac/keystone_registration.h

@@ -48,19 +48,10 @@ extern NSString* KSUpdateCheckSuccessfulKey;
 extern NSString* KSUpdateCheckSuccessfullyInstalledKey;
 
 extern NSString* KSRegistrationRemoveExistingTag;
-
-extern NSString* KSReportingAttributeValueKey;
-extern NSString* KSReportingAttributeExpirationDateKey;
-extern NSString* KSReportingAttributeAggregationTypeKey;
 #define KSRegistrationPreserveExistingTag nil
 
 }  // namespace keystone_registration
 
-typedef enum {
-  kKSReportingAggregationSum = 0,  // Adds attribute value across user accounts
-  kKSReportingAggregationDefault = kKSReportingAggregationSum,
-} KSReportingAggregationType;
-
 @interface KSRegistration : NSObject
 
 + (id)registrationWithProductID:(NSString*)productID;
@@ -71,30 +62,10 @@ typedef enum {
                 authorization:(AuthorizationRef)authorization;
 
 - (BOOL)setActive;
-- (BOOL)setActiveWithReportingAttributes:(NSArray*)reportingAttributes
-                                   error:(NSError**)error;
 - (void)checkForUpdateWasUserInitiated:(BOOL)userInitiated;
 - (void)startUpdate;
 - (keystone_registration::KSRegistrationTicketType)ticketType;
 
 @end  // @interface KSRegistration
 
-
-// Declarations of the Keystone attribute reporting bits needed here.
-// Full definition is at:
-// //depot/googlemac/opensource/update-engine/Common/KSReportingAttribute.h
-@interface KSReportingAttribute : NSObject
-
-@end  // @interface KSReportingAttribute
-
-@interface KSUnsignedReportingAttribute : KSReportingAttribute
-
-+ (KSUnsignedReportingAttribute *)reportingAttributeWithValue:(uint32_t)value
-               name:(NSString *)name
-    aggregationType:(KSReportingAggregationType)aggregationType
-              error:(NSError **)error;
-
-@end  // @interface KSUnsignedReportingAttribute
-
-
 #endif  // CHROME_BROWSER_MAC_KEYSTONE_REGISTRATION_H_

chrome/browser/mac/keystone_registration.mm

@@ -39,8 +39,4 @@ NSString* KSUpdateCheckSuccessfullyInstalledKey = @"SuccessfullyInstalled";
 
 NSString* KSRegistrationRemoveExistingTag = @"";
 
-NSString* KSReportingAttributeValueKey = @"value";
-NSString* KSReportingAttributeExpirationDateKey = @"expiresAt";
-NSString* KSReportingAttributeAggregationTypeKey = @"aggregation";
-
 }  // namespace keystone_registration

chrome/browser/profiles/profile_metrics.cc

@@ -20,10 +20,7 @@
 
 namespace {
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
 const int kMaximumReportedProfileCount = 5;
-#endif
-
 const int kMaximumDaysOfDisuse = 4 * 7;  // Should be integral number of weeks.
 
 size_t number_of_profile_switches_ = 0;
@@ -70,6 +67,12 @@ ProfileMetrics::ProfileType GetProfileType(
   return metric;
 }
 
+void UpdateReportedOSProfileStatistics(int active, int signedin) {
+#if defined(OS_WIN)
+  GoogleUpdateSettings::UpdateProfileCounts(active, signedin);
+#endif
+}
+
 void LogLockedProfileInformation(ProfileManager* manager) {
   const ProfileInfoCache& info_cache = manager->GetProfileInfoCache();
   size_t number_of_profiles = info_cache.GetNumberOfProfiles();
@@ -174,11 +177,10 @@ bool ProfileMetrics::CountProfileInformation(ProfileManager* manager,
 }
 
 void ProfileMetrics::UpdateReportedProfilesStatistics(ProfileManager* manager) {
-#if defined(OS_WIN) || defined(OS_MACOSX)
   ProfileCounts counts;
   if (CountProfileInformation(manager, &counts)) {
-    size_t limited_total = counts.total;
-    size_t limited_signedin = counts.signedin;
+    int limited_total = counts.total;
+    int limited_signedin = counts.signedin;
     if (limited_total > kMaximumReportedProfileCount) {
       limited_total = kMaximumReportedProfileCount + 1;
       limited_signedin =
@@ -187,7 +189,6 @@ void ProfileMetrics::UpdateReportedProfilesStatistics(ProfileManager* manager) {
     }
     UpdateReportedOSProfileStatistics(limited_total, limited_signedin);
   }
-#endif
 }
 
 void ProfileMetrics::LogNumberOfProfileSwitches() {
@@ -195,14 +196,6 @@ void ProfileMetrics::LogNumberOfProfileSwitches() {
                            number_of_profile_switches_);
 }
 
-// The OS_MACOSX implementation of this function is in profile_metrics_mac.mm.
-#if defined(OS_WIN)
-void ProfileMetrics::UpdateReportedOSProfileStatistics(
-    size_t active, size_t signedin) {
-  GoogleUpdateSettings::UpdateProfileCounts(active, signedin);
-}
-#endif
-
 void ProfileMetrics::LogNumberOfProfiles(ProfileManager* manager) {
   ProfileCounts counts;
   bool success = CountProfileInformation(manager, &counts);
@@ -224,10 +217,7 @@ void ProfileMetrics::LogNumberOfProfiles(ProfileManager* manager) {
                              counts.auth_errors);
 
     LogLockedProfileInformation(manager);
-
-#if defined(OS_WIN) || defined(OS_MACOSX)
     UpdateReportedOSProfileStatistics(counts.total, counts.signedin);
-#endif
   }
 }
 

chrome/browser/profiles/profile_metrics.h

@@ -208,11 +208,6 @@ class ProfileMetrics {
                                       ProfileCounts* counts);
 
   static void LogNumberOfProfileSwitches();
-#if defined(OS_WIN) || defined(OS_MACOSX)
-  // Update OS level tracking of profile counts.
-  static void UpdateReportedOSProfileStatistics(size_t active, size_t signedin);
-#endif
-
   static void LogNumberOfProfiles(ProfileManager* manager);
   static void LogProfileAddNewUser(ProfileAdd metric);
   static void LogProfileAvatarSelection(size_t icon_index);

chrome/browser/profiles/profile_metrics_mac.mm

-// Copyright 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/profiles/profile_metrics.h"
-
-#include "base/numerics/safe_conversions.h"
-#include "chrome/browser/mac/keystone_glue.h"
-
-void ProfileMetrics::UpdateReportedOSProfileStatistics(
-    size_t active, size_t signedin) {
-  if (base::IsValueInRangeForNumericType<uint32_t>(active) &&
-      base::IsValueInRangeForNumericType<uint32_t>(signedin)) {
-    [[KeystoneGlue defaultKeystoneGlue]
-        updateProfileCountsWithNumProfiles:active
-                       numSignedInProfiles:signedin];
-  }
-}

chrome/chrome_browser.gypi

@@ -2264,7 +2264,6 @@
       'browser/profiles/profile_metrics.cc',
       'browser/profiles/profile_metrics.h',
       'browser/profiles/profile_metrics_list.h',
-      'browser/profiles/profile_metrics_mac.mm',
       'browser/profiles/profile_shortcut_manager_win.cc',
       'browser/profiles/profile_shortcut_manager_win.h',
       'browser/profiles/profiles_state.cc',

chrome/installer/util/google_update_settings.cc

@@ -5,7 +5,6 @@
 #include "chrome/installer/util/google_update_settings.h"
 
 #include <algorithm>
-#include <limits>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -71,7 +70,7 @@ bool ReadGoogleUpdateStrKey(const wchar_t* const name, base::string16* value) {
 bool WriteGoogleUpdateAggregateNumKeyInternal(
     const AppRegistrationData& app_reg_data,
     const wchar_t* const name,
-    size_t value,
+    int value,
     const wchar_t* const aggregate) {
   DCHECK(aggregate);
   DCHECK(GoogleUpdateSettings::IsSystemInstall());
@@ -91,11 +90,7 @@ bool WriteGoogleUpdateAggregateNumKeyInternal(
   reg_path.append(name);
   RegKey key(HKEY_LOCAL_MACHINE, reg_path.c_str(), kAccess);
   key.WriteValue(google_update::kRegAggregateMethod, aggregate);
-
-  DWORD dword_value = (value > std::numeric_limits<DWORD>::max() ?
-      std::numeric_limits<DWORD>::max() :
-      static_cast<DWORD>(value));
-  return (key.WriteValue(uniquename.c_str(), dword_value) == ERROR_SUCCESS);
+  return (key.WriteValue(uniquename.c_str(), value) == ERROR_SUCCESS);
 }
 
 // Updates a registry key |name| to be |value| for the given |app_reg_data|.
@@ -551,8 +546,8 @@ bool GoogleUpdateSettings::UpdateGoogleUpdateApKey(
   return modified;
 }
 
-void GoogleUpdateSettings::UpdateProfileCounts(size_t profiles_active,
-                                               size_t profiles_signedin) {
+void GoogleUpdateSettings::UpdateProfileCounts(int profiles_active,
+                                               int profiles_signedin) {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   // System-level installs must write into the ClientStateMedium key shared by
   // all users. Special treatment is used to aggregate across those users.
@@ -574,10 +569,10 @@ void GoogleUpdateSettings::UpdateProfileCounts(size_t profiles_active,
     // user-level installs.
     WriteGoogleUpdateStrKeyInternal(dist->GetAppRegistrationData(),
                                     google_update::kRegProfilesActive,
-                                    base::SizeTToString16(profiles_active));
+                                    base::IntToString16(profiles_active));
     WriteGoogleUpdateStrKeyInternal(dist->GetAppRegistrationData(),
                                     google_update::kRegProfilesSignedIn,
-                                    base::SizeTToString16(profiles_signedin));
+                                    base::IntToString16(profiles_signedin));
   }
 }
 

chrome/installer/util/google_update_settings.h

@@ -218,8 +218,7 @@ class GoogleUpdateSettings {
 
   // This method updates the values that report how many profiles are in use
   // and how many of those are signed-in.
-  static void UpdateProfileCounts(size_t profiles_active,
-                                  size_t profiles_signedin);
+  static void UpdateProfileCounts(int profiles_active, int profiles_signedin);
 
   // For system-level installs, we need to be able to communicate the results
   // of the Toast Experiments back to Google Update. The problem is just that