RawDraw: fix glyph missing during renderer destruction

Right now, when renderer is destroy, we just consider all cached
font can be released immediately. However there could be several
frames which are still using those cached font. So we added a
local ref_count_ for the font's discardable handle. When it
reaches 0, it means there isn't any reference in viz. We can
safely release it when the renderer is destroyed already.

(cherry picked from commit 410ae6b)

Bug: 1316058,1316352
Change-Id: I8febc74f02ab66ba9ad8aa519fe44230223e5a2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3580848
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#992833}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3596830
Auto-Submit: Peng Huang <penghuang@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#49}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}

gpu/command_buffer/service/raster_decoder.cc

@@ -3779,8 +3779,7 @@ void RasterDecoderImpl::DoEndRasterCHROMIUM() {
     scoped_shared_image_raster_write_->set_callback(base::BindOnce(
         [](scoped_refptr<ServiceFontManager> font_manager,
            std::vector<SkDiscardableHandleId> handles) {
-          if (!font_manager->is_destroyed())
-            font_manager->Unlock(handles);
+          font_manager->Unlock(handles);
         },
         font_manager_, std::move(locked_handles_)));
     scoped_shared_image_raster_write_.reset();

gpu/command_buffer/service/service_font_manager.cc

@@ -153,14 +153,15 @@ ServiceFontManager::ServiceFontManager(Client* client,
 
 ServiceFontManager::~ServiceFontManager() {
   DCHECK(destroyed_);
+  DLOG_IF(ERROR, !discardable_handle_map_.empty())
+      << "discardable_handle_map_ is not empty.";
 }
 
 void ServiceFontManager::Destroy() {
   base::AutoLock hold(lock_);
 
   client_ = nullptr;
   strike_client_.reset();
-  discardable_handle_map_.clear();
   destroyed_ = true;
 }
 
@@ -187,8 +188,9 @@ bool ServiceFontManager::Deserialize(
 
     scoped_refptr<gpu::Buffer> buffer = client_->GetShmBuffer(handle.shm_id);
     if (!DiscardableHandleBase::ValidateParameters(buffer.get(),
-                                                   handle.byte_offset))
+                                                   handle.byte_offset)) {
       return false;
+    }
 
     if (!AddHandle(handle.handle_id,
                    ServiceDiscardableHandle(
@@ -207,9 +209,15 @@ bool ServiceFontManager::Deserialize(
     return false;
 
   locked_handles->resize(num_locked_handles);
-  for (uint32_t i = 0; i < num_locked_handles; ++i) {
-    if (!deserializer.Read<SkDiscardableHandleId>(&locked_handles->at(i)))
+  for (auto& locked_handle : *locked_handles) {
+    if (!deserializer.Read<SkDiscardableHandleId>(&locked_handle))
       return false;
+    auto it = discardable_handle_map_.find(locked_handle);
+    if (it == discardable_handle_map_.end()) {
+      DLOG(ERROR) << "Got an invalid SkDiscardableHandleId:" << locked_handle;
+      continue;
+    }
+    it->second.Lock();
   }
 
   // Skia font data.
@@ -236,8 +244,6 @@ bool ServiceFontManager::AddHandle(SkDiscardableHandleId handle_id,
 bool ServiceFontManager::Unlock(
     const std::vector<SkDiscardableHandleId>& handles) {
   base::AutoLock hold(lock_);
-  DCHECK(!destroyed_);
-
   for (auto handle_id : handles) {
     auto it = discardable_handle_map_.find(handle_id);
     if (it == discardable_handle_map_.end())
@@ -249,10 +255,6 @@ bool ServiceFontManager::Unlock(
 
 bool ServiceFontManager::DeleteHandle(SkDiscardableHandleId handle_id) {
   base::AutoLock hold(lock_);
-
-  if (destroyed_)
-    return true;
-
   // If this method returns true, the strike associated with the handle will be
   // deleted which deletes the memory for all glyphs cached by the strike. On
   // mac this is resulting in hangs during strike deserialization when a bunch
@@ -262,24 +264,32 @@ bool ServiceFontManager::DeleteHandle(SkDiscardableHandleId handle_id) {
   // where skia is used, except for single process webview where the renderer
   // and GPU run in the same process.
   const bool report_progress =
-      base::PlatformThread::CurrentId() == client_thread_id_;
+      base::PlatformThread::CurrentId() == client_thread_id_ && !destroyed_;
 
   auto it = discardable_handle_map_.find(handle_id);
   if (it == discardable_handle_map_.end()) {
     LOG(ERROR) << "Tried to delete invalid SkDiscardableHandleId: "
                << handle_id;
-    if (report_progress)
+    if (report_progress) {
+      DCHECK(client_);
       client_->ReportProgress();
+    }
     return true;
   }
 
-  bool deleted = it->second.Delete();
+  // If the renderer is destroyed, we just need check if the local ref count is
+  // 0.
+  bool deleted = destroyed_ ? it->second.ref_count() == 0 : it->second.Delete();
   if (!deleted)
     return false;
 
+  DCHECK_EQ(it->second.ref_count(), 0);
   discardable_handle_map_.erase(it);
-  if (report_progress)
+  if (report_progress) {
+    DCHECK(client_);
     client_->ReportProgress();
+  }
+
   return true;
 }
 

gpu/command_buffer/service/service_font_manager.h

@@ -55,8 +55,25 @@ class GPU_GLES2_EXPORT ServiceFontManager
   raw_ptr<Client> client_;
   const base::PlatformThreadId client_thread_id_;
   std::unique_ptr<SkStrikeClient> strike_client_;
-  base::flat_map<SkDiscardableHandleId, ServiceDiscardableHandle>
-      discardable_handle_map_;
+
+  class Handle {
+   public:
+    explicit Handle(ServiceDiscardableHandle handle)
+        : handle_(std::move(handle)) {}
+    void Unlock() {
+      --ref_count_;
+      handle_.Unlock();
+    }
+    void Lock() { ++ref_count_; }
+    bool Delete() { return handle_.Delete(); }
+    int ref_count() const { return ref_count_; }
+
+   private:
+    ServiceDiscardableHandle handle_;
+    // ref count hold by GPU service.
+    int ref_count_ = 0;
+  };
+  base::flat_map<SkDiscardableHandleId, Handle> discardable_handle_map_;
   bool destroyed_ = false;
   const bool disable_oopr_debug_crash_dump_;
 };