[PartitionAlloc] Make pages inaccessible *after* discarding

crrev.com/c/2550119 caused a surprising memory regression on Mac.
It was taken care of in crrev.com/c/2563038, by "relaxing" one caller,
but the question of other caller remained. Turns out that reordering
SetSystemPagesAccess past DiscardSystemPages in DecommitSystemPages also
takes care of that regression:
https://pinpoint-dot-chromeperf.appspot.com/job/1425cfb7520000

This is unfortunate, as preferably we'd want to avoid a window when
discarded pages can be faulted in. However, prior to crrev.com/c/2550119
almost all callers (except one) would call SetSystemPagesAccess after
DiscardSystemPages, so in that sense this CL brings back the old, well
tested behavior.

Similarly reorder operations in RecommitSystemPages. The callers were
split 50/50 on this prior to crrev.com/c/2550119 and no regressions
were observed, so it likely doesn't matter either way. It just seems
to make more sense to have operations in the opposite order to
DecommitSystemPages.

PS
This CL doesn't change memory usage on Linux or Android, but also no
regressions were observed there to begin with.
https://pinpoint-dot-chromeperf.appspot.com/job/172620fb520000
https://pinpoint-dot-chromeperf.appspot.com/job/15212cd7520000

PS2
It has been confirmed theoretically that ordering on Fuchsia shouldn't
matter, so stick to the preferred ordering.

Bug: 1153021
Change-Id: Icb937ffed2420859e6006aa2e1af555661bfe541
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2563052Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833631}

base/allocator/partition_allocator/page_allocator_internals_posix.h

@@ -247,14 +247,20 @@ void DecommitSystemPagesInternal(
     void* address,
     size_t length,
     PageAccessibilityDisposition accessibility_disposition) {
-  if (accessibility_disposition == PageUpdatePermissions) {
-    SetSystemPagesAccess(address, length, PageInaccessible);
-  }
-
   // In POSIX, there is no decommit concept. Discarding is an effective way of
   // implementing the Windows semantics where the OS is allowed to not swap the
   // pages in the region.
   DiscardSystemPages(address, length);
+
+  // Make pages inaccessible, unless the caller requested to keep permissions.
+  //
+  // Note, there is a small window between these calls when the pages can be
+  // incorrectly touched and brought back to memory. Not ideal, but doing those
+  // operaions in the opposite order resulted in PMF regression on Mac (see
+  // crbug.com/1153021).
+  if (accessibility_disposition == PageUpdatePermissions) {
+    SetSystemPagesAccess(address, length, PageInaccessible);
+  }
 }
 
 void RecommitSystemPagesInternal(
@@ -262,12 +268,6 @@ void RecommitSystemPagesInternal(
     size_t length,
     PageAccessibilityConfiguration accessibility,
     PageAccessibilityDisposition accessibility_disposition) {
-#if defined(OS_APPLE)
-  // On macOS, to update accounting, we need to make another syscall. For more
-  // details, see https://crbug.com/823915.
-  madvise(address, length, MADV_FREE_REUSE);
-#endif
-
   // On POSIX systems, the caller need simply read the memory to recommit it.
   // This has the correct behavior because the API requires the permissions to
   // be the same as before decommitting and all configurations can read.
@@ -276,6 +276,12 @@ void RecommitSystemPagesInternal(
   if (accessibility_disposition == PageUpdatePermissions) {
     SetSystemPagesAccess(address, length, accessibility);
   }
+
+#if defined(OS_APPLE)
+  // On macOS, to update accounting, we need to make another syscall. For more
+  // details, see https://crbug.com/823915.
+  madvise(address, length, MADV_FREE_REUSE);
+#endif
 }
 
 void DiscardSystemPagesInternal(void* address, size_t length) {