[webkit-reviews] review granted: [Bug 222381] prefers-reduced-motion is not reactive on iOS : [Attachment 427414] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 222381: prefers-reduced-motion is not reactive on iOS
https://bugs.webkit.org/show_bug.cgi?id=222381

Attachment 427414: Patch

https://bugs.webkit.org/attachment.cgi?id=427414&action=review




--- Comment #17 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 427414
  --> https://bugs.webkit.org/attachment.cgi?id=427414
Patch

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

r=me

> Source/WebKit/ChangeLog:10
> +	   the notification in the WebProcess. Resolve this by updating the
cached settings values in Accessibility when a

I think it might be more accurate to say:

"... race between the notification that an accessibility preference has been
changed by the user, and the the in-memory Accessibility preference cache in
the WebContent process. We resolve this by using new SPI to clear the cached
Accessibility settings when we are notified of an accessibility change so they
always reflect the user's most recent setting."

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1127
> +	       WTFLogAlways("Updating Web Accessibility settings.");

I suggest using RELEASE_LOG_IF_ALLOWED here, so that the error message is
formatted in a way that our triage script can easily consume.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1129
> +	   }

I suggest logging when the SPI doesn't exist so we can review logs and verify
this is why a user is not seeing the right behavior.

I also suggest using RELEASE_LOG_ERROR_IF_ALLOWED for this non-SPI case so we
can catch it in triage.