arc: Fix Default icon issue when cached icon file is corrupted.

chrome/browser/ui/app_list/arc/arc_app_icon.cc

 // Copyright 2015 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 "chrome/browser/ui/app_list/arc/arc_app_icon.h"
 
 #include <algorithm>
 #include <map>
 #include <memory>
 #include <utility>
@@ skipping 154 matching lines @@
 class ArcAppIcon::DecodeRequest : public ImageDecoder::ImageRequest {
  public:
   DecodeRequest(const base::WeakPtr<ArcAppIcon>& host,
                 int dimension,
                 ui::ScaleFactor scale_factor);
   ~DecodeRequest() override;
 
   // ImageDecoder::ImageRequest
   void OnImageDecoded(const SkBitmap& bitmap) override;
   void OnDecodeImageFailed() override;
+
  private:
   base::WeakPtr<ArcAppIcon> host_;
   int dimension_;
   ui::ScaleFactor scale_factor_;
 
   DISALLOW_COPY_AND_ASSIGN(DecodeRequest);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // ArcAppIcon::DecodeRequest
@@ skipping 14 matching lines @@
 
   if (!host_)
     return;
 
   int expected_dim = static_cast<int>(
       ui::GetScaleForScaleFactor(scale_factor_) * dimension_ + 0.5f);
   if (bitmap.width() != expected_dim || bitmap.height() != expected_dim) {
     VLOG(2) << "Decoded ARC icon has unexpected dimension "
             << bitmap.width() << "x" << bitmap.height() << ". Expected "
             << expected_dim << "x" << ".";
+
+    host_->MaybeRequestIcon(scale_factor_);
     host_->DiscardDecodeRequest(this);
     return;
   }
 
   gfx::ImageSkia image_skia;
   image_skia.AddRepresentation(gfx::ImageSkiaRep(
       bitmap,
       ui::GetScaleForScaleFactor(scale_factor_)));
-
   host_->Update(&image_skia);
   host_->DiscardDecodeRequest(this);
 }
 
 void ArcAppIcon::DecodeRequest::OnDecodeImageFailed() {
   VLOG(2) << "Failed to decode ARC icon.";
 
   if (!host_)
     return;
 
+  host_->MaybeRequestIcon(scale_factor_);
   host_->DiscardDecodeRequest(this);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ArcAppIcon
 
 // static
 void ArcAppIcon::DisableSafeDecodingForTesting() {
   disable_safe_decoding = true;
 }
@@ skipping 25 matching lines @@
     return;
 
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE, base::TaskTraits().MayBlock().WithPriority(
                      base::TaskPriority::BACKGROUND),
       base::Bind(&ArcAppIcon::ReadOnFileThread, scale_factor, path,
                  prefs->MaybeGetIconPathForDefaultApp(app_id_, scale_factor)),
       base::Bind(&ArcAppIcon::OnIconRead, weak_ptr_factory_.GetWeakPtr()));
 }
 
-void ArcAppIcon::RequestIcon(ui::ScaleFactor scale_factor) {
+void ArcAppIcon::MaybeRequestIcon(ui::ScaleFactor scale_factor) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(context_);
   DCHECK(prefs);
 
+  // Request for Icon has been recorded. Don't resend same request within same
+  // user session.
+  if (prefs->IsIconRequestRecorded(app_id_, scale_factor))

[khmel, 2017/03/15 01:16:55]

Could you please move this logic into prefs. It would be nice to have
centralized logic that handles request there.

[lgcheng, 2017/03/15 19:59:31]

Done.

+    return;
   // ArcAppListPrefs notifies ArcAppModelBuilder via Observer when icon is ready
   // and ArcAppModelBuilder refreshes the icon of the corresponding item by
   // calling LoadScaleFactor.
   prefs->RequestIcon(app_id_, scale_factor);
 }
 
 // static
 std::unique_ptr<ArcAppIcon::ReadResult> ArcAppIcon::ReadOnFileThread(
     ui::ScaleFactor scale_factor,
     const base::FilePath& path,
@@ skipping 22 matching lines @@
   return base::WrapUnique(new ArcAppIcon::ReadResult(false,
                                                      path_to_read != path,
                                                      scale_factor,
                                                      unsafe_icon_data));
 }
 
 void ArcAppIcon::OnIconRead(
     std::unique_ptr<ArcAppIcon::ReadResult> read_result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  if (read_result->request_to_install)
-    RequestIcon(read_result->scale_factor);
+  // When unsafe_icon_data is empty, there are four senarios.
+  // 1. Default app icon path is non valid. In this case, request_to_install is
+  // true.
+  // 2. Fail to read default icon file. In this case request_to_install is true.
+  // 3. Default icon file is empty. In this case request_to_install is true.
+  // 4. Icon file for app_id_ is empty. This indicates a file corruption of some
+  // sort of issue. We should try to request app icon.
+  if (read_result->request_to_install || read_result->unsafe_icon_data.empty())

[khmel, 2017/03/15 01:16:55]

Could you please move read_result->unsafe_icon_data.empty() check to place where
it is actually used?

[lgcheng, 2017/03/15 19:59:31]

Done.

+    MaybeRequestIcon(read_result->scale_factor);
 
   if (!read_result->unsafe_icon_data.empty()) {
     decode_requests_.push_back(base::MakeUnique<DecodeRequest>(
         weak_ptr_factory_.GetWeakPtr(), resource_size_in_dip_,
         read_result->scale_factor));
     if (disable_safe_decoding) {
       SkBitmap bitmap;
       if (!read_result->unsafe_icon_data.empty() &&
           gfx::PNGCodec::Decode(
               reinterpret_cast<const unsigned char*>(
@@ skipping 31 matching lines @@
 void ArcAppIcon::DiscardDecodeRequest(DecodeRequest* request) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   auto it = std::find_if(decode_requests_.begin(), decode_requests_.end(),
                          [request](const std::unique_ptr<DecodeRequest>& ptr) {
                            return ptr.get() == request;
                          });
   CHECK(it != decode_requests_.end());
   decode_requests_.erase(it);
 }