Remove all FATAL_IFs from graphicsstats service

As graphicsstats can be subjected to data coming
from the disk and is in system_server we want
to bias towards best-effort instead of strict
no-errors that the rest of HWUI typically uses.

So treat any dump/merge of graphics stats as
best effort, ignoring any errors that occur.

Bug: 65652900
Test: verified 'dumpsys graphicsstats' still works

Change-Id: Ia9b91b745c2a9aedad2f22e3087e1d4bf37a1135

1 files changed, 33 insertions, 16 deletions

diff --git a/libs/hwui/service/GraphicsStatsService.cpp b/libs/hwui/service/GraphicsStatsService.cpp
index 3a77195824ad..afb1193657e1 100644
--- a/libs/hwui/service/GraphicsStatsService.cpp
+++ b/libs/hwui/service/GraphicsStatsService.cpp
@@ -40,7 +40,7 @@ static_assert(sizeof(sCurrentFileVersion) == sHeaderSize, "Header size is wrong"
 
 constexpr int sHistogramSize = ProfileData::HistogramSize();
 
-static void mergeProfileDataIntoProto(service::GraphicsStatsProto* proto,
+static bool mergeProfileDataIntoProto(service::GraphicsStatsProto* proto,
         const std::string& package, int versionCode, int64_t startTime, int64_t endTime,
         const ProfileData* data);
 static void dumpAsTextToFd(service::GraphicsStatsProto* proto, int outFd);
@@ -159,7 +159,7 @@ bool GraphicsStatsService::parseFromFile(const std::string& path, service::Graph
     return success;
 }
 
-void mergeProfileDataIntoProto(service::GraphicsStatsProto* proto, const std::string& package,
+bool mergeProfileDataIntoProto(service::GraphicsStatsProto* proto, const std::string& package,
         int versionCode, int64_t startTime, int64_t endTime, const ProfileData* data) {
     if (proto->stats_start() == 0 || proto->stats_start() > startTime) {
         proto->set_stats_start(startTime);
@@ -188,23 +188,31 @@ void mergeProfileDataIntoProto(service::GraphicsStatsProto* proto, const std::st
         proto->mutable_histogram()->Reserve(sHistogramSize);
         creatingHistogram = true;
     } else if (proto->histogram_size() != sHistogramSize) {
-        LOG_ALWAYS_FATAL("Histogram size mismatch, proto is %d expected %d",
+        ALOGE("Histogram size mismatch, proto is %d expected %d",
                 proto->histogram_size(), sHistogramSize);
+        return false;
     }
     int index = 0;
+    bool hitMergeError = false;
     data->histogramForEach([&](ProfileData::HistogramEntry entry) {
+        if (hitMergeError) return;
+
         service::GraphicsStatsHistogramBucketProto* bucket;
         if (creatingHistogram) {
             bucket = proto->add_histogram();
             bucket->set_render_millis(entry.renderTimeMs);
         } else {
             bucket = proto->mutable_histogram(index);
-            LOG_ALWAYS_FATAL_IF(bucket->render_millis() != static_cast<int32_t>(entry.renderTimeMs),
-                    "Frame time mistmatch %d vs. %u", bucket->render_millis(), entry.renderTimeMs);
+            if (bucket->render_millis() != static_cast<int32_t>(entry.renderTimeMs)) {
+                ALOGW("Frame time mistmatch %d vs. %u", bucket->render_millis(), entry.renderTimeMs);
+                hitMergeError = true;
+                return;
+            }
         }
         bucket->set_frame_count(bucket->frame_count() + entry.frameCount);
         index++;
     });
+    return !hitMergeError;
 }
 
 static int32_t findPercentile(service::GraphicsStatsProto* proto, int percentile) {
@@ -221,9 +229,11 @@ static int32_t findPercentile(service::GraphicsStatsProto* proto, int percentile
 
 void dumpAsTextToFd(service::GraphicsStatsProto* proto, int fd) {
     // This isn't a full validation, just enough that we can deref at will
-    LOG_ALWAYS_FATAL_IF(proto->package_name().empty()
-            || !proto->has_summary(), "package_name() '%s' summary %d",
-            proto->package_name().c_str(), proto->has_summary());
+    if (proto->package_name().empty() || !proto->has_summary()) {
+        ALOGW("Skipping dump, invalid package_name() '%s' or summary %d",
+                proto->package_name().c_str(), proto->has_summary());
+        return;
+    }
     dprintf(fd, "\nPackage: %s", proto->package_name().c_str());
     dprintf(fd, "\nVersion: %d", proto->version_code());
     dprintf(fd, "\nStats since: %lldns", proto->stats_start());
@@ -254,14 +264,20 @@ void GraphicsStatsService::saveBuffer(const std::string& path, const std::string
     if (!parseFromFile(path, &statsProto)) {
         statsProto.Clear();
     }
-    mergeProfileDataIntoProto(&statsProto, package, versionCode, startTime, endTime, data);
+    if (!mergeProfileDataIntoProto(&statsProto, package, versionCode, startTime, endTime, data)) {
+        return;
+    }
     // Although we might not have read any data from the file, merging the existing data
     // should always fully-initialize the proto
-    LOG_ALWAYS_FATAL_IF(!statsProto.IsInitialized(), "%s",
-            statsProto.InitializationErrorString().c_str());
-    LOG_ALWAYS_FATAL_IF(statsProto.package_name().empty()
-            || !statsProto.has_summary(), "package_name() '%s' summary %d",
-            statsProto.package_name().c_str(), statsProto.has_summary());
+    if (!statsProto.IsInitialized()) {
+        ALOGE("proto initialization error %s", statsProto.InitializationErrorString().c_str());
+        return;
+    }
+    if (statsProto.package_name().empty() || !statsProto.has_summary()) {
+        ALOGE("missing package_name() '%s' summary %d",
+                statsProto.package_name().c_str(), statsProto.has_summary());
+        return;
+    }
     int outFd = open(path.c_str(), O_CREAT | O_RDWR | O_TRUNC, 0660);
     if (outFd <= 0) {
         int err = errno;
@@ -312,8 +328,9 @@ void GraphicsStatsService::addToDump(Dump* dump, const std::string& path, const
     if (!path.empty() && !parseFromFile(path, &statsProto)) {
         statsProto.Clear();
     }
-    if (data) {
-        mergeProfileDataIntoProto(&statsProto, package, versionCode, startTime, endTime, data);
+    if (data && !mergeProfileDataIntoProto(
+            &statsProto, package, versionCode, startTime, endTime, data)) {
+        return;
     }
     if (!statsProto.IsInitialized()) {
         ALOGW("Failed to load profile data from path '%s' and data %p",