Update base::win use of IsWow64Process to IsWow64Process2 [chromium/src : main]

293 views
Skip to first unread message

Sarah Murphy (Gerrit)

unread,
Sep 7, 2021, 5:37:26 PM9/7/21
to Pavel Feldman, Camille Lamy, Eric Seckler, Varun Khaneja, Mark Seaborn, Gabriel Charette, Nico Weber, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Robert Liao, Will Harris

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Eric Seckler, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

Sarah Murphy would like Pavel Feldman, Camille Lamy, Eric Seckler, Varun Khaneja, Mark Seaborn, Gabriel Charette and Nico Weber to review this change.

View Change

Update base::win use of IsWow64Process to IsWow64Process2

This change updates the use of IsWow64Process in base/win to
IsWow64Process2, and updates the WOW64Status enum to WOWStatus. The
function will now return WOW_ARM for use of WOW64 on an ARM64 host
machine, and WOW_64 for all other uses. This change also updates the
places in code where wow64_status is checked.

R=erik.a...@microsoft.com, rob...@chromium.org, w...@chromium.org

Bug: 978257
Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
---
M base/debug/gdi_debug_util_win.cc
M base/win/windows_version.cc
M base/win/windows_version.h
M chrome/browser/nacl_host/test/nacl_gdb_browsertest.cc
M chrome/chrome_cleaner/http/http_agent_impl.cc
M chrome/chrome_cleaner/os/disk_util.cc
M chrome/chrome_cleaner/os/system_util.cc
M chrome/test/nacl/nacl_browsertest.cc
M components/nacl/browser/nacl_process_host.cc
M components/update_client/protocol_serializer.cc
M content/browser/tracing/tracing_controller_impl.cc
M content/common/user_agent.cc
M sandbox/win/src/lpc_policy_test.cc
M sandbox/win/tests/common/controller.cc
14 files changed, 73 insertions(+), 45 deletions(-)


To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 1
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Eric Seckler <esec...@chromium.org>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-MessageType: newchange

Sarah Murphy (Gerrit)

unread,
Sep 7, 2021, 5:37:31 PM9/7/21
to grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Eric Seckler, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Eric Seckler, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

Patch set 1:Commit-Queue +1

View Change

1 comment:

  • Patchset:

    • Patch Set #1:

      I'm not too familiar of who is the appropriate owner for these areas of code - please feel free to add/remove reviewers as is appropriate. Thank you!

Gerrit-Comment-Date: Tue, 07 Sep 2021 21:37:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Erik Anderson (Gerrit)

unread,
Sep 7, 2021, 6:52:46 PM9/7/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Eric Seckler, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Eric Seckler, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

View Change

