Improve linearized pdf load/show time.

pdf/url_loader_wrapper_impl.cc

Index: pdf/url_loader_wrapper_impl.cc
diff --git a/pdf/url_loader_wrapper_impl.cc b/pdf/url_loader_wrapper_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..79fe12ec80188650faf5e06cfa8c837481433111
--- /dev/null
+++ b/pdf/url_loader_wrapper_impl.cc
@@ -0,0 +1,296 @@
+// Copyright (c) 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/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 {
+// We should read with delay to prevent block UI thread, and reduce CPU usage.
+const int kReadDelayMs = 2;
+
+pp::URLRequestInfo MakeRangeRequest(pp::Instance* plugin_instance,
+                                    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);
+  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)) {

[Lei Zhang, 2016/10/21 09:33:10]

How about early returns instead?

if (!base::StartsWith(content_range_str, "bytes",
base::CompareCase::INSENSITIVE_ASCII)) {
  return false;
}

std::string range = content_range_str.substr(strlen("bytes"));
// everything gets less indenting
return true;

[snake, 2016/10/21 15:13:16]

Done.

+    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());

[Lei Zhang, 2016/10/21 09:33:10]

What happens if |range| contains garbage?

[snake, 2016/10/21 15:13:16]

No proplems with this before:
https://cs.chromium.org/chromium/src/pdf/document_loader.cc?rcl=0&l=41

[Lei Zhang, 2016/10/25 06:01:49]

Got it. You are just moving code around. I will make a note of this and
re-examine it later, separately.

+    *end = atoi(range_end.c_str());
+    return true;
+  }
+  return false;
+}
+
+// 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 {
+  PP_DCHECK(start);

[Lei Zhang, 2016/10/21 09:33:10]

You can just regular DCHECK().

[snake, 2016/10/21 15:13:16]

Done.

+  PP_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_.reset(new ReadStarter(this));

[Lei Zhang, 2016/10/21 09:33:10]

base:MakeUnique

[snake, 2016/10/21 15:13:16]

Done.

+}
+
+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;

[Lei Zhang, 2016/10/21 09:33:10]

nit: I like blank lines after returns, because that is a natural separator
between different parts of the code.

[snake, 2016/10/21 15:13:16]

Done.

+  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=");
+        PP_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) {

[Lei Zhang, 2016/10/21 09:33:10]

Again, early return if <= 0?

[snake, 2016/10/21 15:13:16]

no just return, additionaly we should call did_read_callback_.

[Lei Zhang, 2016/10/25 06:01:49]

What I mean is:

if (result <= 0) {
  did_read_callback_.Run(result);
  return;
}

...


Yes, this requires calling did_read_callback_.Run() in a second location, but it
feels like an ok trade-off.

[snake, 2016/10/25 13:57:34]

Done.

+    char* start = buffer_;
+    size_t length = result;
+    if (!multi_part_processed_ && is_multipart_ && result > 2) {
+      multi_part_processed_ = true;
+      for (int i = 2; i < result; ++i) {
+        if ((buffer_[i - 1] == '\n' && buffer_[i - 2] == '\n') ||
+            (i >= 4 && buffer_[i - 1] == '\n' && buffer_[i - 2] == '\r' &&
+             buffer_[i - 3] == '\n' && buffer_[i - 4] == '\r')) {
+          int start_pos, end_pos;
+          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) {
+        return;
+      }
+      PP_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();
+
+  if (headers_var.is_string()) {

[Lei Zhang, 2016/10/21 09:33:10]

SetResponseHeaders(headers_var.is_string() ? headers_var.AsString() : "");

[snake, 2016/10/21 15:13:16]

Done.

+    SetResponseHeaders(headers_var.AsString());
+  } else {
+    SetResponseHeaders("");
+  }
+}
+
+}  // namespace chrome_pdf