From c2239a84ca03a01bb95d6055d2a6c75b767979fb Mon Sep 17 00:00:00 2001 From: treib Date: Fri, 12 Jun 2015 00:53:55 -0700 Subject: [PATCH] Remove ExtensionPermission2* histograms. They've been replaced by ExtensionPermission3*. BUG=484102 Review URL: https://codereview.chromium.org/1164503005 Cr-Commit-Position: refs/heads/master@{#334133} --- chrome/browser/extensions/extension_service.cc | 17 ---- extensions/common/PRESUBMIT.py | 49 ++++------- extensions/common/{ => permissions}/PRESUBMIT.py | 16 ++-- extensions/common/permissions/permission_message.h | 4 +- tools/metrics/histograms/histograms.xml | 96 ++++++++++++---------- .../histograms/update_extension_permission.py | 11 +-- 6 files changed, 76 insertions(+), 117 deletions(-) rewrite extensions/common/PRESUBMIT.py (66%) copy extensions/common/{ => permissions}/PRESUBMIT.py (71%) diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index fb604df1619d..16c3a6b5a46f 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -1018,23 +1018,6 @@ void ExtensionService::RecordPermissionMessagesHistogram( const Extension* extension, const char* histogram) { // Since this is called from multiple sources, and since the histogram macros // use statics, we need to manually lookup the histogram ourselves. - base::HistogramBase* legacy_counter = base::LinearHistogram::FactoryGet( - base::StringPrintf("Extensions.Permissions_%s2", histogram), - 1, - PermissionMessage::kEnumBoundary, - PermissionMessage::kEnumBoundary + 1, - base::HistogramBase::kUmaTargetedHistogramFlag); - - // TODO(treib): Remove the legacy "2" histograms. See crbug.com/484102. - PermissionMessageIDs legacy_permissions = - extension->permissions_data()->GetLegacyPermissionMessageIDs(); - if (legacy_permissions.empty()) { - legacy_counter->Add(PermissionMessage::kNone); - } else { - for (PermissionMessage::ID id : legacy_permissions) - legacy_counter->Add(id); - } - base::HistogramBase* counter = base::LinearHistogram::FactoryGet( base::StringPrintf("Extensions.Permissions_%s3", histogram), 1, diff --git a/extensions/common/PRESUBMIT.py b/extensions/common/PRESUBMIT.py dissimilarity index 66% index cc49fc07ddf6..16c19465c429 100644 --- a/extensions/common/PRESUBMIT.py +++ b/extensions/common/PRESUBMIT.py @@ -1,35 +1,14 @@ -# 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. - -"""Chromium presubmit script for src/extensions/common. - -See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts -for more details on the presubmit API built into depot_tools. -""" - -import sys - -def _CreateAPIPermissionIDChecker(input_api, output_api): - original_sys_path = sys.path - - try: - sys.path.append(input_api.os_path.join( - input_api.PresubmitLocalPath(), '..', '..', 'tools', - 'strict_enum_value_checker')) - from strict_enum_value_checker import StrictEnumValueChecker - finally: - sys.path = original_sys_path - - return StrictEnumValueChecker(input_api, output_api, - start_marker=' enum ID {', end_marker=' // Last entry:', - path='extensions/common/permissions/api_permission.h') - -def CheckChangeOnUpload(input_api, output_api): - results = [] - results += _CreateAPIPermissionIDChecker(input_api, output_api).Run() - results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api) - return results - -def CheckChangeOnCommit(input_api, output_api): - return _CreateAPIPermissionIDChecker(input_api, output_api).Run() +# 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. + +"""Chromium presubmit script for src/extensions/common. + +See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts +for more details on the presubmit API built into depot_tools. +""" + +import sys + +def CheckChangeOnUpload(input_api, output_api): + return input_api.canned_checks.CheckPatchFormatted(input_api, output_api) diff --git a/extensions/common/PRESUBMIT.py b/extensions/common/permissions/PRESUBMIT.py similarity index 71% copy from extensions/common/PRESUBMIT.py copy to extensions/common/permissions/PRESUBMIT.py index cc49fc07ddf6..421841c30091 100644 --- a/extensions/common/PRESUBMIT.py +++ b/extensions/common/permissions/PRESUBMIT.py @@ -1,21 +1,23 @@ -# Copyright 2015 The Chromium Authors. All rights reserved. +# Copyright 2014 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. -"""Chromium presubmit script for src/extensions/common. +"""Chromium presubmit script for src/extensions/common/permissions. See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts for more details on the presubmit API built into depot_tools. """ - import sys +def GetPreferredTrySlaves(): + return ['linux_chromeos'] + def _CreateAPIPermissionIDChecker(input_api, output_api): original_sys_path = sys.path try: sys.path.append(input_api.os_path.join( - input_api.PresubmitLocalPath(), '..', '..', 'tools', + input_api.PresubmitLocalPath(), '..', '..', '..', 'tools', 'strict_enum_value_checker')) from strict_enum_value_checker import StrictEnumValueChecker finally: @@ -26,10 +28,8 @@ def _CreateAPIPermissionIDChecker(input_api, output_api): path='extensions/common/permissions/api_permission.h') def CheckChangeOnUpload(input_api, output_api): - results = [] - results += _CreateAPIPermissionIDChecker(input_api, output_api).Run() - results += input_api.canned_checks.CheckPatchFormatted(input_api, output_api) - return results + return _CreateAPIPermissionIDChecker(input_api, output_api).Run() def CheckChangeOnCommit(input_api, output_api): return _CreateAPIPermissionIDChecker(input_api, output_api).Run() + diff --git a/extensions/common/permissions/permission_message.h b/extensions/common/permissions/permission_message.h index a4e62b569cdb..d95af255c73b 100644 --- a/extensions/common/permissions/permission_message.h +++ b/extensions/common/permissions/permission_message.h @@ -109,9 +109,7 @@ class PermissionMessage { kAutofillPrivate, kPasswordsPrivate, kUsersPrivate, - // Last entry: Add new entries above and ensure to update the - // "ExtensionPermission2" enum in tools/metrics/histograms/histograms.xml - // (by running update_extension_permission.py). + // Last entry: Add new entries above. kEnumBoundary, }; static_assert(PermissionMessage::kNone > PermissionMessage::kUnknown, diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 35a8795dd151..1dd93e88c1f7 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -10657,15 +10657,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when it is automatically disabled - due to a permission increase (e.g., after an extension upgrade). To find - places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - AutoDisable. + due to a permission increase (e.g., after an extension upgrade). + + Deprecated as of 6/2015, replaced by Extensions.Permissions_AutoDisable3. + kalman@chromium.org rpaquay@chromium.org @@ -10695,14 +10695,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. kalman@chromium.org rpaquay@chromium.org - The permissions present in an extension when it was installed. To find - places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - Install. + The permissions present in an extension when it was installed. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_Install3. + kalman@chromium.org rpaquay@chromium.org @@ -10730,15 +10730,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when installation was aborted, not - including installation errors and user cancels. To find places where this - histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - InstallAbort. + including installation errors and user cancels. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_InstallAbort3. + kalman@chromium.org rpaquay@chromium.org @@ -10769,15 +10769,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. kalman@chromium.org rpaquay@chromium.org - The permissions present in an extension when installation was canceled. To - find places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - InstallCancel. + The permissions present in an extension when installation was canceled. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_InstallCancel3. + kalman@chromium.org rpaquay@chromium.org @@ -10807,13 +10807,12 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_Load3. + kalman@chromium.org rpaquay@chromium.org - - The permissions present in an extension when it was loaded. To find places - where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument Load. - + The permissions present in an extension when it was loaded. @@ -10835,13 +10834,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when it was re-enabled from a - confirmation prompt. To find places where this histogram may be emitted, - look for calls to ExtensionService::RecordPermissionMessagesHistogram with - the argument ReEnable. + confirmation prompt. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_ReEnable3. + kalman@chromium.org rpaquay@chromium.org @@ -10870,15 +10870,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when the re-enable prompt was - aborted, not including installation errors and manual user cancels. To find - places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - ReEnableAbort. + aborted, not including installation errors and manual user cancels. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_ReEnableAbort3. + kalman@chromium.org rpaquay@chromium.org @@ -10910,14 +10910,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when the re-enable was canceled from - the confirmation prompt. To find places where this histogram may be emitted, - look for calls to ExtensionService::RecordPermissionMessagesHistogram with - the argument ReEnableCancel. + the confirmation prompt. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_ReEnableCancel3. + kalman@chromium.org rpaquay@chromium.org @@ -10945,14 +10946,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. kalman@chromium.org rpaquay@chromium.org - The permissions present in an extension when it was uninstalled. To find - places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - Uninstall. + The permissions present in an extension when it was uninstalled. + + Deprecated as of 6/2015, replaced by Extensions.Permissions_Uninstall3. + kalman@chromium.org rpaquay@chromium.org @@ -10981,14 +10982,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when it was installed through the - web store. To find places where this histogram may be emitted, look for - calls to ExtensionService::RecordPermissionMessagesHistogram with the - argument WebStoreInstall. + web store. + + Deprecated as of 6/2015, replaced by + Extensions.Permissions_WebStoreInstall3. + kalman@chromium.org rpaquay@chromium.org @@ -11020,15 +11023,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when installation from the web store - was aborted, not including installation errors and user cancels. To find - places where this histogram may be emitted, look for calls to - ExtensionService::RecordPermissionMessagesHistogram with the argument - WebStoreInstallAbort. + was aborted, not including installation errors and user cancels. + + Deprecated as of 6/2015, replaced by + Extensions.Permissions_WebStoreInstallAbort3. + kalman@chromium.org rpaquay@chromium.org @@ -11061,14 +11065,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. rpaquay@chromium.org The permissions present in an extension when installation from the web store - was canceled. To find places where this histogram may be emitted, look for - calls to ExtensionService::RecordPermissionMessagesHistogram with the - argument WebStoreInstallCancel. + was canceled. + + Deprecated as of 6/2015, replaced by + Extensions.Permissions_WebStoreInstallCancel3. + kalman@chromium.org rpaquay@chromium.org diff --git a/tools/metrics/histograms/update_extension_permission.py b/tools/metrics/histograms/update_extension_permission.py index 4e80249fe8e9..85073f98b529 100644 --- a/tools/metrics/histograms/update_extension_permission.py +++ b/tools/metrics/histograms/update_extension_permission.py @@ -2,9 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -"""Updates ExtensionPermission2 and ExtensionPermission3 enums in histograms.xml -file with values read from permission_message.h and api_permission.h, -respectively. +"""Updates the ExtensionPermission3 enum in the histograms.xml file with values +read from api_permission.h. If the file was pretty-printed, the updated version is pretty-printed too. """ @@ -20,12 +19,6 @@ if __name__ == '__main__': sys.stderr.write(__doc__) sys.exit(1) - header_file = 'extensions/common/permissions/permission_message.h' - UpdateHistogramEnum(histogram_enum_name='ExtensionPermission2', - source_enum_path=header_file, - start_marker='^enum ID {', - end_marker='^kEnumBoundary') - header_file = 'extensions/common/permissions/api_permission.h' UpdateHistogramEnum(histogram_enum_name='ExtensionPermission3', source_enum_path=header_file, -- 2.11.4.GIT