Bug 196458 – Remove legacy webkitRequestAnimationFrame time quirk

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Simon Fraser (smfr)]

We should remove m_useLegacyTimeBase from webkitRequestAnimationFrame() (and eventually remove webkitRequestAnimationFrame altogether).

[Radar WebKit Bug Importer]

<rdar://problem/49490207>

[Chris Dumez]

Created attachment 366524 [details]
Patch

[Chris Dumez]

Created attachment 366525 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 366525 [details]
Patch

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

> Source/WebCore/dom/ScriptedAnimationController.cpp:-219
> -            if (callback->m_useLegacyTimeBase)
> -                callback->handleEvent(legacyHighResNowMs);
> -            else
> -                callback->handleEvent(highResNowMs);

This change will conflict with my patch in https://bugs.webkit.org/show_bug.cgi?id=177484. But this is fine I will resolve the conflict if this patches before mine. Actually this code is confusing and it is good that we are going to remove it.

[Simon Fraser (smfr)]

Comment on attachment 366525 [details]
Patch

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

> Source/WebCore/page/DOMWindow.cpp:1707
> +    if (auto* document = this->document())
> +        document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "webkitRequestAnimationFrame() is deprecated and will be removed. Please use requestAnimationFrame() instead."_s);

Maybe this should be logged only once per document per load?

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 366525 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366525&action=review
> 
> > Source/WebCore/page/DOMWindow.cpp:1707
> > +    if (auto* document = this->document())
> > +        document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "webkitRequestAnimationFrame() is deprecated and will be removed. Please use requestAnimationFrame() instead."_s);
> 
> Maybe this should be logged only once per document per load?

Do we have any precedent for doing this? I thought we usually logged on every call.

[Simon Fraser (smfr)]

(In reply to Chris Dumez from comment #6)
> (In reply to Simon Fraser (smfr) from comment #5)
> > Comment on attachment 366525 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=366525&action=review
> > 
> > > Source/WebCore/page/DOMWindow.cpp:1707
> > > +    if (auto* document = this->document())
> > > +        document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "webkitRequestAnimationFrame() is deprecated and will be removed. Please use requestAnimationFrame() instead."_s);
> > 
> > Maybe this should be logged only once per document per load?
> 
> Do we have any precedent for doing this? I thought we usually logged on
> every call.

    static bool firstTime = true;
    if (firstTime && context().scriptExecutionContext()) {
        context().scriptExecutionContext()->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "AudioBufferSourceNode 'looping' attribute is deprecated.  Use 'loop' instead."_s);
        firstTime = false;
    }

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #7)
> (In reply to Chris Dumez from comment #6)
> > (In reply to Simon Fraser (smfr) from comment #5)
> > > Comment on attachment 366525 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=366525&action=review
> > > 
> > > > Source/WebCore/page/DOMWindow.cpp:1707
> > > > +    if (auto* document = this->document())
> > > > +        document->addConsoleMessage(MessageSource::JS, MessageLevel::Warning, "webkitRequestAnimationFrame() is deprecated and will be removed. Please use requestAnimationFrame() instead."_s);
> > > 
> > > Maybe this should be logged only once per document per load?
> > 
> > Do we have any precedent for doing this? I thought we usually logged on
> > every call.
> 
>     static bool firstTime = true;
>     if (firstTime && context().scriptExecutionContext()) {
>        
> context().scriptExecutionContext()->addConsoleMessage(MessageSource::JS,
> MessageLevel::Warning, "AudioBufferSourceNode 'looping' attribute is
> deprecated.  Use 'loop' instead."_s);
>         firstTime = false;
>     }

Ugh, this relies on a static which means it would survive loads and potentially not log when it should.

[Chris Dumez]

Created attachment 366613 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 366613 [details]
Patch

Clearing flags on attachment: 366613

Committed r243810: <https://trac.webkit.org/changeset/243810>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.