New issue
Advanced search Search tips

Issue 821414 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression

Blocked on:
issue 828478
issue 829941

Blocking:
issue 833461


Participants' hotlists:
Modern-Media-Controls


Sign in to add a comment

2.4% regression in system_health.memory_mobile at 542539:542583

Project Member Reported by hjd@google.com, Mar 13 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Mar 13 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=821414

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=d593dec6a040dad88177b274656780f8e0d28aa523fe87d7650c8dd779093dbe


Bot(s) for this bug's original alert(s):

android-webview-nexus6
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Mar 14 2018

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/12908d76440000
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Mar 26 2018

Cc: steimel@chromium.org isherman@chromium.org dalecur...@chromium.org mlamouri@chromium.org
Owner: steimel@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/15981f3d440000

Add fieldtrial testing config for new media controls by steimel@chromium.org
https://chromium.googlesource.com/chromium/src/+/fdfc5ae4154a67e0539b9d0a77b3c5e2da38f40b

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Mar 26 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/13a80eed440000

Add fieldtrial testing config for new media controls by steimel@chromium.org
https://chromium.googlesource.com/chromium/src/+/fdfc5ae4154a67e0539b9d0a77b3c5e2da38f40b

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Mar 27 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16d31f7d440000

Add fieldtrial testing config for new media controls by steimel@chromium.org
https://chromium.googlesource.com/chromium/src/+/fdfc5ae4154a67e0539b9d0a77b3c5e2da38f40b

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Cc: beccahughes@chromium.org
Labels: -Pri-2 OS-Android Pri-1
Components: Blink>Media>Controls
Owner: beccahughes@chromium.org
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Mar 30 2018

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/117dc3c7440000
Project Member

Comment 17 by 42576172...@developer.gserviceaccount.com, Mar 30 2018

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14e61e27440000
Project Member

Comment 19 by 42576172...@developer.gserviceaccount.com, Mar 31 2018

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/16fc7b47440000
Project Member

Comment 20 by 42576172...@developer.gserviceaccount.com, Mar 31 2018

Owner: steimel@chromium.org
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11c7d603440000

Add fieldtrial testing config for new media controls by steimel@chromium.org
https://chromium.googlesource.com/chromium/src/+/fdfc5ae4154a67e0539b9d0a77b3c5e2da38f40b

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 21 by bugdroid1@chromium.org, Apr 2 2018

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 3 2018

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

commit 5f02ed9270f274ed7dd1cc6a861bcd6eb2d65892
Author: Becca Hughes <beccahughes@chromium.org>
Date: Tue Apr 03 16:18:08 2018

Media Controls: Only update slider positions when needed

The slider positions are constantly updated even when they have
not changed. This changes them to only be updated when changed.

BUG= 821961 ,821414

Change-Id: Ia978e130d51915bce121913db7f66caccbd9e636
Reviewed-on: https://chromium-review.googlesource.com/991075
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547727}
[modify] https://crrev.com/5f02ed9270f274ed7dd1cc6a861bcd6eb2d65892/third_party/WebKit/Source/modules/media_controls/elements/MediaControlSliderElement.cpp

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 3 2018

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

commit 07b346a00facb25e8458e18b5f6546a059d81c1c
Author: Becca Hughes <beccahughes@chromium.org>
Date: Tue Apr 03 17:19:57 2018

Media Controls: Clean up the DOM

Do not create the volume slider on the new media controls
as we don't need it. Also, avoids adding elements to the
DOM that are permanently hidden.

BUG= 821961 ,821414

Change-Id: Ia0e334b6079450d4a3b084371629f6b6ba37176c
Reviewed-on: https://chromium-review.googlesource.com/991293
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547743}
[modify] https://crrev.com/07b346a00facb25e8458e18b5f6546a059d81c1c/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/07b346a00facb25e8458e18b5f6546a059d81c1c/third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.cpp
[modify] https://crrev.com/07b346a00facb25e8458e18b5f6546a059d81c1c/third_party/WebKit/Source/modules/media_controls/MediaControlsWindowEventListener.h

Blockedon: 828478
Status: Started (was: Assigned)
After those CLs have landed these are the current metrics:

