Record whether and why the media default controls are being shown.

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 71 matching lines @@
 #include "public/platform/WebInbandTextTrack.h"
 #include "wtf/CurrentTime.h"
 #include "wtf/MainThread.h"
 #include "wtf/MathExtras.h"
 #include "wtf/text/CString.h"
 #include <limits>
 
 
 namespace blink {
 
+namespace {
+
+enum DefaultControlsShow {

[philipj_slow, 2016/02/16 06:20:27]

I'd call it MediaControls, because these are the only controls that Blink knows
anything about. And enum class for less repetition?

[mlamouri (slow - plz ping), 2016/02/18 15:49:31]

Done.
enum class are not very friendly with macros so I would have to convert to int
when passing to the macro which would be... unfortunate :)

+    DefaultControlsShowAttribute = 0,
+    DefaultControlsShowFullscreen,
+    DefaultControlsShowNoScript,
+    DefaultControlsShowNone,
+    DefaultControlsShowMax
+};
+
+} // anonymous namespace

[philipj_slow, 2016/02/16 06:20:27]

Move the below static things into the anonymous namespace too? Nobody is ever
going to do it after this change. ("This looks strange, but there's probably a
good reason for it, never mind.")

[mlamouri (slow - plz ping), 2016/02/18 15:49:31]

I did that here: https://codereview.chromium.org/1710013002
This CL will be based on top of it.

+
 #if !LOG_DISABLED
 static String urlForLoggingMedia(const KURL& url)
 {
     static const unsigned maximumURLLengthForLogging = 128;
 
     if (url.string().length() < maximumURLLengthForLogging)
         return url.string();
     return url.string().substring(0, maximumURLLengthForLogging) + "...";
 }
 
@@ skipping 1982 matching lines @@
     if (frame && !frame->script().canExecuteScripts(NotAboutToExecuteScript))
         return true;
 
     // Always show controls when in full screen mode.
     if (isFullscreen())
         return true;
 
     return fastHasAttribute(controlsAttr);
 }
 
+void HTMLMediaElement::recordShowControls() const
+{
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, showControlsHistogram, ("Media.Controls.Show", DefaultControlsShowMax));
+
+    if (fastHasAttribute(controlsAttr)) {

[philipj_slow, 2016/02/16 06:20:27]

Duplicating the conditions of shouldShowControls() but in a different order
makes it non-obvious that it's precisely correct, and that should be trivial. Is
it separate for performance concerns? If so, make shouldShowControls() return an
enum value which is just treated as bool everywhere except before
mediaControls()->show(), where it's used to count?

[mlamouri (slow - plz ping), 2016/02/18 15:49:31]

Fixed. I know have an internal show controls that record the result.

+        showControlsHistogram.count(DefaultControlsShowAttribute);
+        return;
+    }
+
+    if (isFullscreen()) {
+        showControlsHistogram.count(DefaultControlsShowFullscreen);
+        return;
+    }
+
+    LocalFrame* frame = document().frame();
+    if (frame && !frame->script().canExecuteScripts(NotAboutToExecuteScript)) {
+        showControlsHistogram.count(DefaultControlsShowNoScript);
+        return;
+    }
+
+    ASSERT(!shouldShowControls());
+    showControlsHistogram.count(DefaultControlsShowNone);
+}
+
 double HTMLMediaElement::volume() const
 {
     return m_volume;
 }
 
 void HTMLMediaElement::setVolume(double vol, ExceptionState& exceptionState)
 {
     WTF_LOG(Media, "HTMLMediaElement::setVolume(%p, %f)", this, vol);
 
     if (m_volume == vol)
@@ skipping 1232 matching lines @@
 void HTMLMediaElement::configureMediaControls()
 {
     if (!inDocument()) {
         if (mediaControls())
             mediaControls()->hide();
         return;
     }
 
     ensureMediaControls();
     mediaControls()->reset();
+
+    recordShowControls();
     if (shouldShowControls())
         mediaControls()->show();
     else
         mediaControls()->hide();
 }
 
 CueTimeline& HTMLMediaElement::cueTimeline()
 {
     if (!m_cueTimeline)
         m_cueTimeline = adoptPtrWillBeNoop(new CueTimeline(*this));
@@ skipping 269 matching lines @@
 {
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 
 } // namespace blink