Commit a7ee1081 authored by kylechar's avatar kylechar Committed by Commit Bot

Add logging to CopyOutputResult deserialization.

This CL adds temporary detailed logging for all the failures cases in
the CopyOutputResult deserialization code. https://crbug.com/967856 is
the result of a deserialization error and I haven't been able to
reproduce it on a dev machine. Add logging so it gets picked up in
canary or dev release to give more insight.

Bug: 967856
Change-Id: I3b390c3ba9800f5c7f19d75a3cc0b9457c39d2c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1643495
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666272}
parent a77193b5
......@@ -157,20 +157,33 @@ bool StructTraits<viz::mojom::CopyOutputResultDataView,
viz::CopyOutputResult::Format format;
gfx::Rect rect;
if (!data.ReadFormat(&format) || !data.ReadRect(&rect))
// TODO(crbug.com/967856): Removed detailed logging for deserialization
// failures after bug is fixed.
if (!data.ReadFormat(&format)) {
LOG(ERROR) << "CopyOutputResult: Failed to read |format|";
return false;
}
if (!data.ReadRect(&rect)) {
LOG(ERROR) << "CopyOutputResult: Failed to read |rect|.";
return false;
}
switch (format) {
case viz::CopyOutputResult::Format::RGBA_BITMAP: {
SkBitmap bitmap;
if (!data.ReadBitmap(&bitmap))
if (!data.ReadBitmap(&bitmap)) {
LOG(ERROR) << "CopyOutputResult: Failed to read |bitmap|";
return false;
}
bool has_bitmap = bitmap.readyToDraw();
// The rect should be empty iff there is no bitmap.
if (!(has_bitmap == !rect.IsEmpty()))
if (has_bitmap == rect.IsEmpty()) {
LOG(ERROR) << "CopyOutputResult: has_bitmap == rect.IsEmpty()";
return false;
}
*out_p = std::make_unique<viz::CopyOutputSkBitmapResult>(
rect, std::move(bitmap));
......@@ -179,20 +192,28 @@ bool StructTraits<viz::mojom::CopyOutputResultDataView,
case viz::CopyOutputResult::Format::RGBA_TEXTURE: {
base::Optional<gpu::Mailbox> mailbox;
if (!data.ReadMailbox(&mailbox) || !mailbox)
if (!data.ReadMailbox(&mailbox) || !mailbox) {
LOG(ERROR) << "CopyOutputResult: Failed to read |mailbox|";
return false;
}
base::Optional<gpu::SyncToken> sync_token;
if (!data.ReadSyncToken(&sync_token) || !sync_token)
if (!data.ReadSyncToken(&sync_token) || !sync_token) {
LOG(ERROR) << "CopyOutputResult: Failed to read |sync_token|";
return false;
}
base::Optional<gfx::ColorSpace> color_space;
if (!data.ReadColorSpace(&color_space) || !color_space)
if (!data.ReadColorSpace(&color_space) || !color_space) {
LOG(ERROR) << "CopyOutputResult: Failed to read |color_space|";
return false;
}
bool has_mailbox = !mailbox->IsZero();
// The rect should be empty iff there is no texture.
if (!(has_mailbox == !rect.IsEmpty()))
if (has_mailbox == rect.IsEmpty()) {
LOG(ERROR) << "CopyOutputResult: has_mailbox == rect.IsEmpty()";
return false;
}
if (!has_mailbox) {
// Returns an empty result.
......@@ -203,8 +224,10 @@ bool StructTraits<viz::mojom::CopyOutputResultDataView,
viz::mojom::TextureReleaserPtr releaser =
data.TakeReleaser<viz::mojom::TextureReleaserPtr>();
if (!releaser)
if (!releaser) {
LOG(ERROR) << "CopyOutputResult: Failed to take |releaser|";
return false; // Illegal to provide texture without Releaser.
}
// Returns a result with a SingleReleaseCallback that will return
// here and proxy the callback over mojo to the CopyOutputResult's
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment