[gin] SetProtection should return result

When SetProtection is used for wasm memory, it can fail if not enough
memory is available when growing. Fix to return the result of system
calls.

As coordination in needed between V8/Gin - this change adds a
new SetProtection method with a boolean return, subsequent changes
in V8/Chromium will swap this out to be the default SetProtection
method.

Bug: 775047
Change-Id: I9f55d156c8317be8c012de60940d1698db439c86
Reviewed-on: https://chromium-review.googlesource.com/753429Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524457}

gin/array_buffer.cc

@@ -85,27 +85,29 @@ void ArrayBufferAllocator::Free(void* data,
 void ArrayBufferAllocator::SetProtection(void* data,
                                          size_t length,
                                          Protection protection) {
+  CHECK(SetProtection(protection, data, length));
+}
+
+bool ArrayBufferAllocator::SetProtection(Protection protection,
+                                         void* data,
+                                         size_t length) {
   switch (protection) {
-    case Protection::kNoAccess: {
+    case Protection::kNoAccess:
 #if defined(OS_POSIX)
-      int ret = mprotect(data, length, PROT_NONE);
-      CHECK(!ret);
+      return mprotect(data, length, PROT_NONE) == 0;
 #else
-      BOOL ret = VirtualFree(data, length, MEM_DECOMMIT);
-      CHECK(ret);
+      return VirtualFree(data, length, MEM_DECOMMIT);
 #endif
-      break;
-    }
     case Protection::kReadWrite:
 #if defined(OS_POSIX)
-      mprotect(data, length, PROT_READ | PROT_WRITE);
+      return mprotect(data, length, PROT_READ | PROT_WRITE) == 0;
 #else
-      VirtualAlloc(data, length, MEM_COMMIT, PAGE_READWRITE);
+      return VirtualAlloc(data, length, MEM_COMMIT, PAGE_READWRITE);
 #endif
-      break;
     default:
       NOTREACHED();
   }
+  return false;
 }
 
 ArrayBufferAllocator* ArrayBufferAllocator::SharedInstance() {

gin/array_buffer.h

@@ -25,7 +25,12 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
   void Free(void* data, size_t length) override;
   void Free(void* data, size_t length, AllocationMode mode) override;
 
-  void SetProtection(void* data, size_t length, Protection protection) override;
+  // SetProtection should return result of mprotect calls - duplication of
+  // methods here as this change needs coordination between V8/Chromium.
+  // Subsequent changes will implement 'bool SetProtection(...)' to be the
+  // default method.
+  void SetProtection(void* data, size_t length, Protection protection) final;
+  bool SetProtection(Protection protection, void* data, size_t length);
 
   GIN_EXPORT static ArrayBufferAllocator* SharedInstance();
 };

third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp

@@ -575,19 +575,20 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
     }
   }
 
-  void SetProtection(void* data,
-                     size_t length,
-                     Protection protection) override {
+  void SetProtection(void* data, size_t length, Protection protection) {
+    CHECK(SetProtection(protection, data, length));
+  }
+
+  bool SetProtection(Protection protection, void* data, size_t length) {
     switch (protection) {
       case Protection::kNoAccess:
-        CHECK(WTF::SetSystemPagesAccess(data, length, WTF::PageInaccessible));
-        return;
+        return WTF::SetSystemPagesAccess(data, length, WTF::PageInaccessible);
       case Protection::kReadWrite:
-        CHECK(WTF::SetSystemPagesAccess(data, length, WTF::PageReadWrite));
-        return;
+        return WTF::SetSystemPagesAccess(data, length, WTF::PageReadWrite);
       default:
         NOTREACHED();
     }
+    return false;
   }
 };