<div dir="ltr"><div>The problem I was having was the spice-html5 client was throwing exceptions for trying to access that property in cases where it is not a valid value.<br><br></div>I got to the error because I'm developing a plugin for cockpit that makes use of spice-html5 for connecting to VMs, and when spice-html5 starts displaying graphics, it throws this error repeatedly, causing cockpit to show an awfull "Oops" message.<br><div><div><div><br>It just failed throwing exceptions to console. It stopped the playback when failed, so with the change it stops anyway but without throwing errors.<br><br></div><div>I hope this answer your questions. My knowledge about spice-html5 or spice is limited, so I don't know if this info is enough for you.<br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 28, 2016 at 12:47 PM, Christophe Fergeau <span dir="ltr"><<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey,<br>
<br>
Thanks for the patch!<br>
<br>
I'd expand a bit in the commit log, that this.source_buffer can be used<br>
before being checked for null, and that this commit moves the check<br>
before the first use of this.source_buffer.<br>
You could also describe what happens when this triggers/how this<br>
triggers (I assume playback stops?)<br>
<span class=""><br>
On Thu, Jul 28, 2016 at 10:50:05AM +0100, Oliver Gutierrez wrote:<br>
> ---<br>
>  playback.js | 9 ++++-----<br>
>  1 file changed, 4 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/playback.js b/playback.js<br>
> index 9659381..b5954da 100644<br>
> --- a/playback.js<br>
> +++ b/playback.js<br>
> @@ -107,21 +107,20 @@ SpicePlaybackConn.prototype.process_channel_message = function(msg)<br>
>               So we do two things.  First, we seek forward.  Second, we compute how much of a gap<br>
>               there would have been, and essentially eliminate it.<br>
>          */<br>
> +        if (! this.source_buffer)<br>
> +            return true;<br>
> +<br>
>          if (this.last_data_time && data.time >= (this.last_data_time + GAP_DETECTION_THRESHOLD))<br>
>          {<br>
>              this.skip_until = data.time;<br>
> -            this.gap_time = (data.time - this.start_time) -<br>
> +            this.gap_time = (data.time - this.start_time) -<br>
<br>
</span>This looks like an unrelated whitespace change<br>
<br>
<br>
Looks good otherwise!<br>
<br>
Reviewed-by: Christophe Fergeau <<a href="mailto:cfergeau@redhat.com">cfergeau@redhat.com</a>><br>
<span class="HOEnZb"><font color="#888888"><br>
Christophe<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><span>Oliver Gutierrez<br>Associate Software Engineer - Desktop Management tools<br>Red Hat</span></div></div>
</div>