New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634130 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Enhance Plugin Power Saver Metrics

Project Member Reported by tommycli@chromium.org, Aug 3 2016

Issue description

1. Unthrottle metric should also record unthrottles of tiny plugins via omnibox icon.

2. The PeripheralContentHeuristic metric should be forked into a new one that records the final decision only. Then the old one can be deprecated.
 
Labels: Merge-Request-53
Any chance for a 53 merge? The above patch is quite low risk, but could yield good metrics.

Comment 3 by dimu@chromium.org, Aug 9 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f

commit 8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f
Author: tommycli <tommycli@chromium.org>
Date: Wed Aug 10 23:05:08 2016

Plugin Power Saver Tiny: Fix Plugin.PowerSaver.PeripheralHeuristic UMA

Previously, it counted every status query. As we implemented PPS tiny,
the number of queries for tiny plugins increased, but we didn't
enforce that it was only recorded once.

This CL enforces that only the initial decision for a plugin is
recorded. It also migrates the UMA to a
Plugin.PowerSaver.PeripheralHeuristicInitialDecision name to prevent
further confusion.

This is complicated because there are three paths a plugin may follow:

1. Essential origin case. No placeholder, no throttler - Record
   immediately, in PowerSaverInfo.

2. Placeholder spawned first (because PPS tiny or background tab or
   prerendering) - Don't record in PowerSaverInfo. Record in
   LoadablePluginPlaceholder after size known. If a Throttler is
   spawned from the placeholder, tell it to not record.

3. Throttler spawned first - Record in the throttler.

Once PPS Tiny is launched 100%, case #3 can be eliminated.

BUG= 634130 

Review-Url: https://codereview.chromium.org/2211753002
Cr-Commit-Position: refs/heads/master@{#411174}

[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/chrome/renderer/plugins/chrome_plugin_placeholder.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/chrome/renderer/plugins/power_saver_info.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/components/plugins/renderer/loadable_plugin_placeholder.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/components/plugins/renderer/loadable_plugin_placeholder.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/public/renderer/plugin_instance_throttler.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/public/renderer/render_frame.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/pepper_webplugin_impl_browsertest.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_instance_throttler_impl.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_instance_throttler_impl.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_instance_throttler_impl_unittest.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_power_saver_helper.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_power_saver_helper.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/pepper/plugin_power_saver_helper_browsertest.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/content/renderer/render_frame_impl.h
[modify] https://crrev.com/8a1ef84d9dab70b79d20b2aa6d07ade20e54d19f/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by sheriffbot@chromium.org, Aug 13 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Started)
Thanks all. I'm not merging the second patch since it's pretty huge.

Tommy
Project Member

Comment 7 by sheriffbot@chromium.org, Aug 16 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-53

Sign in to add a comment