Commit 932101ae authored by Justin DeWitt's avatar Justin DeWitt Committed by Commit Bot

[EoS] Fix image decoding

This patch fixes a few issues found while trying to hook up image
decoding end-to-end:
* The images are improperly inserted into the database, since binding as
  string uses the text type, which tries to use UTF8 encoding (which
  doesn't work for binary blobs.)
* Decode failures used to cause a DCHECK, now just bail out with an
  error value
* A callback was called one too many times, even after std::move.
* A crash would happen if a category had no images.

Bug: 867488
Change-Id: Ie28c01e28bd746b9a90563c228775ff3b5a8f83f
Reviewed-on: https://chromium-review.googlesource.com/1248041Reviewed-by: default avatarCathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595514}
parent 201f7aa0
...@@ -55,7 +55,7 @@ void CatalogReady(ScopedJavaGlobalRef<jobject>(j_result_obj), ...@@ -55,7 +55,7 @@ void CatalogReady(ScopedJavaGlobalRef<jobject>(j_result_obj),
void ImageReady(ScopedJavaGlobalRef<jobject>(j_callback_obj), void ImageReady(ScopedJavaGlobalRef<jobject>(j_callback_obj),
std::unique_ptr<SkBitmap> bitmap) { std::unique_ptr<SkBitmap> bitmap) {
if (!bitmap) { if (!bitmap || bitmap->isNull()) {
DVLOG(1) << "Site icon is empty."; DVLOG(1) << "Site icon is empty.";
base::android::RunObjectCallbackAndroid(j_callback_obj, nullptr); base::android::RunObjectCallbackAndroid(j_callback_obj, nullptr);
return; return;
......
...@@ -58,8 +58,6 @@ void ExploreSitesServiceImpl::GetCategoryImage(int category_id, ...@@ -58,8 +58,6 @@ void ExploreSitesServiceImpl::GetCategoryImage(int category_id,
explore_sites_store_.get(), category_id, kFaviconsPerCategoryImage, explore_sites_store_.get(), category_id, kFaviconsPerCategoryImage,
base::BindOnce(&ExploreSitesServiceImpl::DecodeImageBytes, base::BindOnce(&ExploreSitesServiceImpl::DecodeImageBytes,
std::move(callback)))); std::move(callback))));
// TODO(dewittj, freedjm): implement.
std::move(callback).Run(nullptr);
} }
void ExploreSitesServiceImpl::GetSiteImage(int site_id, void ExploreSitesServiceImpl::GetSiteImage(int site_id,
...@@ -136,6 +134,8 @@ void ExploreSitesServiceImpl::AddUpdatedCatalog( ...@@ -136,6 +134,8 @@ void ExploreSitesServiceImpl::AddUpdatedCatalog(
// static // static
void ExploreSitesServiceImpl::OnDecodeDone(BitmapCallback callback, void ExploreSitesServiceImpl::OnDecodeDone(BitmapCallback callback,
const SkBitmap& decoded_image) { const SkBitmap& decoded_image) {
DVLOG(1) << "Decoded images, result "
<< (decoded_image.isNull() ? "null" : "non-null");
std::unique_ptr<SkBitmap> bitmap = std::make_unique<SkBitmap>(decoded_image); std::unique_ptr<SkBitmap> bitmap = std::make_unique<SkBitmap>(decoded_image);
std::move(callback).Run(std::move(bitmap)); std::move(callback).Run(std::move(bitmap));
} }
...@@ -145,8 +145,11 @@ void ExploreSitesServiceImpl::DecodeImageBytes(BitmapCallback callback, ...@@ -145,8 +145,11 @@ void ExploreSitesServiceImpl::DecodeImageBytes(BitmapCallback callback,
EncodedImageList images) { EncodedImageList images) {
// TODO(freedjm) Fix to handle multiple images when support is added for // TODO(freedjm) Fix to handle multiple images when support is added for
// creating composite images for the NTP tiles. // creating composite images for the NTP tiles.
DCHECK(images.size() > 0); DVLOG(1) << "Requested decoding for " << images.size() << " images";
std::unique_ptr<EncodedImageBytes> image_data = std::move(images[0]); if (images.size() == 0) {
std::move(callback).Run(nullptr);
return;
}
service_manager::mojom::ConnectorRequest connector_request; service_manager::mojom::ConnectorRequest connector_request;
std::unique_ptr<service_manager::Connector> connector = std::unique_ptr<service_manager::Connector> connector =
...@@ -155,7 +158,7 @@ void ExploreSitesServiceImpl::DecodeImageBytes(BitmapCallback callback, ...@@ -155,7 +158,7 @@ void ExploreSitesServiceImpl::DecodeImageBytes(BitmapCallback callback,
->GetConnector() ->GetConnector()
->BindConnectorRequest(std::move(connector_request)); ->BindConnectorRequest(std::move(connector_request));
data_decoder::DecodeImage(connector.get(), *image_data, data_decoder::DecodeImage(connector.get(), *images[0],
data_decoder::mojom::ImageCodec::DEFAULT, false, data_decoder::mojom::ImageCodec::DEFAULT, false,
data_decoder::kDefaultMaxSizeInBytes, gfx::Size(), data_decoder::kDefaultMaxSizeInBytes, gfx::Size(),
base::BindOnce(&OnDecodeDone, std::move(callback))); base::BindOnce(&OnDecodeDone, std::move(callback)));
......
...@@ -82,7 +82,8 @@ bool ImportCatalogSync(std::string version_token, ...@@ -82,7 +82,8 @@ bool ImportCatalogSync(std::string version_token,
category_statement.BindString(col++, version_token); category_statement.BindString(col++, version_token);
category_statement.BindInt(col++, static_cast<int>(category.type())); category_statement.BindInt(col++, static_cast<int>(category.type()));
category_statement.BindString(col++, category.localized_title()); category_statement.BindString(col++, category.localized_title());
category_statement.BindString(col++, category.icon()); category_statement.BindBlob(col++, category.icon().data(),
category.icon().length());
category_statement.Run(); category_statement.Run();
...@@ -96,7 +97,7 @@ bool ImportCatalogSync(std::string version_token, ...@@ -96,7 +97,7 @@ bool ImportCatalogSync(std::string version_token,
site_statement.BindString(col++, site.site_url()); site_statement.BindString(col++, site.site_url());
site_statement.BindInt64(col++, category_id); site_statement.BindInt64(col++, category_id);
site_statement.BindString(col++, site.title()); site_statement.BindString(col++, site.title());
site_statement.BindString(col++, site.icon()); site_statement.BindBlob(col++, site.icon().data(), site.icon().length());
site_statement.Run(); site_statement.Run();
} }
......
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