Remove buffer size return value in FontLoaderMac.

It is now possible to get the same information from the returned buffer.

BUG=788243

Change-Id: I5e76eedb5b5539167f93b1d275007f33f82a1c2b
Reviewed-on: https://chromium-review.googlesource.com/977055Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Wei Li <weili@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548105}

content/child/child_process_sandbox_support_impl_mac.cc

@@ -4,7 +4,10 @@
 
 #include "content/child/child_process_sandbox_support_impl_mac.h"
 
+#include <utility>
+
 #include "base/mac/scoped_cftyperef.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string16.h"
 #include "base/strings/sys_string_conversions.h"
 #include "content/common/mac/font_loader.h"
@@ -14,30 +17,29 @@
 namespace content {
 
 bool LoadFont(CTFontRef font, CGFontRef* out, uint32_t* font_id) {
-  uint32_t font_data_size;
   base::ScopedCFTypeRef<CFStringRef> name_ref(CTFontCopyPostScriptName(font));
   base::string16 font_name = SysCFStringRefToUTF16(name_ref);
   float font_point_size = CTFontGetSize(font);
   mojo::ScopedSharedBufferHandle font_data;
-  if (!content::ChildThread::Get()->LoadFont(
-          font_name, font_point_size, &font_data_size, &font_data, font_id)) {
-    *out = nullptr;
-    *font_id = 0;
-    return false;
-  }
-
-  if (font_data_size == 0 || !font_data.is_valid() || *font_id == 0) {
+  bool success = content::ChildThread::Get()->LoadFont(
+                     font_name, font_point_size, &font_data, font_id) &&
+                 *font_id > 0 && font_data.is_valid();
+  if (!success) {
     DLOG(ERROR) << "Bad response from LoadFont() for " << font_name;
     *out = nullptr;
     *font_id = 0;
     return false;
   }
 
+  uint64_t font_data_size = font_data->GetSize();
+  DCHECK_GT(font_data_size, 0U);
+  DCHECK(base::IsValueInRangeForNumericType<uint32_t>(font_data_size));
+
   // TODO(jeremy): Need to call back into the requesting process to make sure
   // that the font isn't already activated, based on the font id.  If it's
   // already activated, don't reactivate it here - https://crbug.com/72727 .
-  return FontLoader::CGFontRefFromBuffer(std::move(font_data), font_data_size,
-                                         out);
+  return FontLoader::CGFontRefFromBuffer(
+      std::move(font_data), static_cast<uint32_t>(font_data_size), out);
 }
 
 }  // namespace content

content/child/child_thread_impl.cc

@@ -676,11 +676,10 @@ mojom::FontCacheWin* ChildThreadImpl::GetFontCacheWin() {
 #elif defined(OS_MACOSX)
 bool ChildThreadImpl::LoadFont(const base::string16& font_name,
                                float font_point_size,
-                               uint32_t* out_buffer_size,
                                mojo::ScopedSharedBufferHandle* out_font_data,
                                uint32_t* out_font_id) {
-  return GetFontLoaderMac()->LoadFont(
-      font_name, font_point_size, out_buffer_size, out_font_data, out_font_id);
+  return GetFontLoaderMac()->LoadFont(font_name, font_point_size, out_font_data,
+                                      out_font_id);
 }
 
 mojom::FontLoaderMac* ChildThreadImpl::GetFontLoaderMac() {

content/child/child_thread_impl.h

@@ -95,7 +95,6 @@ class CONTENT_EXPORT ChildThreadImpl
 #elif defined(OS_MACOSX)
   bool LoadFont(const base::string16& font_name,
                 float font_point_size,
-                uint32_t* out_buffer_size,
                 mojo::ScopedSharedBufferHandle* out_font_data,
                 uint32_t* out_font_id) override;
 #endif

content/common/font_loader_mac.mojom

@@ -10,5 +10,5 @@ interface FontLoaderMac {
   // Request the browser to load a font into shared memory for us.
   [Sync]
   LoadFont(mojo_base.mojom.String16 font_name, float font_point_size)
-      => (uint32 buffer_size, handle<shared_buffer>? font_data, uint32 font_id);
+      => (handle<shared_buffer>? font_data, uint32 font_id);
 };

content/common/mac/font_loader.h

@@ -5,11 +5,11 @@
 #ifndef CONTENT_COMMON_MAC_FONT_LOADER_H_
 #define CONTENT_COMMON_MAC_FONT_LOADER_H_
 
-#include <memory>
-
 #include <CoreGraphics/CoreGraphics.h>
 #include <stdint.h>
 
+#include <memory>
+
 #include "base/callback_forward.h"
 #include "content/common/content_export.h"
 #include "mojo/public/cpp/system/buffer.h"
@@ -27,19 +27,18 @@ class FontLoader {
   struct CONTENT_EXPORT ResultInternal {
     ResultInternal();
     ~ResultInternal();
-    uint32_t font_data_size = 0;
+
     mojo::ScopedSharedBufferHandle font_data;
     uint32_t font_id = 0;
   };
 
   // Callback for the reporting result of LoadFont().
-  // - The first argument is the data size.
   // - The ScopedSharedBufferHandle points to a shared memory buffer containing
   //   the raw data for the font file.
   // - The last argument is the font_id: a unique identifier for the on-disk
   //   file we load for the font.
-  using LoadedCallback = base::OnceCallback<
-      void(uint32_t, mojo::ScopedSharedBufferHandle, uint32_t)>;
+  using LoadedCallback =
+      base::OnceCallback<void(mojo::ScopedSharedBufferHandle, uint32_t)>;
 
   // Load a font specified by |font| into a shared memory buffer suitable for
   // sending over IPC. On failure, zeroes and an invalid handle are reported

content/common/mac/font_loader.mm

@@ -95,8 +95,6 @@ std::unique_ptr<FontLoader::ResultInternal> LoadFontOnFileThread(
     return nullptr;
   }
 
-  result->font_data_size = font_file_size_32;
-
   // Font loading used to call ATSFontGetContainer() and used that as font id.
   // ATS is deprecated. CoreText offers up the ATSFontRef typeface ID via
   // CTFontGetPlatformFont.
@@ -112,15 +110,12 @@ std::unique_ptr<FontLoader::ResultInternal> LoadFontOnFileThread(
 void ReplyOnUIThread(FontLoader::LoadedCallback callback,
                      std::unique_ptr<FontLoader::ResultInternal> result) {
   if (!result) {
-    std::move(callback).Run(0, mojo::ScopedSharedBufferHandle(), 0);
+    std::move(callback).Run(mojo::ScopedSharedBufferHandle(), 0);
     return;
   }
 
-  DCHECK_NE(0u, result->font_data_size);
   DCHECK_NE(0u, result->font_id);
-
-  std::move(callback).Run(result->font_data_size, std::move(result->font_data),
-                          result->font_id);
+  std::move(callback).Run(std::move(result->font_data), result->font_id);
 }
 
 }  // namespace

content/common/sandbox_mac_fontloading_unittest.mm

@@ -112,16 +112,19 @@ TEST_F(MacSandboxTest, FontLoadingTest) {
 
   std::unique_ptr<FontLoader::ResultInternal> result =
       FontLoader::LoadFontForTesting(base::ASCIIToUTF16("Geeza Pro"), 16);
-  EXPECT_GT(result->font_data_size, 0U);
+  ASSERT_TRUE(result);
+  ASSERT_TRUE(result->font_data.is_valid());
+  uint64_t font_data_size = result->font_data->GetSize();
+  EXPECT_GT(font_data_size, 0U);
   EXPECT_GT(result->font_id, 0U);
 
   mojo::ScopedSharedBufferMapping mapping =
-      result->font_data->Map(result->font_data_size);
+      result->font_data->Map(font_data_size);
   ASSERT_TRUE(mapping);
 
   base::WriteFileDescriptor(fileno(temp_file),
                             static_cast<const char*>(mapping.get()),
-                            result->font_data_size);
+                            font_data_size);
 
   ASSERT_TRUE(RunTestInSandbox(service_manager::SANDBOX_TYPE_RENDERER,
                                "FontLoadingTestCase",

content/public/child/child_thread.h

@@ -85,7 +85,6 @@ class CONTENT_EXPORT ChildThread : public IPC::Sender {
   // Load specified font into shared memory.
   virtual bool LoadFont(const base::string16& font_name,
                         float font_point_size,
-                        uint32_t* out_buffer_size,
                         mojo::ScopedSharedBufferHandle* out_font_data,
                         uint32_t* out_font_id) = 0;
 #endif

content/public/test/mock_render_thread.cc

@@ -262,7 +262,6 @@ void MockRenderThread::ReleaseCachedFonts() {
 #elif defined(OS_MACOSX)
 bool MockRenderThread::LoadFont(const base::string16& font_name,
                                 float font_point_size,
-                                uint32_t* out_buffer_size,
                                 mojo::ScopedSharedBufferHandle* out_font_data,
                                 uint32_t* out_font_id) {
   return false;  // Not implemented.

content/public/test/mock_render_thread.h

@@ -94,7 +94,6 @@ class MockRenderThread : public RenderThread {
 #elif defined(OS_MACOSX)
   bool LoadFont(const base::string16& font_name,
                 float font_point_size,
-                uint32_t* out_buffer_size,
                 mojo::ScopedSharedBufferHandle* out_font_data,
                 uint32_t* out_font_id) override;
 #endif