Add UMA logging of extension creation flags.

chrome/browser/extensions/installed_loader.cc

 // Copyright (c) 2012 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/extensions/installed_loader.h"
 
 #include "base/files/file_path.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 96 matching lines @@
 }
 
 BackgroundPageType GetBackgroundPageType(const Extension* extension) {
   if (!BackgroundInfo::HasBackgroundPage(extension))
     return NO_BACKGROUND_PAGE;
   if (BackgroundInfo::HasPersistentBackgroundPage(extension))
     return BACKGROUND_PAGE_PERSISTENT;
   return EVENT_PAGE;
 }
 
+// Records the creation flags of an extension grouped by
+// Extension::InitFromValueFlags.
+void RecordCreationFlags(const Extension* extension) {
+  // Since each flag is a separate bit in an int, this loop is needed to find
+  // the number of flags.
+  int highest_flag_bit = 0;
+  for (int i = 1; i < Extension::LAST_INIT_FROM_VALUE_FLAGS; i <<= 1)
+    ++highest_flag_bit;

[jar (doing other things), 2014/04/29 16:25:56]

I didn't read this section of the code carefully...

As per your off-line comments.... this "would be" calculation does generate a
constant... and even is often optimized away by the compiler.

It would be much clearer to have a constant value here, rather than having to
read code to deduce that a const value is generated.

...and in fact...

Looking at your enums.... I can see that 
LAST_INIT_FROM_VALUE_FLAGS == 1 << 10

As a result, the loop will be executed a total of 9 times, and create the
constant:
 higest_flag_bit == 9.

...which does not seem to be as intended, and leads to further problems below.

It would be much simpler (clearer, and correct) to use a constant here.

[calamity, 2014/05/12 03:39:10]

Done.

+
+  int flag = 1;
+  for (int i = 1; i <= highest_flag_bit; ++i) {

[jar (doing other things), 2014/04/29 16:25:56]

Since higest_flag_bit == 9, this loop will only record samples for 
i == 1 ... 9

In contrast, I noted that you have a total 11 enums 
{ 1 << 0, ..., 1 << 10 }

As a result, you'll only test 9 of the 11 flags.

+    if (extension->creation_flags() & flag) {

[jar (doing other things), 2014/04/29 16:25:56]

Since you're only looking at 9 choices for |flag|, and the first choice is |1 <<
0|, it appears that you'll only check for flags 
{1 << 0, ... 1 << 8}

But then you'll populate the histogram with samples {1, ... 9}.
Looking at the xml file, it appears that a sample of 0 should have corresponded
to REQUIRED_KEY == 1 << 0, but you never have an i==0 recorded in the histogram.
The sample of 1 corresponds to a the first iteration through the loop, and hence
that records when the flag 1 << 0 is used.

I don't think this is as intended.

+      UMA_HISTOGRAM_ENUMERATION(
+          "Extensions.LoadCreationFlags", i, highest_flag_bit + 1);

[jar (doing other things), 2014/04/29 01:32:25]

Other than |i| (which is the sample), the other arguments act as "definitions"
for this histogtram. As a result, you can't change them dynamically (either via
the macro, or by breaking-open the macro, and calling FactoryGet(...).)

Please try to define the histogram so that |higest_flag_bit| is a constant in
the program.  Most typically, it is a bounding value in an enumeration.

Reading the code, I *think* this is meant to be 11 or 12.  You should probably
define the list in an enumeration, and then expand it in the flag definition
(where you currently have the existing enum) using those enumerated values from
0-10 as "shift values" for defining your flags.  YMMV.

...but you probably shouldn't try to calculate the highest flag bit
*dynamically* found in a word.  That will give differing definitions, and mess
up the histogram.

+    }
+    flag <<= 1;
+  }
+}
+
 }  // namespace
 
 InstalledLoader::InstalledLoader(ExtensionService* extension_service)
     : extension_service_(extension_service),
       extension_registry_(ExtensionRegistry::Get(extension_service->profile())),
       extension_prefs_(ExtensionPrefs::Get(extension_service->profile())) {}
 
 InstalledLoader::~InstalledLoader() {
 }
 
@@ skipping 269 matching lines @@
     ExtensionActionManager* extension_action_manager =
         ExtensionActionManager::Get(extension_service_->profile());
     if (extension_action_manager->GetPageAction(*ex->get()))
       ++page_action_count;
     if (extension_action_manager->GetBrowserAction(*ex->get()))
       ++browser_action_count;
 
     if (extensions::ManagedModeInfo::IsContentPack(ex->get()))
       ++content_pack_count;
 
+    RecordCreationFlags(*ex);
+
     extension_service_->RecordPermissionMessagesHistogram(
         ex->get(), "Extensions.Permissions_Load");
   }
 
   const ExtensionSet& disabled_extensions =
       extension_registry_->disabled_extensions();
   for (ex = disabled_extensions.begin(); ex != disabled_extensions.end();
        ++ex) {
     if (extension_prefs_->DidExtensionEscalatePermissions((*ex)->id())) {
       ++disabled_for_permissions_count;
@@ skipping 61 matching lines @@
 int InstalledLoader::GetCreationFlags(const ExtensionInfo* info) {
   int flags = extension_prefs_->GetCreationFlags(info->extension_id);
   if (!Manifest::IsUnpackedLocation(info->extension_location))
     flags |= Extension::REQUIRE_KEY;
   if (extension_prefs_->AllowFileAccess(info->extension_id))
     flags |= Extension::ALLOW_FILE_ACCESS;
   return flags;
 }
 
 }  // namespace extensions