13 comments:

  • File base/win/windows_version.h:

    • Patch Set #1, Line 109: // Chrome on 64-bit Windows". WOW_ARM means the native machine is an ARM64,

      Please update this comment to represent that WOW_ARM means "running under a WoW environment (the wrapper that allows non-native processes to run on Windows)." As written, it reads as though the value will be WOW_ARM even when running a native process (when in actuality we want it it to be WOW_DIABLED).

  • File base/win/windows_version.cc:

    • Patch Set #1, Line 259: if (!::IsWow64Process2(process_handle, &process_machine, &native_machine))

      Per https://docs.microsoft.com/en-us/windows/win32/api/wow64apiset/nf-wow64apiset-IsWow64Process2, this was added in Windows 10 version 1511.

      There's a couple of pre-existing examples of code (e.g. in process_mitigations.cc) calling IsWow64Process2 with a runtime availability check which you should follow.

      As part of this change or a future one, we should pick up the work to complete teh TODO on that method so that it calls this new base implementation instead (e.g. it could call this method and return true if this returns WOW_ARM).

    • Patch Set #1, Line 262: return WOW_ARM;

      Per the docs (https://docs.microsoft.com/en-us/windows/win32/api/wow64apiset/nf-wow64apiset-iswow64process2), when not a WoW process, pProcessMachine will be IMAGE_FILE_MACHINE_UNKNOWN.

      The pNativeMachine value may be set even when not a WoW process, so the check below for native_machine == IMAGE_FILE_MACHINE_ARM64 could possibly cause us to output WOW_ARM even for a native process.

      Can we restructure the check like so?:

        USHORT process_machine;
      USHORT native_machine;
      // (fix IsWow64Process2 call per other comment to dynamically check if it's available)
      if (!::IsWow64Process2(process_handle, &process_machine, &native_machine))
      return WOW_UNKNOWN;
      if (process_machine == IMAGE_FILE_MACHINE_UNKNOWN)
      return WOW_DISABLED;
        switch (native_machine) {
      case IMAGE_FILE_MACHINE_ARM64:
      return WOW_ARM;
      case default:
      return WOW_64;
      }


      Note that the existing process_mitigations.cc check looks directly at native_machine == IMAGE_FILE_MACHINE_ARM64, so I'm not if that's getting the wrong result with a native ARM64 binary today. It's quite possible since the only outcome of that is that ACG would be off on ARM64 in general.

      Can you please get an ARM64 device and check the actual behavior of IsRunning32bitEmulatedOnArm64 to confirm if this would be a net change if we move that to use this updated logic?

  • File chrome/chrome_cleaner/http/http_agent_impl.cc:

    • Patch Set #1, Line 517: os_info->wow_status() == base::win::OSInfo::WOW_ARM) {

      In general, I don't believe we should advertise WOW64 in any UA strings on ARM64 since the meaning of that token means x86-on-AMD64. Since we don't have any other token we'd want to use, let's keep this check scoped to WOW_64 (don't check WOW_ARM).

  • File chrome/chrome_cleaner/os/disk_util.cc:

    • Patch Set #1, Line 792: base::win::OSInfo::GetInstance()->wow_status() ==

      Given the pre-existing code would have skipped this path, I'm inclined to keep this code as it was (tied to just WOW_64).

      If it's currently broken on ARM install (I don't know if it actually is; the ExpandEnvPath may work just fine on the line above) that should be investigated at a later point.

  • File chrome/chrome_cleaner/os/system_util.cc:

    • Patch Set #1, Line 64: return (base::win::OSInfo::GetInstance()->wow_status() ==

      Note: I was inclined to say not to touch this one either, but the only caller is line 77 which has an IsX64Architecture check which is tied to X64_ARCHITECTURE.

      So, this change should be a no-op which is fine. If the "Chrome Cleaner" code needs further updates for ARM64 that can be handled at some later point, e.g. by modifying the IsX64Architecture check and its callers.

  • File components/nacl/browser/nacl_process_host.cc:

    • Patch Set #1, Line 159: base::win::OSInfo::GetInstance()->wow_status() ==

      Similar sentiment on only touching things that we know need to be upgraded to a better understanding-- I don't know if NaCL is even supported on ARM64 on Windows; even if it is, if it's working with a WOW64_ENABLED check today we should let it stick with being scoped to that.

  • File components/update_client/protocol_serializer.cc:

    • Patch Set #1, Line 112: if (base::win::OSInfo::GetInstance()->wow_status() ==

      It's possible the updater folks would like to be able to differentiate here on ARM64, but for now I'm not inclined to change what it observes today. Let's also keep this scoped x86-on-AMD64 (WOW_64).

  • File content/browser/tracing/tracing_controller_impl.cc:

    • Patch Set #1, Line 294: base::win::OSInfo::GetInstance()->wow_status() ==

      As authored, the surrounding X64_ARCHITECTURE check will mean we never see this evaluate to true.

      Since outputting this metadata would be reasonable and likely useful, in this instance I would expand the surrounding check to also check against ARM64_ARCHITECTURE so that we run this check.

  • File content/common/user_agent.cc:

    • Patch Set #1, Line 83: os_info->wow_status() == base::win::OSInfo::WOW_ARM) {

      We don't want to output the WOW64 token on ARM64 since the meaning to sites is different. Let's scope exposure of this new context to UA Client Hints (which can come in a follow-on change). Make this check just for WOW_64.

  • File sandbox/win/src/lpc_policy_test.cc:

    • Patch Set #1, Line 34: return (base::win::OSInfo::GetInstance()->wow_status() ==

      Unless we have a specific test observation that this doesn't work in the ARM64 WoW environment, let's keep this change more minimal and keep this check scoped to x86-on-AMD64.

  • File sandbox/win/tests/common/controller.cc:

    • Patch Set #1, Line 77: base::win::OSInfo::WOW_ARM)

      If we want to upgrade this check to do what it's mean to (an architecture-independent path for the system32 folder matching this process's arhcitecture), we'd actually need to map this to SysArm32 per https://docs.microsoft.com/en-us/windows/win32/winprog64/file-system-redirector.

      However, this is another example where if we don't have known broken tests, the file system redirector is likely doing just fine for these tests and we shouldn't try to bring this functionality forward to WOW_ARM.

    • Patch Set #1, Line 163: if (base::win::OSInfo::GetInstance()->wow_status() ==

      Let's keep this scoped to WOW64_ENABLED.

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 1
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Eric Seckler <esec...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Tue, 07 Sep 2021 22:52:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Erik Anderson (Gerrit)

