CEF Forum

I have a file read handler that handles videos served in web pages within my CEF browser window. This custom handler manually parsed headers to provide full support for range requests which are common in video serving. I'm working to remove the now deprecated ProcessRequest and ReadResponse methods from this handler and see that there is now a Skip() method the seems to indicate that range requests are now supported internally (by cef itself). I'm wondering if that is indeed the case and if the full range requests spec is supported (i.e. single and multi-part, etc.).

I don't believe the full spec is supported. Also, there have been reports on this forum that CefResourceHandler + video might not be behaving correctly. We don't currently have unit test coverage for this use case so it's hard to say for sure.

To provide a bit more info, there are comments in StreamReaderURLLoader::ParseRange about not supporting multi-range requests.

That looks promising, at least that their is direct support for parts of the spec. I'm wondering if all the correct response headers, etc are handled. Video works for us with the old handler methods. I'm going to try to remove my custom range handling and see if it works with the new Seek/Read calls and will report back on whether it works with our video usage.

Okay I finally got around to updating our resource handlers that deal with things like streaming video and single byte range handling does seem to work but there are some catches. I wanted to document them here in case anyone else runs into them.

First, the Skip method does work as needed but your handler must still parse any range request headers and respond appropriately with the 206 status code, content-range and content-length headers. I was thinking those headers may be handled for me but they are not.

Second, there is a bad DCHECK that will cause debug builds to fail unnecessarily. In StreamReaderURLLoader::OnReaderSkipCompleted below the initial range request for "0-" meaning beginning of file to the end will cause expected_content_length to be negative as first_byte_position will be zero, last_byte_position will be -1 (unknown). This also happens in subsequent range requests such as "18000-" meaning byte 18000 to the end. Likewise that calculation results in -18000. I skipped the DCHECK in the debugger and all the code seems to work so I think that check needs to be removed or changed to handle the cases I mention. In fact, I'm not sure what expected_content_length is even needed for in that method.

Code: Select all

void StreamReaderURLLoader::OnReaderSkipCompleted(int64_t bytes_skipped) {
  DCHECK(thread_checker_.CalledOnValidThread());

  if (!byte_range_valid()) {
    // Expected content length is unspecified.
    HeadersComplete(net::HTTP_OK, -1);
  } else if (bytes_skipped == byte_range_.first_byte_position()) {
    // We skipped the expected number of bytes.
    int64_t expected_content_length = byte_range_.last_byte_position() -
                                      byte_range_.first_byte_position() + 1;
    DCHECK_GE(expected_content_length, 0);
    HeadersComplete(net::HTTP_OK, expected_content_length);
  } else {
    RequestComplete(bytes_skipped < 0 ? bytes_skipped : net::ERR_FAILED);
  }
}


Thanks for the update. The OnReaderSkipCompleted issue is also discussed in viewtopic.php?f=6&t=17309.

Based on the ResourceResponseWrapper::GetResponseHeaders snippet below

Code: Select all

    // A |content_length| value may already be specified if the request included
    // a Range header.
    if (response_length >= 0 && *content_length == -1)
      *content_length = response_length;


It seems like StreamReaderURLLoader::OnReaderSkipCompleted should set the expected length to -1 if it is not calculable. So

Code: Select all

   else if (bytes_skipped == byte_range_.first_byte_position()) {
    // We skipped the expected number of bytes.
    int64_t expected_content_length = byte_range_.last_byte_position() -
                                      byte_range_.first_byte_position() + 1;
    DCHECK_GE(expected_content_length, 0);
    HeadersComplete(net::HTTP_OK, expected_content_length);
}


Probably should become something like

Code: Select all

   else if (bytes_skipped == byte_range_.first_byte_position()) {
    // We skipped the expected number of bytes.
    int64_t expected_content_length = -1;  // Expected length not known.
    if (byte_range_.HasLastBytePosition()) {
      expected_content_length = byte_range_.last_byte_position() -
                                                byte_range_.first_byte_position() + 1;
      DCHECK_GE(expected_content_length, 0);
    }
    HeadersComplete(net::HTTP_OK, expected_content_length);
}


I think that allows the handlers GetResponseHeaders() method to respond with the correct content length, assuming it reads and processes the range header correctly. Then ResourceResponseWrapper::GetResponseHeaders will just use that value. If the range was calculable, then it should work normally.

Sounds reasonable. Can you submit a PR? Thanks.

Okay, will do.