Revert of Add support for single sample metrics.

Revert of Add support for single sample metrics. (patchset #15 id:360001 of https://codereview.chromium.org/2687583002/ )

Reason for revert:
Flaky SingleSampleMetricsFactoryImplTest.MultithreadedMetrics test and cronet size regression.

Original issue's description:
> Add support for single sample metrics.
> 
> Single sample metrics offer a mechanism for clients to modify
> a histogram sample repeatedly while having it only reported once;
> either upon destruction of a helper class or at process termination.
> 
> This is helpful since the fast shutdown path simply kills renderer
> processes without going through any destructors. In this case we
> lose histogram values that might otherwise be reported at that
> time.
> 
> In media/ code we've created a cumbersome proxy mechanism which
> sends histogram values like this to the browser process and records
> them there (see WatchTimeReporter and MediaInternals interactions).
> 
> Single sample histograms are implemented through a new mojo service
> hosted by the browser process that receives samples and logs the
> last received sample upon mojo channel closure.
> 
> base:: shims are provided so that these metrics can be created by
> anyone in the renderer process. For non-renderer processes a default
> implementation is provided without the mojo channel behavior.
> 
> Usage looks like the following:
>   std::unique_ptr<base::SingleSampleMetric> metric(
>       base::SingleSampleMetricsFactory::Get()->CreateCustomCountsMetric(
>           "Media.VideoRenderer.CadenceChanges", 1, 10, 10));
>   metric->SetSample(1);
>   metric->SetSample(2);
>   metric->SetSample(3);
>   metric.reset();
> 
> Only the last sample (3) would end up being reported to the histogram.
> 
> BUG=689751
> TEST=new tests
> 
> Review-Url: https://codereview.chromium.org/2687583002
> Cr-Commit-Position: refs/heads/master@{#469524}
> Committed: https://chromium.googlesource.com/chromium/src/+/4a9839a21d7fd95641ba584396bf328fbc1a96b0

TBR=bcwhite@chromium.org,blundell@chromium.org,dcheng@chromium.org,nick@chromium.org,oysteine@chromium.org,rockot@chromium.org,dalecurtis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=689751

[Alexei Svitkine (slow), 2017-05-05 19:24:43]

The CQ bit was checked by asvitkine@chromium.org

[Alexei Svitkine (slow), 2017-05-05 19:24:43]

Created Revert of Add support for single sample metrics.

[commit-bot: I haz the power, 2017-05-05 19:25:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 19:26:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 19:26:17]

Failed to apply patch for
content/browser/renderer_host/render_process_host_impl.cc:
While running git apply --index -3 -p1;
  error: patch failed:
content/browser/renderer_host/render_process_host_impl.cc:53
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: content/browser/renderer_host/render_process_host_impl.cc: patch does
not apply

Patch:       content/browser/renderer_host/render_process_host_impl.cc
Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc
b/content/browser/renderer_host/render_process_host_impl.cc
index
fbe15860b0a41a051fe69e967a1ad0901b3f7a4d..1234f79bce682006893b6f895177143921e69cf9
100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -53,7 +53,6 @@
 #include "cc/base/switches.h"
 #include "cc/output/buffer_to_texture_target_map.h"
 #include
"components/discardable_memory/service/discardable_shared_memory_manager.h"
-#include "components/metrics/single_sample_metrics.h"
 #include "components/tracing/common/tracing_switches.h"
 #include "content/browser/appcache/appcache_dispatcher_host.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
@@ -1329,9 +1328,6 @@
   registry->AddInterface(
       base::Bind(&VideoCaptureHost::Create,
                  BrowserMainLoop::GetInstance()->media_stream_manager()));
-
-  registry->AddInterface(
-      base::Bind(&metrics::CreateSingleSampleMetricsProvider));
 
   if (base::FeatureList::IsEnabled(features::kOffMainThreadFetch)) {
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context(

[xunjieli, 2017-05-05 20:42:49]

Description was changed from

==========
Revert of Add support for single sample metrics. (patchset #15 id:360001 of
https://codereview.chromium.org/2687583002/ )

Reason for revert:
Flaky SingleSampleMetricsFactoryImplTest.MultithreadedMetrics test and cronet
size regression.

Original issue's description:
> Add support for single sample metrics.
> 
> Single sample metrics offer a mechanism for clients to modify
> a histogram sample repeatedly while having it only reported once;
> either upon destruction of a helper class or at process termination.
> 
> This is helpful since the fast shutdown path simply kills renderer
> processes without going through any destructors. In this case we
> lose histogram values that might otherwise be reported at that
> time.
> 
> In media/ code we've created a cumbersome proxy mechanism which
> sends histogram values like this to the browser process and records
> them there (see WatchTimeReporter and MediaInternals interactions).
> 
> Single sample histograms are implemented through a new mojo service
> hosted by the browser process that receives samples and logs the
> last received sample upon mojo channel closure.
> 
> base:: shims are provided so that these metrics can be created by
> anyone in the renderer process. For non-renderer processes a default
> implementation is provided without the mojo channel behavior.
> 
> Usage looks like the following:
>   std::unique_ptr<base::SingleSampleMetric> metric(
>       base::SingleSampleMetricsFactory::Get()->CreateCustomCountsMetric(
>           "Media.VideoRenderer.CadenceChanges", 1, 10, 10));
>   metric->SetSample(1);
>   metric->SetSample(2);
>   metric->SetSample(3);
>   metric.reset();
> 
> Only the last sample (3) would end up being reported to the histogram.
> 
> BUG=689751
> TEST=new tests
> 
> Review-Url: https://codereview.chromium.org/2687583002
> Cr-Commit-Position: refs/heads/master@{#469524}
> Committed:
https://chromium.googlesource.com/chromium/src/+/4a9839a21d7fd95641ba584396bf...

TBR=bcwhite@chromium.org,blundell@chromium.org,dcheng@chromium.org,nick@chrom...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=689751
==========

to

==========
Revert of Add support for single sample metrics. (patchset #15 id:360001 of
https://codereview.chromium.org/2687583002/ )

Reason for revert:
Flaky SingleSampleMetricsFactoryImplTest.MultithreadedMetrics test and cronet
size regression.

Original issue's description:
> Add support for single sample metrics.
> 
> Single sample metrics offer a mechanism for clients to modify
> a histogram sample repeatedly while having it only reported once;
> either upon destruction of a helper class or at process termination.
> 
> This is helpful since the fast shutdown path simply kills renderer
> processes without going through any destructors. In this case we
> lose histogram values that might otherwise be reported at that
> time.
> 
> In media/ code we've created a cumbersome proxy mechanism which
> sends histogram values like this to the browser process and records
> them there (see WatchTimeReporter and MediaInternals interactions).
> 
> Single sample histograms are implemented through a new mojo service
> hosted by the browser process that receives samples and logs the
> last received sample upon mojo channel closure.
> 
> base:: shims are provided so that these metrics can be created by
> anyone in the renderer process. For non-renderer processes a default
> implementation is provided without the mojo channel behavior.
> 
> Usage looks like the following:
>   std::unique_ptr<base::SingleSampleMetric> metric(
>       base::SingleSampleMetricsFactory::Get()->CreateCustomCountsMetric(
>           "Media.VideoRenderer.CadenceChanges", 1, 10, 10));
>   metric->SetSample(1);
>   metric->SetSample(2);
>   metric->SetSample(3);
>   metric.reset();
> 
> Only the last sample (3) would end up being reported to the histogram.
> 
> BUG=689751
> TEST=new tests
> 
> Review-Url: https://codereview.chromium.org/2687583002
> Cr-Commit-Position: refs/heads/master@{#469524}
> Committed:
https://chromium.googlesource.com/chromium/src/+/4a9839a21d7fd95641ba584396bf...

TBR=bcwhite@chromium.org,blundell@chromium.org,dcheng@chromium.org,nick@chrom...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=689751
==========

[ncarter (slow), 2017-05-05 21:03:13]

On 2017/05/05 19:26:17, commit-bot: I haz the power wrote:
> Failed to apply patch for
> content/browser/renderer_host/render_process_host_impl.cc:
> While running git apply --index -3 -p1;
>   error: patch failed:
> content/browser/renderer_host/render_process_host_impl.cc:53
>   error: repository lacks the necessary blob to fall back on 3-way merge.
>   error: content/browser/renderer_host/render_process_host_impl.cc: patch does
> not apply
> 
> Patch:       content/browser/renderer_host/render_process_host_impl.cc
> Index: content/browser/renderer_host/render_process_host_impl.cc
> diff --git a/content/browser/renderer_host/render_process_host_impl.cc
> b/content/browser/renderer_host/render_process_host_impl.cc
> index
>
fbe15860b0a41a051fe69e967a1ad0901b3f7a4d..1234f79bce682006893b6f895177143921e69cf9
> 100644
> --- a/content/browser/renderer_host/render_process_host_impl.cc
> +++ b/content/browser/renderer_host/render_process_host_impl.cc
> @@ -53,7 +53,6 @@
>  #include "cc/base/switches.h"
>  #include "cc/output/buffer_to_texture_target_map.h"
>  #include
> "components/discardable_memory/service/discardable_shared_memory_manager.h"
> -#include "components/metrics/single_sample_metrics.h"
>  #include "components/tracing/common/tracing_switches.h"
>  #include "content/browser/appcache/appcache_dispatcher_host.h"
>  #include "content/browser/appcache/chrome_appcache_service.h"
> @@ -1329,9 +1328,6 @@
>    registry->AddInterface(
>        base::Bind(&VideoCaptureHost::Create,
>                   BrowserMainLoop::GetInstance()->media_stream_manager()));
> -
> -  registry->AddInterface(
> -      base::Bind(&metrics::CreateSingleSampleMetricsProvider));
>  
>    if (base::FeatureList::IsEnabled(features::kOffMainThreadFetch)) {
>      scoped_refptr<ServiceWorkerContextWrapper> service_worker_context(

FYI dale is out of office for a while (he's getting married on Sunday) so he
won't be able to revert

[xunjieli, 2017-05-08 13:56:13]

A friendly ping. Are we proceeding with the manual revert? 
Thanks.

[Alexei Svitkine (slow), 2017-05-08 13:58:59]

Given revert didn't apply cleanly, maybe we can stick with this being
landed? You can then your follow up CL to fix sizes for cronet.

On May 8, 2017 9:56 AM, <xunjieli@chromium.org> wrote:

> A friendly ping. Are we proceeding with the manual revert?
> Thanks.
>
> https://codereview.chromium.org/2862333002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.