Theme Installer: spinner animated GIF and high CPU usage

[afercia]

In the Theme Installer (Themes > Add New > Preview a Theme), the spinner loading indicator causes the same performance issues noticed for the Customizer spinner a while ago, see https://core.trac.wordpress.org/ticket/31196#comment:14

On modern hardware, with a dedicated GPU and hardware acceleration support, you will barely notice this but on old hardware the CPU usage can easily be over 50% and in the worst case always stuck at 100% since the spinner is always "spinning" in the background even when overlaid by the iframe.

Looks like modern browser still have issues with animated GIFs especially when some CSS properties (e.g. opacity, box-shadow) are set on the GIF container. I can reproduce high CPU usage using both Chrome and Firefox, they behave differently but the result is the same.

Regardless, when the preview iframe loads, the background spinner image should be removed as already done for the Customizer.

See also related #33311 for the Thickbox spinner.

[afercia]

First pass: the proposed patch uses a callback on the iframe load event to toggle a CSS class and remove the background spinner.

[atomicjack]

-removed-

[helen]

@atomicjack afercia has been around quite a bit, we do check on various older systems. Calling them "unsupported" is not quite accurate, though our support may amount to all of "make sure we don't crash the browser", which is what is happening here. Philosophically, we would like to enable as many people to publish as possible, and there are times where they are not in control of their computing environment. These are often the people who are in need of a voice on the web.

I'd kindly suggest that you spend a little more time observing before making such sweeping statements to experienced contributors about best practices for this specific project.

[afercia]

No worries :) WordPress is still officially supporting IE 8 and as I see it it should then support also the OS and the hardware where IE 8 typically runs. When speaking about performance, it's a bit pointless testing IE 8 on a Virtual Machine running on a powerful Mac :) Would recommend also to consider that outside the US Windows XP is still largely used. See also #33311 where similar issues happens also on a MacBook Pro and the video posted there that shows the continuous repaints.

[jdgrimes]

Just to confirm, on my MacBook Pro running OS X 10.4 I experience high CPU usage from these spinners if they are displayed for long periods of time (more than a few seconds).

[adamsilverstein]

33322.2.patch​:

• refresh against trunk

@afercia Thanks, this looks good! tested and see the class being added and removed and spinner hidden as a result when iframe loading completes. I trust this solves the issue, although I didn't test for or observe the high cpu use on my local machine.

@jdgrimes can you test the patch to see if it resolves the issue on your end?

[jdgrimes]

Replying to adamsilverstein:

@jdgrimes can you test the patch to see if it resolves the issue on your end?

Yep, that fixes it for me.

[helen]

This wasn't introduced in 4.3, so I'm not inclined to put it into a point release. Let's get it in, though.

[helen]

@wonderboymusic noted an unnecessary usage of this.$el on a quick review request from me, he's going to do a final review and commit.

[afercia]

There's one more thing to consider: the CPU usage while the spinner is being displayed. As per the testing run on #33311 applying an animated GIF as background on a so large area forces browsers to continuously repaint the whole area. For example, here's what Chrome does. The console "Enable paint flashing" option will allow you to actually see the re-painted area. There's a similar option in Firefox's console.
To minimize the high CPU usage, the spinner should be used as background of the smallest possible area (20px x 20px) as experimented on #33311 using CSS generated content.

[afercia]

Refreshed patch. Using the same CSS technique experimented in #33311 the re-painted area is now just 20px x 20px, see the screenshot below. The CPU usage is now noticeably lower in Chrome and a bit lower in Firefox even while the spinner is being displayed. I'd recommend to consider using this CSS technique wherever a background spinner is used in the admin.
Tried also to cache a bit this.$el.

[wonderboymusic]

In 35281:

Theme Installer: toggle iframe-ready on the overlay to control the spinner animated GIF and high CPU usage.

Props afercia, adamsilverstein.
Fixes #33322.

[SergeyBiryukov]

In 35310:

Autoprefixer for [35281].

See #33322.