Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(14)

Issue 2581353002: Use the Windows MDM API to check if the machine is being managed. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months ago by Roger Tawa OOO till Jul 10th
Modified:
5 months, 2 weeks ago
CC:
chromium-reviews, grt+watch_chromium.org, vmpstr+watch_chromium.org, wfh+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use the Windows MDM API to check if the machine is being managed. BUG=690163 Review-Url: https://codereview.chromium.org/2581353002 Cr-Original-Commit-Position: refs/heads/master@{#452028} Committed: https://chromium.googlesource.com/chromium/src/+/ee0c03ddb4a10c8c9d14244bdefc8034054e846e Review-Url: https://codereview.chromium.org/2581353002 Cr-Commit-Position: refs/heads/master@{#452484} Committed: https://chromium.googlesource.com/chromium/src/+/57aed38568117ea2787c4b7aa7e24ce27934c7e8

Patch Set 1 #

Patch Set 2 : Fix components tests #

Total comments: 2

Patch Set 3 : Log hr and state separately #

Patch Set 4 : Check HRESULT separately #

Patch Set 5 : rebased #

Patch Set 6 : Remove debug logging #

Patch Set 7 : Add uma logging for new api #

Total comments: 11

Patch Set 8 : Address review comments #

Patch Set 9 : Address review comments #

Patch Set 10 : rebased #

Patch Set 11 : Fix missing rename from enterprise-user to entprise-managed #

Total comments: 12

Patch Set 12 : Address review comments #

Total comments: 2

Patch Set 13 : Address review comments #

Patch Set 14 : rebased #

Patch Set 15 : Fix tests #

Total comments: 6

Patch Set 16 : Address review comments #

Total comments: 10

Patch Set 17 : Address review comments #

Patch Set 18 : rebased #

Patch Set 19 : rebased and fixed hard coded string in render_process_impl.cc #

Total comments: 2

Patch Set 20 : Fix long line and comment #

Patch Set 21 : rebased #

Patch Set 22 : Fix typo in official build #

Patch Set 23 : Add missing file after rebase patchset #21 #

