Plugin Power Saver: Collect size histogram on Flash Plugins users click.

content/renderer/pepper/pepper_plugin_instance_impl.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 "content/renderer/pepper/pepper_plugin_instance_impl.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_offset_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "cc/base/latency_info_swap_promise.h"
 #include "cc/blink/web_layer_impl.h"
 #include "cc/layers/texture_layer.h"
 #include "cc/trees/layer_tree_host.h"
 #include "content/common/content_constants_internal.h"
@@ skipping 154 matching lines @@
 using blink::WebURLRequest;
 using blink::WebURLResponse;
 using blink::WebUserGestureIndicator;
 using blink::WebUserGestureToken;
 using blink::WebView;
 
 namespace content {
 
 namespace {
 
+static const int kInfiniteRatio = 99999;
+
+#define UMA_HISTOGRAM_ASPECT_RATIO(name, width, height) \
+    UMA_HISTOGRAM_SPARSE_SLOWLY( \

[groby-ooo-7-16, 2014/11/07 21:57:44]

I think  we need cutoffs - beyond 3:1 or 4:1 it's becoming really uninteresting
to sample more details. Also, the effective granularity for aspect ratios < 1 is
worse than the other way around.

So, how about this for the value:

// flip ratio to always be >1
if (height > width) 
  swapped = true;
  std::swap(width, height)
}

// Clamp ratio at 400, 401 is anything > 4:1
if (height)
  ratio = min(((width * 100) / height), 400)
else
  ratio = 401;

// Flag swapped values
if (swapped) 
  ratio = -ratio;

Or, if you want to be concise:
ratio = (min(width,height) ? min(max(width, height) / min(width,height), 400) :
401) * ((width < height) ? -1 : 1); // I'd rather you're not concise and spell
this out in the recording function :)

[tommycli, 2014/11/07 22:46:37]

I used the same aspect ratio encoding as here:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

I was planning to consolidate the two macros in a followup CL.

[groby-ooo-7-16, 2014/11/07 23:02:20]

These are video aspect ratios - they're rarely flipped (i.e. <1), so they don't
need to tackle that problem.  Essentially, they're expected to be 4:3/16:9
pretty much all the time. Hm.

We _expect_ that to be the case as well, so we could indeed re-use that macro.
If we're outside that range, the data will just be slightly less useful. Since
it promotes re-use, let's re-use.  

[tommycli, 2014/11/07 23:14:10]

Done.

+        name, \
+        (height) ? ((width) * 100) / (height) : kInfiniteRatio);
+
 // Check PP_TextInput_Type and ui::TextInputType are kept in sync.
 COMPILE_ASSERT(int(ui::TEXT_INPUT_TYPE_NONE) == int(PP_TEXTINPUT_TYPE_NONE),
                mismatching_enums);
 COMPILE_ASSERT(int(ui::TEXT_INPUT_TYPE_TEXT) == int(PP_TEXTINPUT_TYPE_TEXT),
                mismatching_enums);
 COMPILE_ASSERT(int(ui::TEXT_INPUT_TYPE_PASSWORD) ==
                    int(PP_TEXTINPUT_TYPE_PASSWORD),
                mismatching_enums);
 COMPILE_ASSERT(int(ui::TEXT_INPUT_TYPE_SEARCH) == int(PP_TEXTINPUT_TYPE_SEARCH),
                mismatching_enums);
@@ skipping 185 matching lines @@
       input_sequence);
   new_latency->TraceEventType(WebInputEventTraits::GetName(type));
   if (old_latency) {
     new_latency->CopyLatencyFrom(*old_latency,
                                  ui::INPUT_EVENT_LATENCY_ORIGINAL_COMPONENT);
     new_latency->CopyLatencyFrom(*old_latency,
                                  ui::INPUT_EVENT_LATENCY_UI_COMPONENT);
   }
 }
 
+enum PluginFlashInstanceSize {
+  INSTANCE_SIZE_1_1 = 0,
+  INSTANCE_SIZE_5_5 = 1,
+  INSTANCE_SIZE_10_10 = 2,
+  INSTANCE_SIZE_LARGE = 3,
+  INSTANCE_SIZE_NUM_ITEMS
+};
+
 // How the throttled power saver is unthrottled, if ever.
 // These numeric values are used in UMA logs; do not change them.
 enum PowerSaverUnthrottleMethod {
   UNTHROTTLE_METHOD_NEVER = 0,
   UNTHROTTLE_METHOD_BY_CLICK = 1,
   UNTHROTTLE_METHOD_BY_WHITELIST = 2,
   UNTHROTTLE_METHOD_NUM_ITEMS
 };
 