unread,
Sep 7, 2021, 7:22:23 PM9/7/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Eric Seckler, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Eric Seckler, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

View Change

2 comments:

  • File base/base_paths_win.cc:

    • Patch Set #2, Line 76: base::win::OSInfo::WOW_ARM) {

      If we add the separate state tracking for what type of ARM WoW scenario this is, then this check potentially makes sense in terms of install folder intuitively being the x86 path.

      But at the same time I think the file system redirector should already be doing something reasonable and swapping the meaning of this after installs have already happened may result in a disconnect between where the Chrome Cleaner code is looking for things and where the install previously placed things.

      So, I would either not upgrade this code beyond the existing WOW_64 check. If we need something additional later, the callers could be upgraded and/or a new DIR_* value added for "I know the ARM folder structure" callers.

  • File base/win/windows_version.h:

    • Patch Set #2, Line 116: WOW_ARM,

      Something we should consider further--

      There's at least two possible WoW-on-ARM configurations-- x86-on-ARM64 and arm32-on-ARM64.

      Things that craft paths may actually desire to differ, e.g. x86 processes use `Program Files (x86)` while the arm32 uses `Program Files (arm)`.

      ARM32 binaries should generally be rare (folks targeting ARM on Windows should generally be targeting ARM64 at this point, but I worry about potential confusion that could result from unclear usages.

      We should consider having WOW_X86_ON_AMD64 (same meaning as what WOW64_ENABLED was), WOW_X86_ON_ARM64, and a WOW_NON_X86_ON_ARM64 values. I'm not sure if there's anyone actually using this code in an arm32 binary which is an interesting wrinkle in terms of if the added complexity makes sense.

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 2
Gerrit-Comment-Date: Tue, 07 Sep 2021 23:22:15 +0000

Eric Seckler (Gerrit)

unread,
Sep 8, 2021, 5:07:13 AM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

Patch set 2:Code-Review +1

View Change

1 comment:

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 2
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 09:06:57 +0000

Sarah Murphy (Gerrit)

unread,
Sep 8, 2021, 2:54:36 PM9/8/21
to grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Erik Anderson, Pavel Feldman, Camille Lamy, Will Harris, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

View Change

1 comment:

  • File base/win/windows_version.h:

    • Something we should consider further-- […]

      I'm working on making the other updates now, but thought it would be a good idea to verify my understanding before pushing the code change. This is the update I was thinking of making:

        // This status represents the different relevant scenarios for a process to be
      // running with consideration towards WOW64 (the wrapper that allows 32-bit
      // processes to run on 64-bit versions of Windows).
      // 32-bit Chrome on 32-bit Windows: WOW_DISABLED
      // 64-bit Chrome on 64-bit Windows: WOW_DISABLED
      // 32-bit Chrome on AMD64 Windows: WOW_X86_ON_AMD64
      // 32-bit (x86) Chrome on ARM64 Windows: WOW_X86_ON_ARM64
      // 32-bit (arm32) Chrome on ARM64 Windows: WOW_NON_X86_ON_ARM64
      enum WOWStatus {
      WOW_DISABLED,
      WOW_X86_ON_AMD64,
      WOW_X86_ON_ARM64,
      WOW_NON_X86_ON_ARM64,
      WOW_UNKNOWN,
      };

      Does this look alright/correct to you? Thanks!

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 2
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 18:54:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-MessageType: comment

Mark Seaborn (Gerrit)

unread,
Sep 8, 2021, 3:02:33 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Erik Anderson, Pavel Feldman, Camille Lamy, Will Harris, Varun Khaneja, Gabriel Charette, Robert Liao, Nico Weber.

View Change

1 comment:

  • File components/nacl/browser/nacl_process_host.cc:

    • Similar sentiment on only touching things that we know need to be upgraded to a better understanding […]

      NaCl is not supported on ARM on Windows (either 32-bit or 64-bit ARM), so all of the NaCl-related cases should definitely not be changed to check for WOW on ARM.

Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 19:02:23 +0000

Erik Anderson (Gerrit)

unread,
Sep 8, 2021, 3:13:16 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Gabriel Charette, Robert Liao, Nico Weber.

View Change

1 comment:

  • File base/win/windows_version.h:

    • I'm working on making the other updates now, but thought it would be a good idea to verify my unders […]

      Yes, that's what I had in mind in my comment.

      If we wouldn't, as part of this change, have any code specifically checking WOW_NON_X86_ON_ARM64, my preference would be to instead have a value named something like WOW_OTHER (and map ay other unknown configurations to that value). If future use cases need to know of a specific combo (such as ARM32-on-ARM64), then new enum values could be added on an as-needed basis.

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 2
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 19:13:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Erik Anderson <Erik.A...@microsoft.com>
Comment-In-Reply-To: Sarah Murphy <mur...@microsoft.com>
Gerrit-MessageType: comment

Robert Liao (Gerrit)

unread,
Sep 8, 2021, 6:49:09 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Gabriel Charette, Nico Weber.

View Change

1 comment:

  • File base/win/windows_version.h:

    • Patch Set #2, Line 128: static WOWStatus GetWOWStatusForProcess(HANDLE process_handle);

      Seems like many, if not most, callers just care if we're running under WoW instead of the specific WoW type. It might be more robust to expose something like IsProcessRunningAsWOW (or something similar) rather than checking against WOW_64/WOW_ARM or DISABLED/UNKNOWN at all of these sites.

      Thoughts?

Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 22:48:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Erik Anderson (Gerrit)

unread,
Sep 8, 2021, 7:20:11 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Gabriel Charette, Robert Liao, Nico Weber.

View Change

1 comment:

  • File base/win/windows_version.h:

    • Seems like many, if not most, callers just care if we're running under WoW instead of the specific W […]

      Reviewing most of the existing callers, I believe almost all of them care specifically about the traditional x86-on-amd64. If we had that same set of code become enlightened to x86-on-arm64, there's likely implications on installer code and other things that have otherwise been working okay until this point.

      If we have consumers who truly want to treat all WoW scenarios the same way then I agree a broad helper would make sense.

      For the existing sandbox code that already uses IsWow64Process2, it is specifically checking for running x86-on-ARM64 which it wants to handle differently than the traditional WoW.

      For the UA Client Hints' Sec-CH-UA-Arch value, we also need to explicitly differentiate between AMD64 and ARM64 so that the correct value can be filled in.

      So... I think separate values generally makes sense, though one or more helper methods may make sense if there is a common bucketing of values we care about.

      mur...@microsoft.com-- I think the updated change calling IsWOWEnabled is changing the pre-existing behavior of too much code. This change would ideally keep existing code running with the same behavior as today (modulo spots where we are 100% confident a change is strictly better, e.g. the tracing code) and then in the future if owners of those calls want different behavior they can make sure they understand the impact of the change when running against an existing install.

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 3
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 23:20:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Robert Liao <rob...@chromium.org>
Gerrit-MessageType: comment

Sarah Murphy (Gerrit)

unread,
Sep 8, 2021, 7:33:44 PM9/8/21
to grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Erik Anderson, Pavel Feldman, Camille Lamy, Will Harris, Varun Khaneja, Mark Seaborn, Gabriel Charette, Robert Liao, Nico Weber.

View Change

16 comments:

  • File base/base_paths_win.cc:

    • If we add the separate state tracking for what type of ARM WoW scenario this is, then this check pot […]

      Alright, I've modified this check to only check for x86-on-AMD64! Thanks!

  • File base/win/windows_version.h:

    • Patch Set #2, Line 116: WOW_ARM,

      Yes, that's what I had in mind in my comment. […]

      Alright, I swapped out WOW_NON_X86_ON_ARM64 for WOW_OTHER and implemented what I have above. Thanks!

    • Patch Set #2, Line 128: static WOWStatus GetWOWStatusForProcess(HANDLE process_handle);

      Seems like many, if not most, callers just care if we're running under WoW instead of the specific W […]

    • I added a function IsWOWEnabled that checks the wow_status value. Do you think it would be better to have the function take in the process_handle instead, or to add a private member bool wow_enabled, that could be called via wow_enabled()?

  • File base/win/windows_version.h:

    • Please update this comment to represent that WOW_ARM means "running under a WoW environment (the wra […]

      Updated this significantly, so it more closely matches the comment on the architecture enum up above. Thanks!

  • File base/win/windows_version.cc:

    • Per the docs (https://docs.microsoft. […]

      I've updated to check both the process machine and the native machine, since we added the WOW_OTHER value. I'll leave this comment open until I have a chance to test this on an ARM64 machine!

  • File chrome/chrome_cleaner/http/http_agent_impl.cc:

    • In general, I don't believe we should advertise WOW64 in any UA strings on ARM64 since the meaning o […]

      Fixed, thanks!

  • File chrome/chrome_cleaner/os/disk_util.cc:

    • Given the pre-existing code would have skipped this path, I'm inclined to keep this code as it was ( […]

      I've updated it for now - if the test breaks when I use the ARM machine or in dry-runs, I'll revert it! Keeping this comment open until I get that checked.

  • File chrome/chrome_cleaner/os/system_util.cc:

    • Note: I was inclined to say not to touch this one either, but the only caller is line 77 which has a […]

      Okay! I've updated it to use the IsWOWEnabled function since I think that's more clear. Thanks!

  • File components/nacl/browser/nacl_process_host.cc:

    • NaCl is not supported on ARM on Windows (either 32-bit or 64-bit ARM), so all of the NaCl-related ca […]

      Thanks, I've removed the ARM check!

  • File components/update_client/protocol_serializer.cc:

    • It's possible the updater folks would like to be able to differentiate here on ARM64, but for now I' […]

      Okay, fixed! Thanks!

  • File content/browser/tracing/tracing_controller_impl.cc:

    • As authored, the surrounding X64_ARCHITECTURE check will mean we never see this evaluate to true. […]

      I've added the ARM64_ARCHITECTURE check - thanks!

  • File content/common/user_agent.cc:

    • We don't want to output the WOW64 token on ARM64 since the meaning to sites is different. […]

      Fixed, thanks! I'll make the UA change in a different CL.

  • File sandbox/win/src/lpc_policy_test.cc:

    • Unless we have a specific test observation that this doesn't work in the ARM64 WoW environment, let' […]

      Done, thanks!

  • File sandbox/win/tests/common/controller.cc:

    • If we want to upgrade this check to do what it's mean to (an architecture-independent path for the s […]

      Fixed, thanks!

    • Patch Set #1, Line 163: if (base::win::OSInfo::GetInstance()->wow_status() ==

      Let's keep this scoped to WOW64_ENABLED.

    • Done, thanks!

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 4
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Robert Liao <rob...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 23:33:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Erik Anderson <Erik.A...@microsoft.com>
Comment-In-Reply-To: Sarah Murphy <mur...@microsoft.com>
Comment-In-Reply-To: Mark Seaborn <msea...@chromium.org>

Robert Liao (Gerrit)

unread,
Sep 8, 2021, 7:43:53 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Erik Anderson, Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Nico Weber.

View Change

5 comments:

  • Patchset:

    • Patch Set #4:

      Metacomment: Given the wide reach of the change, I'd recommend breaking this change into smaller changes as there is bit to audit (and get right). This allows you to make forward progress as owners approve their impacted areas.

      Once we have general agreement on the appropriate way to expose this, then update the dependencies (in logical groups) to use the IsWow64Process2 paths.

      After everyone has been moved over, remove GetWOWStatusForProcess.

  • File base/win/windows_version.cc:

Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 23:43:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Erik Anderson (Gerrit)

unread,
Sep 8, 2021, 7:45:07 PM9/8/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Nico Weber.

View Change

2 comments:

  • File base/win/windows_version.h:

    • Patch Set #2, Line 128: static WOWStatus GetWOWStatusForProcess(HANDLE process_handle);

      I added a function IsWOWEnabled that checks the wow_status value. […]

      Almost all of the existing callers should explicitly check against a single value (WOW_X86_ON_AMD64). For now, I would not use the helper.

      If down the road we have repeating code that needs to check a couple of values, then a dedicated helper for that would make sense; I'd also use a more specific name in that case such as IsTraditionalWOWEnabled or IsX86RunningUnderWOW. But I'm not sure we have any current callers that would want to explicitly lump x86-on-AMD64 and x86-on-ARM64 together.

  • File base/win/windows_version.cc:

    • Patch Set #4, Line 262: return WOW_UNKNOWN;

      Instead of bailing if we can't find IsWow64Process2, we need to instead fall back to the IsWow64Process check (if true, it's WOW_X86_ON_AMD64).

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 4
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 23:44:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sarah Murphy <mur...@microsoft.com>

Greg Thompson (Gerrit)

unread,
Sep 9, 2021, 3:42:47 AM9/9/21
to Sarah Murphy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Varun Khaneja, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Varun Khaneja, Mark Seaborn, Gabriel Charette, Nico Weber.

View Change

5 comments:

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 4
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Varun Khaneja <va...@chromium.org>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-CC: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Varun Khaneja <va...@chromium.org>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-Comment-Date: Thu, 09 Sep 2021 07:42:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Sarah Murphy (Gerrit)

unread,
Sep 9, 2021, 7:30:28 PM9/9/21
to grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Greg Thompson, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Gabriel Charette, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Mark Seaborn, Gabriel Charette, Nico Weber.

Sarah Murphy removed Varun Khaneja from this change.

View Change

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 5
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Gabriel Charette <g...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-CC: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>
Gerrit-Attention: Gabriel Charette <g...@chromium.org>
Gerrit-Attention: Nico Weber <tha...@chromium.org>
Gerrit-MessageType: deleteReviewer

Sarah Murphy (Gerrit)

unread,
Sep 9, 2021, 7:30:41 PM9/9/21
to Gabriel Charette, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Greg Thompson, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Camille Lamy, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Camille Lamy, Will Harris, Sarah Murphy, Mark Seaborn, Nico Weber.

Sarah Murphy removed Gabriel Charette from this change.

View Change

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 5
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Camille Lamy <cl...@chromium.org>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-CC: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>
Gerrit-Attention: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Will Harris <w...@chromium.org>
Gerrit-Attention: Sarah Murphy <mur...@microsoft.com>
Gerrit-Attention: Mark Seaborn <msea...@chromium.org>

Sarah Murphy (Gerrit)

unread,
Sep 9, 2021, 7:30:48 PM9/9/21
to Camille Lamy, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Greg Thompson, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Pavel Feldman, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Pavel Feldman, Will Harris, Sarah Murphy, Mark Seaborn, Nico Weber.

Sarah Murphy removed Camille Lamy from this change.

View Change

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 5
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Pavel Feldman <pfel...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-CC: Greg Thompson <g...@chromium.org>
Gerrit-Attention: Pavel Feldman <pfel...@chromium.org>

Sarah Murphy (Gerrit)

unread,
Sep 9, 2021, 7:30:56 PM9/9/21
to Pavel Feldman, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, tracing...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Greg Thompson, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Will Harris, Sarah Murphy, Mark Seaborn, Nico Weber.

Sarah Murphy removed Pavel Feldman from this change.

View Change

To view, visit change 3142901. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Id714bf8300c918ccd40df5931f31d82fb6ceec2b
Gerrit-Change-Number: 3142901
Gerrit-PatchSet: 5
Gerrit-Owner: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Eric Seckler <esec...@chromium.org>
Gerrit-Reviewer: Mark Seaborn <msea...@chromium.org>
Gerrit-Reviewer: Nico Weber <tha...@chromium.org>
Gerrit-Reviewer: Robert Liao <rob...@chromium.org>
Gerrit-Reviewer: Sarah Murphy <mur...@microsoft.com>
Gerrit-Reviewer: Will Harris <w...@chromium.org>
Gerrit-CC: Erik Anderson <Erik.A...@microsoft.com>
Gerrit-CC: Greg Thompson <g...@chromium.org>

Sarah Murphy (Gerrit)

unread,
Sep 9, 2021, 7:31:13 PM9/9/21
to tracing...@chromium.org, grt+...@chromium.org, jessemcke...@google.com, joenotcha...@chromium.org, roblia...@chromium.org, spang...@chromium.org, vmpstr...@chromium.org, wfh+...@chromium.org, erik.a...@microsoft.com, Greg Thompson, Eric Seckler, Erik Anderson, Chromium LUCI CQ, Nico Weber, Mark Seaborn, Robert Liao, Will Harris, chromium...@chromium.org, native-cli...@googlegroups.com

Attention is currently required from: Will Harris, Sarah Murphy, Mark Seaborn, Nico Weber.

Sarah Murphy removed tracing...@chromium.org from this change.

Reply all
Reply to author
Forward
0 new messages