Metric	Diff %
android-nexus5X memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg 	9.91%
android-nexus6 memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg 	7.14%
android-nexus6 memory:chrome:all_processes:reported_by_chrome:malloc:effective_size_avg 	3.91%
android-nexus7v2 memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg 	3.88%
android-webview-nexus6 memory:webview:all_processes:reported_by_chrome:malloc:effective_size_avg 	3.78%
android-webview-nexus6 memory:webview:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg 	3.27%
android-nexus7v2 memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg 	3.27%
android-nexus5 memory:chrome:all_processes:reported_by_os:system_memory:private_dirty_size_avg 	3.03%
android-webview-nexus6 memory:webview:all_processes:reported_by_os:system_memory:proportional_resident_size_avg 	2.62%
android-nexus5 memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg 	2.29%
android-nexus7v2 memory:chrome:all_processes:reported_by_os:system_memory:private_dirty_size_avg 	2.20%
android-nexus5/system_health.memory_mobile / memory:chrome:all_processes:reported_by_chrome:malloc:effective_size_avg 	2.13%
android-webview-nexus6 memory:webview:all_processes:reported_by_os:system_memory:private_footprint_size_avg 	2.06%
android-webview-nexus6 memory:webview:all_processes:reported_by_chrome:malloc:allocated_objects_size_avg 	1.94%
android-nexus5X memory:chrome:all_processes:reported_by_os:system_memory:private_footprint_size_avg 	1.69%
android-nexus7v2 memory:chrome:all_processes:reported_by_os:system_memory:private_footprint_size_avg 	1.68%
android-webview-nexus6 memory:webview:all_processes:reported_by_os:system_memory:private_dirty_size_avg 	0.90%
android-nexus7v2 memory:chrome:all_processes:reported_by_os:system_memory:proportional_resident_size_avg 	0.89%
android-webview-nexus5X memory:webview:all_processes:reported_by_chrome:malloc:effective_size_avg 	0.89%
android-nexus6 memory:chrome:all_processes:reported_by_chrome:malloc:allocated_objects_size_avg 	0.76%
android-nexus5 memory:chrome:all_processes:reported_by_os:system_memory:proportional_resident_size_avg 	0.49%
android-webview-nexus5X memory:webview:all_processes:reported_by_chrome:malloc:allocated_objects_size_avg 	-0.01%
android-nexus6 memory:chrome:all_processes:reported_by_os:gpu_memory:proportional_resident_size_avg 	-0.19%
android-nexus5 memory:chrome:all_processes:reported_by_os:system_memory:private_footprint_size_avg 	-0.41%
android-one memory:chrome:all_processes:reported_by_os:system_memory:private_footprint_size_avg 	-0.65%
android-one memory:chrome:all_processes:reported_by_os:system_memory:private_dirty_size_avg 	-0.92%
android-one memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg 	-3.13%
Project Member

Comment 26 by bugdroid1@chromium.org, Apr 5 2018

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

commit 16d83bf550e6d1580ef68c646db29a1d04878278
Author: Becca Hughes <beccahughes@chromium.org>
Date: Thu Apr 05 19:25:15 2018

Media Controls: Move to different layer

will-change will force the media controls to use a different
layer. According to Pinpoint this will reduce CC memory usage
by 18% and GPU memory usage by 6%.

BUG= 821961 ,821414

Change-Id: I13c62f79577dfef253acf5f3b89cecaf0c5c1ad3
Reviewed-on: https://chromium-review.googlesource.com/996309
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548524}
[modify] https://crrev.com/16d83bf550e6d1580ef68c646db29a1d04878278/third_party/WebKit/Source/modules/media_controls/resources/modernMediaControls.css

Project Member

Comment 28 by bugdroid1@chromium.org, Apr 6 2018

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

commit 041244a9de147288c4e99b6201cda3417710e29f
Author: Becca Hughes <beccahughes@chromium.org>
Date: Fri Apr 06 03:03:42 2018

Media Controls: Create scrubbing message on demand

The scrubbing message has lots of child DOM elements. This
creates/prunes them when the visiblity of the scrubbing message changes.

BUG= 821961 ,821414

Change-Id: I4f0b2dc2c276387be1867ff6459d594dc97d3545
Reviewed-on: https://chromium-review.googlesource.com/990480
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548649}
[modify] https://crrev.com/041244a9de147288c4e99b6201cda3417710e29f/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/041244a9de147288c4e99b6201cda3417710e29f/third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementBase.h
[modify] https://crrev.com/041244a9de147288c4e99b6201cda3417710e29f/third_party/WebKit/Source/modules/media_controls/elements/MediaControlScrubbingMessageElement.cpp
[modify] https://crrev.com/041244a9de147288c4e99b6201cda3417710e29f/third_party/WebKit/Source/modules/media_controls/elements/MediaControlScrubbingMessageElement.h
[add] https://crrev.com/041244a9de147288c4e99b6201cda3417710e29f/third_party/WebKit/Source/modules/media_controls/elements/MediaControlScrubbingMessageElementTest.cpp

Project Member

Comment 29 by bugdroid1@chromium.org, Apr 6 2018

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

commit 0631e18f16653c8e262079e7ee7300e70ec2b9a3
Author: Becca Hughes <beccahughes@chromium.org>
Date: Fri Apr 06 17:19:29 2018

Media Controls: Remove manual paint invalidation

The media controls were performing manual paint invalidation
from when we used to have MediaControlPainter. Now we use
CSS this is handled for us automatically.

According to manual testing this reduces the time painting
from 153 ms to 49ms.