+const char kFlashClickSizeAspectRatioHistogram[] =
+    "Plugin.FlashClickSizeAspectRatio";

[groby-ooo-7-16, 2014/11/07 21:57:44]

Plugin.Flash.ClickSize.AspectRatio ? (And the same for the other metrics). We
seem to usually treat names as a hierarchy of namespaces, so this seems a better
name. 

[tommycli, 2014/11/07 22:46:37]

Done.

+const char kFlashClickSizeHeightHistogram[] = "Plugin.FlashClickSizeHeight";
+const char kFlashClickSizeWidthHistogram[] = "Plugin.FlashClickSizeWidth";
+const char kFlashInstanceSizeHistogram[] = "Plugin.FlashInstanceSize";
 const char kPowerSaverUnthrottleHistogram[] = "Plugin.PowerSaverUnthrottle";
 
+void RecordFlashSizeMetric(int width, int height) {
+  if (width <= 1 && height <= 1) {
+    UMA_HISTOGRAM_ENUMERATION(kFlashInstanceSizeHistogram, INSTANCE_SIZE_1_1,
+                              INSTANCE_SIZE_NUM_ITEMS);
+  } else if (width <= 5 && height <= 5) {
+    UMA_HISTOGRAM_ENUMERATION(kFlashInstanceSizeHistogram, INSTANCE_SIZE_5_5,
+                              INSTANCE_SIZE_NUM_ITEMS);
+  } else if (width <= 10 && height <= 10) {
+    UMA_HISTOGRAM_ENUMERATION(kFlashInstanceSizeHistogram, INSTANCE_SIZE_10_10,
+                              INSTANCE_SIZE_NUM_ITEMS);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(kFlashInstanceSizeHistogram, INSTANCE_SIZE_LARGE,
+                              INSTANCE_SIZE_NUM_ITEMS);
+  }
+}
+
+void RecordFlashClickSizeMetric(int width, int height) {
+  UMA_HISTOGRAM_COUNTS(kFlashClickSizeHeightHistogram, height);

[groby-ooo-7-16, 2014/11/07 21:57:44]

I believe these are exponential histograms. We want a linear one for all of
them. Something like

  base::HistogramBase* histogram =
        base::LinearHistogram::FactoryGet(
            kFlashClickSizeHeightHistogram
            200, // minimum height
            400, // maximum height
            50,  // number of buckets.
            base::HistogramBase::kUmaTargetedHistogramFlag);
    histogram->Add(height);

[tommycli, 2014/11/07 22:46:37]

Done.

I followed this example:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

Are they 'doing it wrong' then?

[groby-ooo-7-16, 2014/11/07 23:02:20]

They're not "doing it wrong", but they might be interested in different data. If
you look at their width/height metrics, you'll clearly see the exponential
bucketing, which might serve the needs they have.

We're interested in getting a fairly exact reading of the cutoff point, so
having things vanish in large buckets would not be helpful. (Also, their range
is bigger than ours - we want a narrow sample around the cutoff point, they
probably want an overview)

[tommycli, 2014/11/07 23:14:10]

Done. Good point.

+  UMA_HISTOGRAM_COUNTS(kFlashClickSizeWidthHistogram, width);
+  UMA_HISTOGRAM_ASPECT_RATIO(kFlashClickSizeAspectRatioHistogram, width,
+                             height);
+}
+
 void RecordUnthrottleMethodMetric(PowerSaverUnthrottleMethod method) {
   UMA_HISTOGRAM_ENUMERATION(kPowerSaverUnthrottleHistogram, method,
                             UNTHROTTLE_METHOD_NUM_ITEMS);
 }
 
 bool IsFlashPlugin(PluginModule* module) {
   return module->name() == kFlashPluginName;
 }
 
 }  // namespace
@@ skipping 89 matching lines @@
     const GURL& plugin_url)
     : RenderFrameObserver(render_frame),
       render_frame_(render_frame),
       module_(module),
       instance_interface_(instance_interface),
       pp_instance_(0),
       container_(container),
       layer_bound_to_fullscreen_(false),
       layer_is_hardware_(false),
       plugin_url_(plugin_url),
