Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/callstack gh 860 #861

Closed
wants to merge 2 commits into from
Closed

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Dec 2, 2013

This addresses bug #860.
First, we check to make sure that we got a duration from the techGet call, which may return nothing if the tech isn't ready yet, and then we call to update the duration if a duration exists. Otherwise, we don't.
If we don't have a cached value for duration, we should default to returning zero.

This is so that we can call Player#duration only if a duration exists.
Otherwise, we get into an infinite loop and blow up the call stack.
@@ -681,7 +686,7 @@ vjs.Player.prototype.duration = function(seconds){
this.onDurationChange();
}

return this.cache_.duration;
return this.cache_.duration || 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think zero would ever get used here. cache_.duration would either be undefined or a number. If it's undefined, onDurationChange on line 686 will set the value before this line is reached.

I think it will take some re-architecting to get it to default to zero properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually does reach here in the special case of when the call stack blows up. In that case we never get a value set in the cache.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. lgtm

@heff heff closed this in 432b60f Dec 3, 2013
@gkatsev gkatsev deleted the bug/callstack_GH-860 branch August 12, 2015 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants