diagnostics: ScopedAllowBlocking cros mojo utils

Allow mojo_utils in wilco_dtc_supportd to use ScopedAllowBlocking's
private ctor.

GetStringPieceFromMojoHandle() runs in the browser process and has
DCHECK errors. This is because GetStringPieceFromMojoHandle() uses
base::file::Fstat() that is marked with ScopedBlockingCall.

Background: Referring to the discussion in
https://chromium-review.googlesource.com/c/chromium/src/+/2042611, the
ideal solution to this problem is to upgrade mojo library in chromeos
(mojo library in chromeos is 1-2 years older than chrome). Doing that,
the new shmem builtin types can be used and we won't need to get the
handle's length in the first place.

The problem with that is upgrade will take time. So, as a temporary
solution, allow mojo_utils in wilco_dtc_supportd to use
ScopedAllowBlocking's private ctor by making the class friend with
ScopedAllowBlocking.

The justification as per alexilin@ is that: Fstat() shouldn't be
blocking for shared memory descriptors (i.e. mojo handle). Fstat() will
read tmpfs metadata that is always stored in RAM. That metadata is
never swapped to disk since it's part of kernel-allocated memory that
is never swapped out.

Bug: 1055467,b:146119375
TEST=building and running unittests

Change-Id: Iba688b7f311c07b051687a1f619b594475892178
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070160
Commit-Queue: Mahmoud Gawad <mgawad@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Oleh Lamzin <lamzin@google.com>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746317}

base/threading/thread_restrictions.h

@@ -126,6 +126,7 @@ class TileTaskManagerImpl;
 }
 namespace chromeos {
 class BlockingMethodCaller;
+class MojoUtils;
 namespace system {
 class StatisticsProviderImpl;
 }
@@ -359,6 +360,7 @@ class BASE_EXPORT ScopedAllowBlocking {
   // in unit tests to avoid the friend requirement.
   friend class AdjustOOMScoreHelper;
   friend class android_webview::ScopedAllowInitGLBindings;
+  friend class chromeos::MojoUtils;  // http://crbug.com/1055467
   friend class content::BrowserProcessSubThread;
   friend class content::RenderWidgetHostViewMac;  // http://crbug.com/121917
   friend class content::WebContentsViewMac;

chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.cc

@@ -17,7 +17,8 @@
 
 namespace chromeos {
 
-base::StringPiece GetStringPieceFromMojoHandle(
+// static
+base::StringPiece MojoUtils::GetStringPieceFromMojoHandle(
     mojo::ScopedHandle handle,
     base::ReadOnlySharedMemoryMapping* shared_memory) {
   DCHECK(shared_memory);
@@ -31,7 +32,7 @@ base::StringPiece GetStringPieceFromMojoHandle(
   size_t file_size = 0;
   {
     // TODO(b/146119375): Remove blocking operation from production code.
-    base::ScopedAllowBlockingForTesting allow_blocking;
+    base::ScopedAllowBlocking allow_blocking;
     file_size = file.GetLength();
   }
   if (file_size <= 0)
@@ -53,7 +54,8 @@ base::StringPiece GetStringPieceFromMojoHandle(
                            shared_memory->size());
 }
 
-mojo::ScopedHandle CreateReadOnlySharedMemoryMojoHandle(
+// static
+mojo::ScopedHandle MojoUtils::CreateReadOnlySharedMemoryMojoHandle(
     const std::string& content) {
   if (content.empty())
     return mojo::ScopedHandle();

chrome/browser/chromeos/wilco_dtc_supportd/mojo_utils.h

@@ -14,33 +14,48 @@
 
 namespace chromeos {
 
-// Allows to get access to the buffer in read only shared memory. It converts
-// mojo::Handle to base::ReadOnlySharedMemoryMapping and returns a string
-// content.
-//
-// |handle| must be a valid mojo handle of the non-empty buffer in the shared
-// memory.
-//
-// Returns an empty string and an invalid |shared_memory| if error.
-//
-// TODO(crbug.com/989503): Use mojo::ScopedSharedBufferHandle or
-// base::ReadOnlySharedMemoryRegion instead of mojo::ScopedHandle
-// once ChromeOS updates to the required version of mojo library.
-base::StringPiece GetStringPieceFromMojoHandle(
-    mojo::ScopedHandle handle,
-    base::ReadOnlySharedMemoryMapping* shared_memory);
-
-// Allocates buffer in shared memory, copies |content| to the buffer and
-// converts shared buffer handle into |mojo::ScopedHandle|.
-//
-// Allocated shared memory is read only for another process.
-//
-// Returns invalid |mojo::ScopedHandle| if |content| is empty or error happened.
-//
-// TODO(crbug.com/989503): Remove mojo::ScopedHandle wrapping once
-// ChromeOS updates to the required version of mojo library.
-mojo::ScopedHandle CreateReadOnlySharedMemoryMojoHandle(
-    const std::string& content);
+// This class is created to enable its functions (i.e.
+// GetStringPieceFromMojoHandle) to use base::ScopedAllowBlocking's private
+// constructor. That is achieved by making this class friend of
+// base::ScopedAllowBlocking.
+// Non-goal: This class is not created to group static member functions (as
+// discouraged by chromium style guide).
+class MojoUtils final {
+ public:
+  // Disallow all implicit constructors.
+  MojoUtils() = delete;
+  MojoUtils(const MojoUtils& mojo_utils) = delete;
+  MojoUtils& operator=(const MojoUtils& mojo_utils) = delete;
+
+  // Allows to get access to the buffer in read only shared memory. It converts
+  // mojo::Handle to base::ReadOnlySharedMemoryMapping and returns a string
+  // content.
+  //
+  // |handle| must be a valid mojo handle of the non-empty buffer in the shared
+  // memory.
+  //
+  // Returns an empty string and an invalid |shared_memory| if error.
+  //
+  // TODO(crbug.com/989503): Use mojo::ScopedSharedBufferHandle or
+  // base::ReadOnlySharedMemoryRegion instead of mojo::ScopedHandle
+  // once ChromeOS updates to the required version of mojo library.
+  static base::StringPiece GetStringPieceFromMojoHandle(
+      mojo::ScopedHandle handle,
+      base::ReadOnlySharedMemoryMapping* shared_memory);
+
+  // Allocates buffer in shared memory, copies |content| to the buffer and
+  // converts shared buffer handle into |mojo::ScopedHandle|.
+  //
+  // Allocated shared memory is read only for another process.
+  //
+  // Returns invalid |mojo::ScopedHandle| if |content| is empty or error
+  // happened.
+  //
+  // TODO(crbug.com/989503): Remove mojo::ScopedHandle wrapping once
+  // ChromeOS updates to the required version of mojo library.
+  static mojo::ScopedHandle CreateReadOnlySharedMemoryMojoHandle(
+      const std::string& content);
+};
 
 }  // namespace chromeos
 

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_bridge.cc

@@ -260,7 +260,8 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
   GURL gurl;
   if (url.is_valid()) {
     base::ReadOnlySharedMemoryMapping shared_memory;
-    gurl = GURL(GetStringPieceFromMojoHandle(std::move(url), &shared_memory));
+    gurl = GURL(MojoUtils::GetStringPieceFromMojoHandle(std::move(url),
+                                                        &shared_memory));
     if (!shared_memory.IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
       std::move(callback).Run(
@@ -280,7 +281,7 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
       continue;
     }
     shared_memories.emplace_back();
-    header_contents.push_back(GetStringPieceFromMojoHandle(
+    header_contents.push_back(MojoUtils::GetStringPieceFromMojoHandle(
         std::move(header), &shared_memories.back()));
     if (!shared_memories.back().IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
@@ -296,8 +297,8 @@ void WilcoDtcSupportdBridge::PerformWebRequest(
   std::string request_body_content;
   if (request_body.is_valid()) {
     base::ReadOnlySharedMemoryMapping shared_memory;
-    request_body_content = std::string(
-        GetStringPieceFromMojoHandle(std::move(request_body), &shared_memory));
+    request_body_content = std::string(MojoUtils::GetStringPieceFromMojoHandle(
+        std::move(request_body), &shared_memory));
     if (!shared_memory.IsValid()) {
       LOG(ERROR) << "Failed to read data from mojo handle";
       std::move(callback).Run(
@@ -319,8 +320,9 @@ void WilcoDtcSupportdBridge::SendWilcoDtcMessageToUi(
   // Extract the string value of the received message.
   DCHECK(json_message);
   base::ReadOnlySharedMemoryMapping json_message_shared_memory;
-  base::StringPiece json_message_string = GetStringPieceFromMojoHandle(
-      std::move(json_message), &json_message_shared_memory);
+  base::StringPiece json_message_string =
+      MojoUtils::GetStringPieceFromMojoHandle(std::move(json_message),
+                                              &json_message_shared_memory);
   if (json_message_string.empty()) {
     LOG(ERROR) << "Failed to read data from mojo handle";
     std::move(callback).Run(mojo::ScopedHandle() /* response_json_message */);
@@ -335,7 +337,7 @@ void WilcoDtcSupportdBridge::SendWilcoDtcMessageToUi(
             mojo::ScopedHandle response_mojo_handle;
             if (!response.empty()) {
               response_mojo_handle =
-                  CreateReadOnlySharedMemoryMojoHandle(response);
+                  MojoUtils::CreateReadOnlySharedMemoryMojoHandle(response);
               if (!response_mojo_handle)
                 LOG(ERROR) << "Failed to create mojo handle for string";
             }

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_messaging.cc

@@ -126,7 +126,7 @@ class WilcoDtcSupportdExtensionOwnedMessageHost final
     }
 
     mojo::ScopedHandle json_message_mojo_handle =
-        CreateReadOnlySharedMemoryMojoHandle(request_string);
+        MojoUtils::CreateReadOnlySharedMemoryMojoHandle(request_string);
     if (!json_message_mojo_handle) {
       LOG(ERROR) << "Cannot create Mojo shared memory handle from string";
       DisposeSelf(kHostInputOutputError);
@@ -168,8 +168,9 @@ class WilcoDtcSupportdExtensionOwnedMessageHost final
     }
 
     base::ReadOnlySharedMemoryMapping response_json_shared_memory;
-    base::StringPiece response_json_string = GetStringPieceFromMojoHandle(
-        std::move(response_json_message), &response_json_shared_memory);
+    base::StringPiece response_json_string =
+        MojoUtils::GetStringPieceFromMojoHandle(
+            std::move(response_json_message), &response_json_shared_memory);
     if (response_json_string.empty()) {
       LOG(ERROR) << "Cannot read response from Mojo shared memory";
       DisposeSelf(kHostInputOutputError);

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_messaging_unittest.cc

@@ -54,7 +54,7 @@ std::string AssertGetStringFromMojoHandle(mojo::ScopedHandle handle) {
     return std::string();
   base::ReadOnlySharedMemoryMapping shared_memory;
   std::string contents =
-      GetStringPieceFromMojoHandle(std::move(handle), &shared_memory)
+      MojoUtils::GetStringPieceFromMojoHandle(std::move(handle), &shared_memory)
           .as_string();
   CHECK(!contents.empty());
   return contents;
@@ -65,7 +65,7 @@ mojo::ScopedHandle AssertCreateReadOnlySharedMemoryMojoHandle(
   if (content.empty())
     return mojo::ScopedHandle();
   mojo::ScopedHandle shared_memory_handle =
-      CreateReadOnlySharedMemoryMojoHandle(content);
+      MojoUtils::CreateReadOnlySharedMemoryMojoHandle(content);
   CHECK(shared_memory_handle);
   return shared_memory_handle;
 }

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_web_request_service.cc

@@ -268,7 +268,8 @@ void WilcoDtcSupportdWebRequestService::OnRequestComplete(
 
   mojo::ScopedHandle response_body_handle;
   if (response_body)
-    response_body_handle = CreateReadOnlySharedMemoryMojoHandle(*response_body);
+    response_body_handle =
+        MojoUtils::CreateReadOnlySharedMemoryMojoHandle(*response_body);
 
   // Got an HTTP error.
   if (!IsHttpOkCode(response_code)) {

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_web_request_service_browsertest.cc

@@ -86,8 +86,8 @@ class ServiceRequestPerformer {
   std::string response_body_release() {
     DCHECK(request_performed_);
     base::ReadOnlySharedMemoryMapping response_shared_memory;
-    return GetStringPieceFromMojoHandle(std::move(response_handle_),
-                                        &response_shared_memory)
+    return MojoUtils::GetStringPieceFromMojoHandle(std::move(response_handle_),
+                                                   &response_shared_memory)
         .as_string();
   }
 

chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_web_request_service_unittest.cc

@@ -53,7 +53,7 @@ class WilcoDtcSupportdWebRequestServiceTest : public testing::Test {
         return;
       }
       base::ReadOnlySharedMemoryMapping shared_memory;
-      response_body = std::string(GetStringPieceFromMojoHandle(
+      response_body = std::string(MojoUtils::GetStringPieceFromMojoHandle(
           std::move(response_body_handle), &shared_memory));
       if (!shared_memory.IsValid()) {
         response_body = "";