BUG= 821961 ,821414

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ie864f914c7ced0031710d81a7a702c8e3697a312
Reviewed-on: https://chromium-review.googlesource.com/998834
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548832}
[modify] https://crrev.com/0631e18f16653c8e262079e7ee7300e70ec2b9a3/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/0631e18f16653c8e262079e7ee7300e70ec2b9a3/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[delete] https://crrev.com/4cb700f0379ba40cb6c69b95cc1022d4e7a633c7/third_party/WebKit/LayoutTests/paint/invalidation/video-mute-repaint-expected.txt
[delete] https://crrev.com/4cb700f0379ba40cb6c69b95cc1022d4e7a633c7/third_party/WebKit/LayoutTests/paint/invalidation/video-mute-repaint.html
[delete] https://crrev.com/4cb700f0379ba40cb6c69b95cc1022d4e7a633c7/third_party/WebKit/LayoutTests/paint/invalidation/video-unmute-repaint-expected.txt
[delete] https://crrev.com/4cb700f0379ba40cb6c69b95cc1022d4e7a633c7/third_party/WebKit/LayoutTests/paint/invalidation/video-unmute-repaint.html
[modify] https://crrev.com/0631e18f16653c8e262079e7ee7300e70ec2b9a3/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/0631e18f16653c8e262079e7ee7300e70ec2b9a3/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
[modify] https://crrev.com/0631e18f16653c8e262079e7ee7300e70ec2b9a3/third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementBase.cpp

Blockedon: 829941
Project Member

Comment 31 by bugdroid1@chromium.org, Apr 6 2018

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

commit a17997b912ab317b9e73ca786523a03759b6725d
Author: Becca Hughes <beccahughes@chromium.org>
Date: Fri Apr 06 18:40:54 2018

Media Controls: Avoid style recalculation

When generating the media controls DOM we were recalculating
styles each time we called 'AppendChild'. This moves us
to using 'ParserAppendChild' which does not do this.

According to manual testing this reduces the time calculating
styles from 230ms to 139ms.

BUG= 821961 ,821414

Change-Id: I0d673ddc6e12ff81a311315466be735a7673c1bd
Reviewed-on: https://chromium-review.googlesource.com/998187
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548872}
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlElementsHelper.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlInputElement.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlLoadingPanelElement.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlOverlayPlayButtonElement.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlScrubbingMessageElement.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlTextTrackListElement.cpp
[modify] https://crrev.com/a17997b912ab317b9e73ca786523a03759b6725d/third_party/WebKit/Source/modules/media_controls/elements/MediaControlTimelineElement.cpp

Project Member

Comment 32 by bugdroid1@chromium.org, Apr 9 2018

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

commit 138363ffd7c05f6152572cfc7e0869ff4484d743
Author: Becca Hughes <beccahughes@chromium.org>
Date: Mon Apr 09 20:44:40 2018

Media Controls: Re-create the play button

As per steimel@'s comment in crrev.com/c/991293 we should always
create the play button as we use it when the video element is
very small.

BUG= 821961 ,821414

Change-Id: Icec781d0cd49529c72dd21c88d54727e81236890
Reviewed-on: https://chromium-review.googlesource.com/1000722
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549266}
[modify] https://crrev.com/138363ffd7c05f6152572cfc7e0869ff4484d743/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc

Owner: beccahughes@chromium.org
Project Member

Comment 34 by bugdroid1@chromium.org, Apr 10 2018

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

commit 5ad3a0af487c043c920283d1bc76b857c4f03ca6
Author: Becca Hughes <beccahughes@chromium.org>
Date: Tue Apr 10 10:27:15 2018

Revert "Media Controls: Move to different layer"

This reverts commit 16d83bf550e6d1580ef68c646db29a1d04878278.

Reason for revert: Performance issues

Original change's description:
> Media Controls: Move to different layer
>
> will-change will force the media controls to use a different
> layer. According to Pinpoint this will reduce CC memory usage
> by 18% and GPU memory usage by 6%.
>
> BUG= 821961 ,821414
>
> Change-Id: I13c62f79577dfef253acf5f3b89cecaf0c5c1ad3
> Reviewed-on: https://chromium-review.googlesource.com/996309
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548524}

TBR=mlamouri@chromium.org,beccahughes@chromium.org


Bug:  821961 , 821414
Change-Id: I5df8f67e4e779dc673ca9cf89221c734c5a2235c
Reviewed-on: https://chromium-review.googlesource.com/1003332
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549475}
[modify] https://crrev.com/5ad3a0af487c043c920283d1bc76b857c4f03ca6/third_party/blink/renderer/modules/media_controls/resources/modernMediaControls.css

Project Member

Comment 35 by bugdroid1@chromium.org, Apr 11 2018

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

commit b5d01e75a40d842fa2575225a194e16396c34dc4
Author: Becca Hughes <beccahughes@chromium.org>
Date: Wed Apr 11 01:42:27 2018

Media Controls: Do not update the time if we are hidden

If the panel is hidden then we should not update the current time
unnecessarily.

BUG= 821961 ,821414

Change-Id: I5c3fed448c52edfb14570d5e2e164c56e4747672
Reviewed-on: https://chromium-review.googlesource.com/1006068
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549720}
[modify] https://crrev.com/b5d01e75a40d842fa2575225a194e16396c34dc4/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc

Blocking: 833461
What work is left on this?

Sign in to add a comment