Patch Set 24 : rebased #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -46 lines) Patch
M base/win/win_util.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +8 lines, -0 lines 0 comments Download
M base/win/win_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +34 lines, -0 lines 2 comments Download
M chrome/app/chrome_crash_reporter_client_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chrome_browser_main.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chrome_browser_main_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +5 lines, -4 lines 0 comments Download
M chrome/browser/component_updater/chrome_component_updater_configurator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +4 lines, -5 lines 0 comments Download
M chrome/browser/prefs/chrome_pref_service_factory.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/upgrade_detector_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common/crash_keys.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common/crash_keys.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +2 lines, -2 lines 0 comments Download
M components/policy/core/common/policy_loader_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 4 chunks +16 lines, -6 lines 0 comments Download
M components/update_client/updater_state.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +3 lines, -3 lines 0 comments Download
M components/update_client/updater_state.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +7 lines, -3 lines 0 comments Download
M components/update_client/updater_state_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +2 lines, -2 lines 0 comments Download
M components/update_client/updater_state_win.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -2 lines 0 comments Download
M components/update_client/utils.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +5 lines, -4 lines 0 comments Download
M content/renderer/render_process_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +4 lines, -4 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +17 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 121 (78 generated)
Roger Tawa OOO till Jul 10th
Hi Georges, Please take a look. This API is already available in windows10 pro version ...
8 months ago (2016-12-16 21:00:54 UTC) #5
Roger Tawa OOO till Jul 10th
Hi Georges, Please take a look. I added UMA logging as we discussed today. A ...
6 months, 1 week ago (2017-02-08 21:47:54 UTC) #21
grt (UTC plus 2)
base/win drive-by https://codereview.chromium.org/2581353002/diff/140001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/140001/base/win/win_util.cc#newcode504 base/win/win_util.cc:504: static IsDeviceRegisteredWithManagementFunction fn = nullptr; please initialize ...
6 months, 1 week ago (2017-02-08 21:59:56 UTC) #23
Roger Tawa OOO till Jul 10th
Thanks Greg. See question below. https://codereview.chromium.org/2581353002/diff/140001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/140001/base/win/win_util.cc#newcode504 base/win/win_util.cc:504: static IsDeviceRegisteredWithManagementFunction fn = ...
6 months, 1 week ago (2017-02-09 14:55:59 UTC) #26
Georges Khalil
Just minor comments. I think we should not enable the MDM check just yet, until ...
6 months, 1 week ago (2017-02-09 14:59:57 UTC) #27
Roger Tawa OOO till Jul 10th
Thanks Georges. I sent an email to Sorin about string value so let's see. https://codereview.chromium.org/2581353002/diff/140001/base/win/win_util.cc ...
6 months, 1 week ago (2017-02-09 16:06:44 UTC) #28
grt (UTC plus 2)
a smattering of comments https://codereview.chromium.org/2581353002/diff/220001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/220001/base/win/win_util.cc#newcode505 base/win/win_util.cc:505: static IsDeviceRegisteredWithManagementFunction fn = nullptr; ...
6 months, 1 week ago (2017-02-10 12:12:26 UTC) #33
Roger Tawa OOO till Jul 10th
Hi Greg, please take another look. Thanks. https://codereview.chromium.org/2581353002/diff/220001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/220001/base/win/win_util.cc#newcode505 base/win/win_util.cc:505: static IsDeviceRegisteredWithManagementFunction ...
6 months ago (2017-02-13 20:30:06 UTC) #34
grt (UTC plus 2)
https://codereview.chromium.org/2581353002/diff/240001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/240001/base/win/win_util.cc#newcode505 base/win/win_util.cc:505: static IsDeviceRegisteredWithManagementFunction fn = nullptr; my remark about avoiding ...
6 months ago (2017-02-13 21:06:25 UTC) #35
Roger Tawa OOO till Jul 10th
Also added comment to explain why value of string string with omaha is not changing. ...
6 months ago (2017-02-14 19:23:44 UTC) #42
grt (UTC plus 2)
apologies for missing these before. base/win lgtm with these changes. https://codereview.chromium.org/2581353002/diff/300001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/300001/base/win/win_util.cc#newcode501 ...
6 months ago (2017-02-14 21:20:13 UTC) #45
Roger Tawa OOO till Jul 10th
Thanks Greg. https://codereview.chromium.org/2581353002/diff/300001/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/300001/base/win/win_util.cc#newcode501 base/win/win_util.cc:501: static auto is_device_registered_with_management = []() { On ...
6 months ago (2017-02-15 19:11:27 UTC) #47
Georges Khalil
LGTM as well.
6 months ago (2017-02-15 19:14:40 UTC) #49
Roger Tawa OOO till Jul 10th
Hi Alexei, Julian, Joshua, Scott, Can you please do owner reviews? I basically changed the ...
6 months ago (2017-02-15 20:36:40 UTC) #51
waffles
component_updater lgtm
6 months ago (2017-02-15 20:44:16 UTC) #52
Alexei Svitkine (slow)
lgtm for histograms.xml
6 months ago (2017-02-15 21:16:28 UTC) #55
sky
https://codereview.chromium.org/2581353002/diff/320001/chrome/browser/chrome_browser_main_win.cc File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2581353002/diff/320001/chrome/browser/chrome_browser_main_win.cc#newcode314 chrome/browser/chrome_browser_main_win.cc:314: // Record whether the machine is domain joined in ...
6 months ago (2017-02-15 21:27:15 UTC) #56
pastarmovj
policy lgtm. https://codereview.chromium.org/2581353002/diff/320001/components/policy/core/common/policy_loader_win.cc File components/policy/core/common/policy_loader_win.cc (right): https://codereview.chromium.org/2581353002/diff/320001/components/policy/core/common/policy_loader_win.cc#newcode93 components/policy/core/common/policy_loader_win.cc:93: bool ShouldHonorPolicies() { nit: I presume the ...
6 months ago (2017-02-17 16:21:17 UTC) #57
Roger Tawa OOO till Jul 10th
Thanks Julian, Scott. Comments addressed, please take a look. Scott: currently the enterprise constant is ...
6 months ago (2017-02-17 19:20:13 UTC) #60
sky
LGTM
6 months ago (2017-02-17 20:53:57 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/360001
6 months ago (2017-02-17 21:02:45 UTC) #66
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
6 months ago (2017-02-17 23:06:54 UTC) #68
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/360001
6 months ago (2017-02-18 17:17:08 UTC) #70
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/385645)
5 months, 4 weeks ago (2017-02-18 21:01:54 UTC) #72
Roger Tawa OOO till Jul 10th
Hi Avi, Could you please do an owner review for content/renderer/render_process_impl.cc? Turns out this string ...
5 months, 3 weeks ago (2017-02-21 18:32:42 UTC) #78
Will Harris (OOO)
https://codereview.chromium.org/2581353002/diff/380001/content/renderer/render_process_impl.cc File content/renderer/render_process_impl.cc (right): https://codereview.chromium.org/2581353002/diff/380001/content/renderer/render_process_impl.cc#newcode142 content/renderer/render_process_impl.cc:142: // Record whether the machine is domain joined in ...
5 months, 3 weeks ago (2017-02-21 18:34:52 UTC) #80
Roger Tawa OOO till Jul 10th
https://codereview.chromium.org/2581353002/diff/380001/content/renderer/render_process_impl.cc File content/renderer/render_process_impl.cc (right): https://codereview.chromium.org/2581353002/diff/380001/content/renderer/render_process_impl.cc#newcode142 content/renderer/render_process_impl.cc:142: // Record whether the machine is domain joined in ...
5 months, 3 weeks ago (2017-02-21 19:03:32 UTC) #81
Avi (ping after 24h)
render process impl lgtm
5 months, 3 weeks ago (2017-02-21 20:32:34 UTC) #84
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/400001
5 months, 3 weeks ago (2017-02-21 22:51:59 UTC) #89
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
5 months, 3 weeks ago (2017-02-22 00:54:43 UTC) #91
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/400001
5 months, 3 weeks ago (2017-02-22 12:59:14 UTC) #93
commit-bot: I haz the power
Committed patchset #20 (id:400001) as https://chromium.googlesource.com/chromium/src/+/ee0c03ddb4a10c8c9d14244bdefc8034054e846e
5 months, 3 weeks ago (2017-02-22 14:03:44 UTC) #96
fdoray
Revert: https://codereview.chromium.org/2710713004/ Didn't use the "Revert Patchset" button because: Revert failed: File too large. This ...
5 months, 3 weeks ago (2017-02-22 14:55:00 UTC) #97
Roger Tawa OOO till Jul 10th
On 2017/02/22 14:55:00, fdoray wrote: > Revert: https://codereview.chromium.org/2710713004/ > > Didn't use the "Revert Patchset" ...
5 months, 3 weeks ago (2017-02-22 20:07:51 UTC) #100
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/460001
5 months, 3 weeks ago (2017-02-22 20:09:45 UTC) #103
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/371522)
5 months, 3 weeks ago (2017-02-22 23:36:13 UTC) #105
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581353002/420002
5 months, 3 weeks ago (2017-02-23 14:46:32 UTC) #112
commit-bot: I haz the power
Committed patchset #24 (id:420002) as https://chromium.googlesource.com/chromium/src/+/57aed38568117ea2787c4b7aa7e24ce27934c7e8
5 months, 3 weeks ago (2017-02-23 14:51:32 UTC) #115
S. Ganesh
https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc File base/win/win_util.cc (right): https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc#newcode502 base/win/win_util.cc:502: HMODULE mdm_dll = ::LoadLibrary(L"MDMRegistration.dll"); I noticed here that the ...
5 months, 2 weeks ago (2017-03-01 03:04:01 UTC) #117
Roger Tawa OOO till Jul 10th
On 2017/03/01 03:04:01, S. Ganesh wrote: > https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc > File base/win/win_util.cc (right): > > https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc#newcode502 ...
5 months, 2 weeks ago (2017-03-01 13:29:12 UTC) #118
S. Ganesh
On 2017/03/01 13:29:12, Roger Tawa wrote: > On 2017/03/01 03:04:01, S. Ganesh wrote: > > ...
5 months, 2 weeks ago (2017-03-01 17:21:22 UTC) #119
Roger Tawa OOO till Jul 10th
On 2017/03/01 17:21:22, S. Ganesh wrote: > On 2017/03/01 13:29:12, Roger Tawa wrote: > > ...
5 months, 2 weeks ago (2017-03-01 17:52:25 UTC) #120
Will Harris (OOO)
5 months, 2 weeks ago (2017-03-01 18:01:09 UTC) #121
Message was sent while issue was closed.
https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc
File base/win/win_util.cc (right):

https://codereview.chromium.org/2581353002/diff/420002/base/win/win_util.cc#n...
base/win/win_util.cc:502: HMODULE mdm_dll =
::LoadLibrary(L"MDMRegistration.dll");
On 2017/03/01 03:04:01, S. Ganesh wrote:
> I noticed here that the DLL is not being freed. Intentional?

perhaps just use base::ScopedNativeLibrary here
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b