Tracking remaining performance regression bugs
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92d457976874c99ded44a48ab92e768f2f274ca9 commit 92d457976874c99ded44a48ab92e768f2f274ca9 Author: Mounir Lamouri <mlamouri@chromium.org> Date: Tue May 01 17:08:59 2018 Media Controls: remove window event listener and listen to event on popup. This is moving the logic to hide the popups to the popup element class. It will hide when the popup no longer has focus, when the window is resized or scrolled. It slightly improves memory usage by only creating the event listener object on demand. Bug: 833461 Change-Id: I979f0882108fcd89fdf84b2612437209b080eb95 Reviewed-on: https://chromium-review.googlesource.com/1023345 Reviewed-by: Becca Hughes <beccahughes@chromium.org> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#555083} [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-item.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside-stoppropagation.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-panel.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-resize.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-scroll.html [add] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/WebKit/LayoutTests/media/controls/overflow-menu-visibility.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-outside.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-panel.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-resize.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-visibility.html [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/WebKit/LayoutTests/media/media-controls-hide-menu-stoppropagation.html [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/BUILD.gn [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_input_element.cc [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.cc [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.h [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.cc [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.h [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/elements/media_control_toggle_closed_captions_button_element.cc [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [modify] https://crrev.com/92d457976874c99ded44a48ab92e768f2f274ca9/third_party/blink/renderer/modules/media_controls/media_controls_impl_test.cc [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.cc [delete] https://crrev.com/a127b827f1e3daa105b24e99fbcdfcaf6e12479a/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e43caa6014719bd4cf5476e76404dfb208e97a8a commit e43caa6014719bd4cf5476e76404dfb208e97a8a Author: Matt Giuca <mgiuca@chromium.org> Date: Wed May 02 06:37:47 2018 Revert "Media Controls: remove window event listener and listen to event on popup." This reverts commit 92d457976874c99ded44a48ab92e768f2f274ca9. Reason for revert: Suspect cause of crash in WebKit layout tests on Windows ( https://crbug.com/838782 ). Original change's description: > Media Controls: remove window event listener and listen to event on popup. > > This is moving the logic to hide the popups to the popup element class. > It will hide when the popup no longer has focus, when the window is > resized or scrolled. It slightly improves memory usage by only creating > the event listener object on demand. > > Bug: 833461 > Change-Id: I979f0882108fcd89fdf84b2612437209b080eb95 > Reviewed-on: https://chromium-review.googlesource.com/1023345 > Reviewed-by: Becca Hughes <beccahughes@chromium.org> > Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> > Cr-Commit-Position: refs/heads/master@{#555083} TBR=mlamouri@chromium.org,beccahughes@chromium.org Change-Id: Ic0bdd6a3422447424c4c00037e57005c9ba1de05 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 833461, 838782 Reviewed-on: https://chromium-review.googlesource.com/1039116 Reviewed-by: Matt Giuca <mgiuca@chromium.org> Commit-Queue: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#555321} [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-item.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside-stoppropagation.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-panel.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-resize.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-scroll.html [delete] https://crrev.com/498ff64dc11978dcfdd294d466454ddb8cb0dd5c/third_party/WebKit/LayoutTests/media/controls/overflow-menu-visibility.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-outside.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-panel.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-resize.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-visibility.html [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/WebKit/LayoutTests/media/media-controls-hide-menu-stoppropagation.html [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/BUILD.gn [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_input_element.cc [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.cc [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.h [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.cc [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.h [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/elements/media_control_toggle_closed_captions_button_element.cc [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [modify] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/media_controls_impl_test.cc [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.cc [add] https://crrev.com/e43caa6014719bd4cf5476e76404dfb208e97a8a/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9efa14b6519436365827fff7444ff66bc935f01 commit b9efa14b6519436365827fff7444ff66bc935f01 Author: Mounir Lamouri <mlamouri@chromium.org> Date: Wed May 02 17:04:12 2018 Reland "Media Controls: remove window event listener and listen to event on popup." Fixes the crash. Original change's description: > Media Controls: remove window event listener and listen to event on popup. > > This is moving the logic to hide the popups to the popup element class. > It will hide when the popup no longer has focus, when the window is > resized or scrolled. It slightly improves memory usage by only creating > the event listener object on demand. > > Bug: 833461 > Change-Id: I979f0882108fcd89fdf84b2612437209b080eb95 > Reviewed-on: https://chromium-review.googlesource.com/1023345 > Reviewed-by: Becca Hughes <beccahughes@chromium.org> > Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> > Cr-Commit-Position: refs/heads/master@{#555083} TBR=mlamouri@chromium.org Change-Id: Id5fa81b6370dcb49d6ae7b0c958e07a4e0c23a4a Reviewed-on: https://chromium-review.googlesource.com/1039587 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#555434} [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/modern/singletap-on-overlay-closes-overflow-menu.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-item.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside-stoppropagation.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-outside.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-click-panel.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-resize.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-hide-on-scroll.html [add] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/WebKit/LayoutTests/media/controls/overflow-menu-visibility.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-outside.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click-panel.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-click.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-hide-on-resize.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/controls/video-controls-overflow-menu-visibility.html [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/WebKit/LayoutTests/media/media-controls-hide-menu-stoppropagation.html [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/BUILD.gn [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_input_element.cc [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.cc [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_overflow_menu_list_element.h [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.cc [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_popup_menu_element.h [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/elements/media_control_toggle_closed_captions_button_element.cc [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/media_controls_impl.cc [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/media_controls_impl.h [modify] https://crrev.com/b9efa14b6519436365827fff7444ff66bc935f01/third_party/blink/renderer/modules/media_controls/media_controls_impl_test.cc [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.cc [delete] https://crrev.com/a5f079381aa96a95dc5e60c064105849b5b7dc58/third_party/blink/renderer/modules/media_controls/media_controls_window_event_listener.h
There are quite a few performance regressions blocked on this bug. Is media controls behind a flag? If not, will these all be addressed by branch cut?
Issue 836477 has been merged into this issue.
Issue 836476 has been merged into this issue.
Comment 1 by beccahughes@chromium.org
, Apr 16 2018