Bug 206860

Summary:

[iOS] Page throttling reasons are not initialized with the device low power mode

Product:

WebKit

Reporter:

Said Abou-Hallawa <sabouhallawa>

Component:

Animations

Assignee:

Said Abou-Hallawa <sabouhallawa>

Status:

RESOLVED CONFIGURATION CHANGED

Severity:

Normal

CC:

commit-queue, dino, simon.fraser, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=204713

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Description Said Abou-Hallawa 2020-01-27 20:11:50 PST

1. Settings/Battery/Turn "Low Power Mode" on
2. Launch MobileSafari
3. Go to https://www.scirra.com/labs/bugs/safari13raf/index.html

Result: FrameRate = 60 fps
Expected: FrameRate = 30 fps

4. Settings/Battery/Turn "Low Power Mode" off
5. Settings/Battery/Turn "Low Power Mode" on
6. Launch MobileSafari
7. Go to https://www.scirra.com/labs/bugs/safari13raf/index.html

Result: FrameRate = 30 fps

Comment 1 Said Abou-Hallawa 2020-01-27 20:13:39 PST

Created attachment 388962 [details]
Patch

Comment 2 Simon Fraser (smfr) 2020-01-28 13:07:55 PST

Comment on attachment 388962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388962&action=review

> Source/WebCore/page/Page.cpp:1166
> +        m_throttlingReasonsOverrideForTestingMask.add(ThrottlingReason::LowPowerMode);

isEnabled could bet set, but false, so always adding ThrottlingReason::LowPowerMode seems wrong here.

> Source/WebCore/page/Page.cpp:1411
> +    if (m_throttlingReasonsOverrideForTestingMask.contains(ThrottlingReason::LowPowerMode))
> +        return;
> +
>      if (isLowPowerModeEnabled == m_throttlingReasons.contains(ThrottlingReason::LowPowerMode))
>          return;

I feel like m_throttlingReasonsOverrideForTestingMask.contains should be consulted at read time, not write time. So you'd always set here, but the getters would consult m_throttlingReasonsOverrideForTestingMask.

Comment 3 Said Abou-Hallawa 2020-01-28 15:20:50 PST

The fix of this bug has to be included in the patch of https://bugs.webkit.org/show_bug.cgi?id=206839 since they are very related.

*** This bug has been marked as a duplicate of bug 206839 ***

Comment 4 Said Abou-Hallawa 2020-01-30 16:07:01 PST

Will fix this bug separately.

Comment 5 Said Abou-Hallawa 2020-01-30 16:47:44 PST

Created attachment 389304 [details]
Patch

Comment 6 WebKit Commit Bot 2020-01-30 20:33:18 PST

Comment on attachment 389304 [details]
Patch

Rejecting attachment 389304 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 389304, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: https://webkit-queues.webkit.org/results/13314306

Comment 7 Said Abou-Hallawa 2020-01-30 20:49:01 PST

Created attachment 389328 [details]
Patch

Comment 8 WebKit Commit Bot 2020-01-30 22:59:26 PST

Comment on attachment 389328 [details]
Patch

Clearing flags on attachment: 389328

Committed r255486: <https://trac.webkit.org/changeset/255486>

Comment 9 WebKit Commit Bot 2020-01-30 22:59:27 PST

All reviewed patches have been landed.  Closing bug.

Comment 10 Radar WebKit Bug Importer 2020-01-30 23:00:14 PST

<rdar://problem/59054827>

Comment 11 Said Abou-Hallawa 2020-02-13 09:16:58 PST

The fix was reverted in r256512. This should not a bug anymore.