Bug 88555 – MediaController.currentTime should be kept stable during script execution.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

MediaController.currentTime should be kept stable during script execution.

[Jer Noble]

Created attachment 146332 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/11616954>

[Darin Adler]

Comment on attachment 146332 [details]
Patch

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

> Source/WebCore/html/MediaController.cpp:50
> +    , m_position(-1)

To me, -1 does not seem like a great floating point value to mean "not set". I’d suggest a named constant or a separate boolean. I suppose the named constant could have the value -1. Although I would probably chose something else, like NaN.

> Source/WebCore/html/MediaController.h:152
> +    mutable Timer<MediaController> m_clearPositionTimer;

Having our own unique zero-duration timer seems a bit unpleasant. Is there any other good solution? Maybe there is a WebKit-wide concept of “exiting from script” that could work. I seem to recall some Chromium engineers proposing or perhaps even implementing something like that.

[Jer Noble]

Comment on attachment 146332 [details]
Patch

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

>> Source/WebCore/html/MediaController.cpp:50
>> +    , m_position(-1)
> 
> To me, -1 does not seem like a great floating point value to mean "not set". I’d suggest a named constant or a separate boolean. I suppose the named constant could have the value -1. Although I would probably chose something else, like NaN.

I agree, and I have a future patch which will add a new enum called InvalidMediaTime.  We already use a "static const float invalidMediaTime = -1;" in HTMLMediaElement for this purpose.  We just need to promote it to a header somewhere.

>> Source/WebCore/html/MediaController.h:152
>> +    mutable Timer<MediaController> m_clearPositionTimer;
> 
> Having our own unique zero-duration timer seems a bit unpleasant. Is there any other good solution? Maybe there is a WebKit-wide concept of “exiting from script” that could work. I seem to recall some Chromium engineers proposing or perhaps even implementing something like that.

:-D

I've got an upcoming patch that does that too.  I actually thought the reverse, however.  That seemed a bit heavy-handed for something that could be solved with a zero-duration timer.

[Jer Noble]

(In reply to comment #4)
> (From update of attachment 146332 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146332&action=review
> 
> >> Source/WebCore/html/MediaController.cpp:50
> >> +    , m_position(-1)
> > 
> > To me, -1 does not seem like a great floating point value to mean "not set". I’d suggest a named constant or a separate boolean. I suppose the named constant could have the value -1. Although I would probably chose something else, like NaN.
> 
> I agree, and I have a future patch which will add a new enum called InvalidMediaTime.  We already use a "static const float invalidMediaTime = -1;" in HTMLMediaElement for this purpose.  We just need to promote it to a header somewhere.

And here is that patch: https://bugs.webkit.org/show_bug.cgi?id=88572

[Eric Carlson]

Comment on attachment 146332 [details]
Patch

r=me if you update to use MediaPlayer::invalidTime()

[Jer Noble]

Committed r123386: <http://trac.webkit.org/changeset/123386>