Improve linearized pdf load/show time.

pdf/url_loader_wrapper_impl.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "pdf/url_loader_wrapper_impl.h"
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "net/http/http_util.h"
+#include "pdf/timer.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/cpp/logging.h"
+#include "ppapi/cpp/url_request_info.h"
+#include "ppapi/cpp/url_response_info.h"
+
+namespace chrome_pdf {
+namespace {

[Lei Zhang, 2016/10/25 18:32:32]

Another place where it would be good to have blank lines for anonymous
namespaces.

[snake, 2016/10/25 19:16:27]

Done.

+// We should read with delay to prevent block UI thread, and reduce CPU usage.
+const int kReadDelayMs = 2;
+
+pp::URLRequestInfo MakeRangeRequest(pp::Instance* const plugin_instance,

[Lei Zhang, 2016/10/25 18:32:32]

No need for const here either.

[snake, 2016/10/25 19:16:27]

Done.

+                                    const std::string& url,
+                                    const std::string& referrer_url,
+                                    uint32_t position,
+                                    uint32_t size) {
+  pp::URLRequestInfo request(plugin_instance);
+  request.SetURL(url);
+  request.SetMethod("GET");
+  request.SetFollowRedirects(true);

[Lei Zhang, 2016/10/25 18:32:32]

Also, this will conflict with https://codereview.chromium.org/2409423004/ which
is already in CQ - I'm setting it to false.

[snake, 2016/10/25 19:16:27]

Done.

[Lei Zhang, 2016/10/25 20:51:58]

The above CL landed, so be sure to sync one more time or the patch for this CL
may not apply.

+  request.SetCustomReferrerURL(referrer_url);
+
+  // According to rfc2616, byte range specifies position of the first and last
+  // bytes in the requested range inclusively. Therefore we should subtract 1
+  // from the position + size, to get index of the last byte that needs to be
+  // downloaded.
+  std::string str_header =
+      base::StringPrintf("Range: bytes=%d-%d", position, position + size - 1);
+  pp::Var header(str_header.c_str());
+  request.SetHeaders(header);
+
+  return request;
+}
+
+bool GetByteRangeFromStr(const std::string& content_range_str,
+                         int* start,
+                         int* end) {
+  std::string range = content_range_str;
+  if (!base::StartsWith(range, "bytes", base::CompareCase::INSENSITIVE_ASCII))
+    return false;
+
+  range = range.substr(strlen("bytes"));
+  std::string::size_type pos = range.find('-');
+  std::string range_end;
+  if (pos != std::string::npos)
+    range_end = range.substr(pos + 1);
+  base::TrimWhitespaceASCII(range, base::TRIM_LEADING, &range);
+  base::TrimWhitespaceASCII(range_end, base::TRIM_LEADING, &range_end);
+  *start = atoi(range.c_str());
+  *end = atoi(range_end.c_str());
+  return true;
+}
+
+// If the headers have a byte-range response, writes the start and end
+// positions and returns true if at least the start position was parsed.
+// The end position will be set to 0 if it was not found or parsed from the
+// response.
+// Returns false if not even a start position could be parsed.
+bool GetByteRangeFromHeaders(const std::string& headers, int* start, int* end) {
+  net::HttpUtil::HeadersIterator it(headers.begin(), headers.end(), "\n");
+  while (it.GetNext()) {
+    if (base::LowerCaseEqualsASCII(it.name(), "content-range")) {
+      if (GetByteRangeFromStr(it.values().c_str(), start, end))
+        return true;
+    }
+  }
+  return false;
+}
+}  // namespace
+
+class URLLoaderWrapperImpl::ReadStarter : public Timer {
+ public:
+  explicit ReadStarter(URLLoaderWrapperImpl* owner)
+      : Timer(kReadDelayMs), owner_(owner) {}
+  ~ReadStarter() override {}
+
+  // Timer overrides:
+  void OnTimer() override { owner_->ReadResponseBodyImpl(); }
+
+ private:
+  URLLoaderWrapperImpl* owner_;
+};
+
+URLLoaderWrapperImpl::URLLoaderWrapperImpl(pp::Instance* plugin_instance,
+                                           const pp::URLLoader& url_loader)
+    : plugin_instance_(plugin_instance),
+      url_loader_(url_loader),
+      callback_factory_(this) {
+  SetHeadersFromLoader();
+}
+
+URLLoaderWrapperImpl::~URLLoaderWrapperImpl() {
+  Close();
+}
+
+int URLLoaderWrapperImpl::GetContentLength() const {
+  return content_length_;
+}
+
+bool URLLoaderWrapperImpl::IsAcceptRangesBytes() const {
+  return accept_ranges_bytes_;
+}
+
+bool URLLoaderWrapperImpl::IsContentEncoded() const {
+  return content_encoded_;
+}
+
+std::string URLLoaderWrapperImpl::GetContentType() const {
+  return content_type_;
+}
+std::string URLLoaderWrapperImpl::GetContentDisposition() const {
+  return content_disposition_;
+}
+
+int URLLoaderWrapperImpl::GetStatusCode() const {
+  return url_loader_.GetResponseInfo().GetStatusCode();
+}
+
+bool URLLoaderWrapperImpl::IsMultipart() const {
+  return is_multipart_;
+}
+
+bool URLLoaderWrapperImpl::GetByteRange(int* start, int* end) const {
+  DCHECK(start);
+  DCHECK(end);
+  *start = byte_range_.start();
+  *end = byte_range_.end();
+  return byte_range_.IsValid();
+}
+
+bool URLLoaderWrapperImpl::GetDownloadProgress(
+    int64_t* bytes_received,
+    int64_t* total_bytes_to_be_received) const {
+  return url_loader_.GetDownloadProgress(bytes_received,
+                                         total_bytes_to_be_received);
+}
+
+void URLLoaderWrapperImpl::Close() {
+  url_loader_.Close();
+  read_starter_.reset();
+}
+
+void URLLoaderWrapperImpl::OpenRange(const std::string& url,
+                                     const std::string& referrer_url,
+                                     uint32_t position,
+                                     uint32_t size,
+                                     const pp::CompletionCallback& cc) {
+  did_open_callback_ = cc;
+  pp::CompletionCallback callback =
+      callback_factory_.NewCallback(&URLLoaderWrapperImpl::DidOpen);
+  int rv = url_loader_.Open(
+      MakeRangeRequest(plugin_instance_, url, referrer_url, position, size),
+      callback);
+  if (rv != PP_OK_COMPLETIONPENDING)
+    callback.Run(rv);
+}
+
+void URLLoaderWrapperImpl::ReadResponseBody(char* buffer,
+                                            int buffer_size,
+                                            const pp::CompletionCallback& cc) {
+  did_read_callback_ = cc;
+  buffer_ = buffer;
+  buffer_size_ = buffer_size;
+  read_starter_ = base::MakeUnique<ReadStarter>(this);
+}
+
+void URLLoaderWrapperImpl::ReadResponseBodyImpl() {
+  read_starter_.reset();
+  pp::CompletionCallback callback =
+      callback_factory_.NewCallback(&URLLoaderWrapperImpl::DidRead);
+  int rv = url_loader_.ReadResponseBody(buffer_, buffer_size_, callback);
+  if (rv != PP_OK_COMPLETIONPENDING) {
+    callback.Run(rv);
+  }
+}
+
+void URLLoaderWrapperImpl::SetResponseHeaders(
+    const std::string& response_headers) {
+  response_headers_ = response_headers;
+  ParseHeaders();
+}
+
+void URLLoaderWrapperImpl::ParseHeaders() {
+  content_length_ = -1;
+  accept_ranges_bytes_ = false;
+  content_encoded_ = false;
+  content_type_.clear();
+  content_disposition_.clear();
+  multipart_boundary_.clear();
+  byte_range_ = gfx::Range::InvalidRange();
+  is_multipart_ = false;
+
+  if (response_headers_.empty())
+    return;
+
+  net::HttpUtil::HeadersIterator it(response_headers_.begin(),
+                                    response_headers_.end(), "\n");
+  while (it.GetNext()) {
+    if (base::LowerCaseEqualsASCII(it.name(), "content-length")) {
+      content_length_ = atoi(it.values().c_str());
+    } else if (base::LowerCaseEqualsASCII(it.name(), "accept-ranges")) {
+      accept_ranges_bytes_ = base::LowerCaseEqualsASCII(it.values(), "bytes");
+    } else if (base::LowerCaseEqualsASCII(it.name(), "content-encoding")) {
+      content_encoded_ = true;
+    } else if (base::LowerCaseEqualsASCII(it.name(), "content-type")) {
+      content_type_ = it.values();
+      size_t semi_colon_pos = content_type_.find(';');
+      if (semi_colon_pos != std::string::npos) {
+        content_type_ = content_type_.substr(0, semi_colon_pos);
+      }
+      base::TrimWhitespaceASCII(content_type_, base::TRIM_ALL, &content_type_);
+      // multipart boundary.
+      std::string type = base::ToLowerASCII(it.values());
+      if (base::StartsWith(type, "multipart/", base::CompareCase::SENSITIVE)) {
+        const char* boundary = strstr(type.c_str(), "boundary=");
+        DCHECK(boundary);
+        if (boundary) {
+          multipart_boundary_ = std::string(boundary + 9);
+          is_multipart_ = !multipart_boundary_.empty();
+        }
+      }
+    } else if (base::LowerCaseEqualsASCII(it.name(), "content-disposition")) {
+      content_disposition_ = it.values();
+    } else if (base::LowerCaseEqualsASCII(it.name(), "content-range")) {
+      int start = 0;
+      int end = 0;
+      if (GetByteRangeFromStr(it.values().c_str(), &start, &end)) {
+        byte_range_ = gfx::Range(start, end);
+      }
+    }
+  }
+}
+
+void URLLoaderWrapperImpl::DidOpen(int32_t result) {
+  SetHeadersFromLoader();
+  did_open_callback_.Run(result);
+}
+
+void URLLoaderWrapperImpl::DidRead(int32_t result) {
+  if (multi_part_processed_) {
+    // Reset this flag so we look inside the buffer in calls of DidRead for this
+    // response only once.  Note that this code DOES NOT handle multi part
+    // responses with more than one part (we don't issue them at the moment, so
+    // they shouldn't arrive).
+    is_multipart_ = false;
+  }
+  if (result <= 0 || !is_multipart_) {
+    did_read_callback_.Run(result);
+    return;
+  }
+  if (result <= 2) {
+    // TODO(art-snake): Accumulate data for parse headers.
+    did_read_callback_.Run(result);
+    return;
+  }
+
+  char* start = buffer_;
+  size_t length = result;
+  multi_part_processed_ = true;
+  for (int i = 2; i < result; ++i) {
+    if ((buffer_[i - 1] == '\n' && buffer_[i - 2] == '\n') ||

[Lei Zhang, 2016/10/25 18:24:52]

Adding an IsEndLine(const char* buffer, size_t size) helper function can help
make this a bit more readable.

[snake, 2016/10/25 19:16:27]

In this we should check double endl (with or without "\r"), which is headers
end. Additional helper function increasing difficulty for read of this check.

For example:

IsEndLineWithoutR(buffer_[i-1]) && IsEndLineWithoutR(buffer_[i-2]) ||
(i>=4 && IsEndLineWithR(buffer_+i-4, 2) && IsEndLineWithR(buffer_+i-2, 2))


JFYI: This just moved from: 
https://cs.chromium.org/chromium/src/pdf/document_loader.cc?q=document_loader...

[Lei Zhang, 2016/10/25 19:40:37]

Yes, I understand we are checking more than one thing. To better and more
correctly state my thoughts, I'm proposing:

bool IsDoubleEndLine(const char* buffer, int index) {
  if (index < 2)
    return false;

  if (buffer_[i - 1] == '\n' && buffer_[i - 2] == '\n')
    return true;

  if (index < 4)
    return false;

  return buffer_[i - 1] == '\n' && buffer_[i - 2] == '\r' && buffer_[i - 3] ==
'\n' && buffer_[i - 4] == '\r');
}

I'm just writing it out in an expanded form for clarity. You can write it more
IsDoubleEndLine() compactly.

Then in this function, we can just make a single helper function call that is
easier to understand than a big blob of comparisons.
Understood, but I think the original code can use the same refactoring. If you
like to leave it as is, that's fine too. Someone else can clean it up later.

[snake, 2016/10/25 20:07:17]

Done.

+        (i >= 4 && buffer_[i - 1] == '\n' && buffer_[i - 2] == '\r' &&
+         buffer_[i - 3] == '\n' && buffer_[i - 4] == '\r')) {
+      int start_pos, end_pos;

[Lei Zhang, 2016/10/25 18:24:52]

One declaration per line please.

[snake, 2016/10/25 19:16:27]

Done.

+      if (GetByteRangeFromHeaders(std::string(buffer_, i), &start_pos,
+                                  &end_pos)) {
+        byte_range_ = gfx::Range(start_pos, end_pos);
+        start += i;
+        length -= i;
+      }
+      break;
+    }
+  }
+  result = length;
+  if (result == 0) {
+    // Continue receiving.
+    return ReadResponseBodyImpl();
+  }
+  DCHECK(result > 0);
+  memmove(buffer_, start, result);
+
+  did_read_callback_.Run(result);
+}
+
+void URLLoaderWrapperImpl::SetHeadersFromLoader() {
+  pp::URLResponseInfo response = url_loader_.GetResponseInfo();
+  pp::Var headers_var = response.GetHeaders();
+
+  SetResponseHeaders(headers_var.is_string() ? headers_var.AsString() : "");
+}
+
+}  // namespace chrome_pdf