Skip to content

Commit

Permalink
[DevTools] Use binary in protocol.pdl for screenshots and PDFs.
Browse files Browse the repository at this point in the history
These changes affect the headless shell also, and similar to this,
C++ code elsewhere which is generated from the .pdl file, e.g.
client code, may be affected. However since the wire format
isn't changing, such code continues to work if it sticks with
string (and continues to encode/decode base64 manually).

Relevant prereqs:
Supporting binary in .pdl files:
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1285695
Rolled into Chromium / v8.

Add protocol::content::Binary to protocol_string{.h,.cc}.
https://chromium-review.googlesource.com/c/chromium/src/+/1280555
https://chromium-review.googlesource.com/c/chromium/src/+/1281166

[DevTools] Add blink::protocol::Binary to v8_inspector_string{.cc,.h}.
https://chromium-review.googlesource.com/c/chromium/src/+/1280210

Bug: chromium:891377
Change-Id: Ie959e251730b68929440385be24075fb430bcb0b
Reviewed-on: https://chromium-review.googlesource.com/c/1303410
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603665}
  • Loading branch information
Johannes Henkel authored and Commit Bot committed Oct 29, 2018
1 parent 18079a3 commit c58a4ae
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 114 deletions.
27 changes: 10 additions & 17 deletions content/browser/devtools/protocol/page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <utility>
#include <vector>

#include "base/base64.h"
#include "base/bind.h"
#include "base/location.h"
#include "base/memory/ref_counted.h"
Expand Down Expand Up @@ -75,9 +74,9 @@ constexpr int kFrameRetryDelayMs = 100;
constexpr int kCaptureRetryLimit = 2;
constexpr int kMaxScreencastFramesInFlight = 2;