+      has_been_clicked_(false),
       power_saver_enabled_(false),
       is_peripheral_content_(false),
       plugin_throttled_(false),
       full_frame_(false),
       sent_initial_did_change_view_(false),
       bound_graphics_2d_platform_(NULL),
       bound_compositor_(NULL),
       has_webkit_focus_(false),
       has_content_area_focus_(false),
       find_identifier_(-1),
@@ skipping 75 matching lines @@
           GetPluginURL());
     }
   }
 
   RendererPpapiHostImpl* host_impl = module_->renderer_ppapi_host();
   resource_creation_ = host_impl->CreateInProcessResourceCreationAPI(this);
 
   if (GetContentClient()->renderer() &&  // NULL in unit tests.
       GetContentClient()->renderer()->IsExternalPepperPlugin(module->name()))
     external_document_load_ = true;
-
-  if (IsFlashPlugin(module_.get())) {
-    RenderThread::Get()->RecordAction(
-        base::UserMetricsAction("Flash.PluginInstanceCreated"));
-  }
 }
 
 PepperPluginInstanceImpl::~PepperPluginInstanceImpl() {
   DCHECK(!fullscreen_container_);
 
   if (plugin_throttled_)
     RecordUnthrottleMethodMetric(UNTHROTTLE_METHOD_NEVER);
 
   // Notify all the plugin objects of deletion. This will prevent blink from
   // calling into the plugin any more.
@@ skipping 240 matching lines @@
 #endif
 }
 
 bool PepperPluginInstanceImpl::Initialize(
     const std::vector<std::string>& arg_names,
     const std::vector<std::string>& arg_values,
     bool full_frame) {
   if (!render_frame_)
     return false;
 
+  blink::WebRect bounds = container_->element().boundsInViewportSpace();
+  if (IsFlashPlugin(module_.get())) {

[groby-ooo-7-16, 2014/11/07 21:57:44]

Curious: Why did this move? To do all recording at the same time?

[tommycli, 2014/11/07 22:46:38]

Yes. And it had to be here because you can only get the bounds after the
constructor finishes.

+    RenderThread::Get()->RecordAction(
+        base::UserMetricsAction("Flash.PluginInstanceCreated"));
+    RecordFlashSizeMetric(bounds.width, bounds.height);
+  }
+
   PluginPowerSaverHelper* power_saver_helper =
       render_frame_->plugin_power_saver_helper();
   GURL content_origin = plugin_url_.GetOrigin();
-  blink::WebRect bounds = container_->element().boundsInViewportSpace();
 
   bool cross_origin = false;
   is_peripheral_content_ =
       IsFlashPlugin(module_.get()) &&
       power_saver_helper->ShouldThrottleContent(content_origin, bounds.width,
                                                 bounds.height, &cross_origin);
 
   power_saver_enabled_ = is_peripheral_content_ &&
                          base::CommandLine::ForCurrentProcess()->HasSwitch(
                              switches::kEnablePluginPowerSaver);
@@ skipping 263 matching lines @@
   gfx::Rect caret(text_input_caret_);
   caret.Offset(view_data_.rect.point.x, view_data_.rect.point.y);
   return caret;
 }
 
 bool PepperPluginInstanceImpl::HandleInputEvent(
     const blink::WebInputEvent& event,
     WebCursorInfo* cursor_info) {
   TRACE_EVENT0("ppapi", "PepperPluginInstanceImpl::HandleInputEvent");
 
+  if (event.type == blink::WebInputEvent::MouseDown && !has_been_clicked_ &&
+      IsFlashPlugin(module_.get())) {
+    has_been_clicked_ = true;
+    blink::WebRect bounds = container_->element().boundsInViewportSpace();
+    RecordFlashClickSizeMetric(bounds.width, bounds.height);
+  }
+
   if (event.type == blink::WebInputEvent::MouseUp && is_peripheral_content_) {
     is_peripheral_content_ = false;
     power_saver_enabled_ = false;
 
     RecordUnthrottleMethodMetric(UNTHROTTLE_METHOD_BY_CLICK);
 
     if (plugin_throttled_) {
       SetPluginThrottled(false /* throttled */);
       return true;
     }
@@ skipping 2195 matching lines @@
     return;
 
   is_peripheral_content_ = false;
   power_saver_enabled_ = false;
   SetPluginThrottled(false);
 
   RecordUnthrottleMethodMetric(UNTHROTTLE_METHOD_BY_WHITELIST);
 }
 
 }  // namespace content