std::string EncodeImage(const gfx::Image& image,
const std::string& format,
int quality) {
Binary EncodeImage(const gfx::Image& image,
const std::string& format,
int quality) {
DCHECK(!image.IsEmpty());

scoped_refptr<base::RefCountedMemory> data;
Expand All @@ -90,20 +89,14 @@ std::string EncodeImage(const gfx::Image& image,
}

if (!data || !data->front())
return std::string();
return protocol::Binary();

std::string base_64_data;
base::Base64Encode(
base::StringPiece(reinterpret_cast<const char*>(data->front()),
data->size()),
&base_64_data);

return base_64_data;
return Binary::fromRefCounted(data);
}

std::string EncodeSkBitmap(const SkBitmap& image,
const std::string& format,
int quality) {
Binary EncodeSkBitmap(const SkBitmap& image,
const std::string& format,
int quality) {
return EncodeImage(gfx::Image::CreateFrom1xBitmap(image), format, quality);
}

Expand Down Expand Up @@ -1036,8 +1029,8 @@ void PageHandler::ScreencastFrameCaptured(

void PageHandler::ScreencastFrameEncoded(
std::unique_ptr<Page::ScreencastFrameMetadata> page_metadata,
const std::string& data) {
if (data.empty()) {
const protocol::Binary& data) {
if (data.size() == 0) {
--frames_in_flight_;
return; // Encode failed.
}
Expand Down
2 changes: 1 addition & 1 deletion content/browser/devtools/protocol/page_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class PageHandler : public DevToolsDomainHandler,
const SkBitmap& bitmap);
void ScreencastFrameEncoded(
std::unique_ptr<Page::ScreencastFrameMetadata> metadata,
const std::string& data);
const protocol::Binary& data);

void ScreenshotCaptured(
std::unique_ptr<CaptureScreenshotCallback> callback,
Expand Down
27 changes: 7 additions & 20 deletions headless/app/headless_shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <string>
#include <utility>

#include "base/base64.h"
#include "base/base_switches.h"
#include "base/bind.h"
#include "base/callback.h"
Expand Down Expand Up @@ -483,7 +482,7 @@ void HeadlessShell::OnPDFCreated(

void HeadlessShell::WriteFile(const std::string& file_path_switch,
const std::string& default_file_name,
const std::string& base64_data) {
const protocol::Binary& data) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

base::FilePath file_name =
Expand All @@ -492,25 +491,17 @@ void HeadlessShell::WriteFile(const std::string& file_path_switch,
if (file_name.empty())
file_name = base::FilePath().AppendASCII(default_file_name);

std::string decoded_data;
if (!base::Base64Decode(base64_data, &decoded_data)) {
LOG(ERROR) << "Failed to decode base64 data";
OnFileOpened(std::string(), file_name, base::File::FILE_ERROR_FAILED);
return;
}

file_proxy_ = std::make_unique<base::FileProxy>(file_task_runner_.get());
if (!file_proxy_->CreateOrOpen(
file_name, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE,
base::BindOnce(&HeadlessShell::OnFileOpened,
weak_factory_.GetWeakPtr(), decoded_data,
file_name))) {
weak_factory_.GetWeakPtr(), data, file_name))) {
// Operation could not be started.
OnFileOpened(std::string(), file_name, base::File::FILE_ERROR_FAILED);
OnFileOpened(protocol::Binary(), file_name, base::File::FILE_ERROR_FAILED);
}
}

void HeadlessShell::OnFileOpened(const std::string& decoded_data,
void HeadlessShell::OnFileOpened(const protocol::Binary& data,
const base::FilePath file_name,
base::File::Error error_code) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Expand All @@ -520,16 +511,12 @@ void HeadlessShell::OnFileOpened(const std::string& decoded_data,
<< base::File::ErrorToString(error_code);
return;
}

auto buf = base::MakeRefCounted<net::IOBufferWithSize>(decoded_data.size());
memcpy(buf->data(), decoded_data.data(), decoded_data.size());

if (!file_proxy_->Write(
0, buf->data(), buf->size(),
0, reinterpret_cast<const char*>(data.data()), data.size(),
base::BindOnce(&HeadlessShell::OnFileWritten,
weak_factory_.GetWeakPtr(), file_name, buf->size()))) {
weak_factory_.GetWeakPtr(), file_name, data.size()))) {
// Operation may have completed successfully or failed.
OnFileWritten(file_name, buf->size(), base::File::FILE_ERROR_FAILED, 0);
OnFileWritten(file_name, data.size(), base::File::FILE_ERROR_FAILED, 0);
}
}

Expand Down
6 changes: 3 additions & 3 deletions headless/app/headless_shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ class HeadlessShell : public HeadlessWebContents::Observer,

void OnPDFCreated(std::unique_ptr<page::PrintToPDFResult> result);

void WriteFile(const std::string& switch_string,
void WriteFile(const std::string& file_path_switch,
const std::string& default_file_name,
const std::string& base64_data);
void OnFileOpened(const std::string& decoded_data,
const protocol::Binary& data);
void OnFileOpened(const protocol::Binary& data,
const base::FilePath file_name,
base::File::Error error_code);
void OnFileWritten(const base::FilePath file_name,
Expand Down
15 changes: 10 additions & 5 deletions headless/lib/browser/headless_print_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>
#include <vector>

#include "base/memory/ref_counted_memory.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
#include "base/strings/string_split.h"
Expand Down Expand Up @@ -145,7 +146,8 @@ void HeadlessPrintManager::GetPDFContents(content::RenderFrameHost* rfh,
DCHECK(callback);

if (callback_) {
std::move(callback).Run(SIMULTANEOUS_PRINT_ACTIVE, std::string());
std::move(callback).Run(SIMULTANEOUS_PRINT_ACTIVE,
base::MakeRefCounted<base::RefCountedString>());
return;
}
printing_rfh_ = rfh;
Expand Down Expand Up @@ -311,10 +313,13 @@ void HeadlessPrintManager::ReleaseJob(PrintResult result) {
return;
}

if (result == PRINT_SUCCESS)
std::move(callback_).Run(result, std::move(data_));
else
std::move(callback_).Run(result, std::string());
if (result == PRINT_SUCCESS) {
std::move(callback_).Run(result,
base::RefCountedString::TakeString(&data_));
} else {
std::move(callback_).Run(result,
base::MakeRefCounted<base::RefCountedString>());
}
printing_rfh_->Send(new PrintMsg_PrintingDone(printing_rfh_->GetRoutingID(),
result == PRINT_SUCCESS));
Reset();
Expand Down
4 changes: 3 additions & 1 deletion headless/lib/browser/headless_print_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <string>
#include <vector>

#include "base/memory/ref_counted_memory.h"
#include "components/printing/browser/print_manager.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents_user_data.h"
Expand Down Expand Up @@ -65,7 +66,8 @@ class HeadlessPrintManager
};

using GetPDFCallback =
base::OnceCallback<void(PrintResult, const std::string&)>;
base::OnceCallback<void(PrintResult,
scoped_refptr<base::RefCountedMemory>)>;

~HeadlessPrintManager() override;

Expand Down
23 changes: 7 additions & 16 deletions headless/lib/browser/protocol/headless_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "headless/lib/browser/protocol/headless_handler.h"

#include "base/base64.h"
#include "base/base_switches.h"
#include "base/command_line.h"
#include "base/lazy_instance.h"
Expand Down Expand Up @@ -34,9 +33,9 @@ base::LazyInstance<std::set<HeadlessHandler*>>::Leaky g_instances =
enum class ImageEncoding { kPng, kJpeg };
constexpr int kDefaultScreenshotQuality = 80;

std::string EncodeBitmap(const SkBitmap& bitmap,
ImageEncoding encoding,
int quality) {
protocol::Binary EncodeBitmap(const SkBitmap& bitmap,
ImageEncoding encoding,
int quality) {
gfx::Image image = gfx::Image::CreateFrom1xBitmap(bitmap);
DCHECK(!image.IsEmpty());

Expand All @@ -48,16 +47,9 @@ std::string EncodeBitmap(const SkBitmap& bitmap,
if (gfx::JPEG1xEncodedDataFromImage(image, quality, &bytes->data()))
data = bytes;
}

if (!data || !data->front())
return std::string();

std::string base_64_data;
base::Base64Encode(
base::StringPiece(reinterpret_cast<const char*>(data->front()),
data->size()),
&base_64_data);
return base_64_data;
return protocol::Binary();
return protocol::Binary::fromRefCounted(data);
}

void OnBeginFrameFinished(
Expand All @@ -67,11 +59,10 @@ void OnBeginFrameFinished(
bool has_damage,
std::unique_ptr<SkBitmap> bitmap) {
if (!bitmap || bitmap->drawsNothing()) {
callback->sendSuccess(has_damage, Maybe<String>());
callback->sendSuccess(has_damage, Maybe<protocol::Binary>());
return;
}
std::string data = EncodeBitmap(*bitmap, encoding, quality);
callback->sendSuccess(has_damage, std::move(data));
callback->sendSuccess(has_damage, EncodeBitmap(*bitmap, encoding, quality));
}

} // namespace
Expand Down
7 changes: 2 additions & 5 deletions headless/lib/browser/protocol/page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "headless/lib/browser/protocol/page_handler.h"

#include "base/base64.h"
#include "content/public/browser/web_contents.h"
#include "printing/units.h"

Expand All @@ -21,12 +20,10 @@ const double kScaleMinVal = 10;

void PDFCreated(std::unique_ptr<PageHandler::PrintToPDFCallback> callback,
HeadlessPrintManager::PrintResult print_result,
const std::string& data) {
scoped_refptr<base::RefCountedMemory> data) {
std::unique_ptr<base::DictionaryValue> response;
if (print_result == HeadlessPrintManager::PRINT_SUCCESS) {
std::string base_64_data;
base::Base64Encode(data, &base_64_data);
callback->sendSuccess(base_64_data);
callback->sendSuccess(protocol::Binary::fromRefCounted(data));
} else {
callback->sendFailure(Response::Error(
HeadlessPrintManager::PrintResultToString(print_result)));
Expand Down
37 changes: 14 additions & 23 deletions headless/lib/headless_web_contents_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <string>
#include <vector>

#include "base/base64.h"
#include "base/command_line.h"
#include "base/json/json_writer.h"
#include "base/logging.h"
Expand Down Expand Up @@ -166,13 +165,8 @@ IN_PROC_BROWSER_TEST_F(HeadlessWebContentsTest, HandleSSLError) {
}

namespace {
bool DecodePNG(std::string base64_data, SkBitmap* bitmap) {
std::string png_data;
if (!base::Base64Decode(base64_data, &png_data))
return false;
return gfx::PNGCodec::Decode(
reinterpret_cast<unsigned const char*>(png_data.data()), png_data.size(),
bitmap);
bool DecodePNG(const protocol::Binary& png_data, SkBitmap* bitmap) {
return gfx::PNGCodec::Decode(png_data.data(), png_data.size(), bitmap);
}
} // namespace

Expand Down Expand Up @@ -211,10 +205,10 @@ class HeadlessWebContentsScreenshotTest

void OnScreenshotCaptured(
std::unique_ptr<page::CaptureScreenshotResult> result) {
std::string base64 = result->GetData();
EXPECT_GT(base64.length(), 0U);
protocol::Binary png_data = result->GetData();
EXPECT_GT(png_data.size(), 0U);
SkBitmap result_bitmap;
EXPECT_TRUE(DecodePNG(base64, &result_bitmap));
EXPECT_TRUE(DecodePNG(png_data, &result_bitmap));

EXPECT_EQ(800, result_bitmap.width());
EXPECT_EQ(600, result_bitmap.height());
Expand Down Expand Up @@ -308,12 +302,9 @@ class HeadlessWebContentsPDFTest : public HeadlessAsyncDevTooledBrowserTest {
}

void OnPDFCreated(std::unique_ptr<page::PrintToPDFResult> result) {
std::string base64 = result->GetData();
EXPECT_GT(base64.length(), 0U);
std::string pdf_data;
EXPECT_TRUE(base::Base64Decode(base64, &pdf_data));

auto pdf_span = base::as_bytes(base::make_span(pdf_data));
protocol::Binary pdf_data = result->GetData();
EXPECT_GT(pdf_data.size(), 0U);
auto pdf_span = base::make_span(pdf_data.data(), pdf_data.size());
int num_pages;
EXPECT_TRUE(chrome_pdf::GetPDFDocInfo(pdf_span, &num_pages, nullptr));
EXPECT_EQ(std::ceil(kDocHeight / kPaperHeight), num_pages);
Expand Down Expand Up @@ -650,10 +641,10 @@ class HeadlessWebContentsBeginFrameControlBasicTest
// First BeginFrame should have caused damage and have a screenshot.
EXPECT_TRUE(result->GetHasDamage());
ASSERT_TRUE(result->HasScreenshotData());
std::string base64 = result->GetScreenshotData();
EXPECT_LT(0u, base64.length());
protocol::Binary png_data = result->GetScreenshotData();
EXPECT_LT(0u, png_data.size());
SkBitmap result_bitmap;
EXPECT_TRUE(DecodePNG(base64, &result_bitmap));
EXPECT_TRUE(DecodePNG(png_data, &result_bitmap));
EXPECT_EQ(200, result_bitmap.width());
EXPECT_EQ(200, result_bitmap.height());
SkColor expected_color = SkColorSetRGB(0x00, 0x00, 0xff);
Expand Down Expand Up @@ -737,10 +728,10 @@ class HeadlessWebContentsBeginFrameControlViewportTest
EXPECT_TRUE(result->GetHasDamage());
EXPECT_TRUE(result->HasScreenshotData());
if (result->HasScreenshotData()) {
std::string base64 = result->GetScreenshotData();
EXPECT_LT(0u, base64.length());
protocol::Binary png_data = result->GetScreenshotData();
EXPECT_LT(0u, png_data.size());
SkBitmap result_bitmap;
EXPECT_TRUE(DecodePNG(base64, &result_bitmap));
EXPECT_TRUE(DecodePNG(png_data, &result_bitmap));

EXPECT_EQ(300, result_bitmap.width());
EXPECT_EQ(300, result_bitmap.height());
Expand Down
Loading

0 comments on commit c58a4ae

Please sign in to comment.