Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2514483002:
Add a crash key to record whether a windows machine is domain joined. (Closed)
|
Created:
4 years, 1 month ago by Georges Khalil Modified:
4 years ago CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd a crash key to record whether a windows machine is domain joined.
BUG=660868
Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4
Committed: https://crrev.com/51cd41252a2c76b57cc298e8284cead2102ab9ca
Cr-Original-Commit-Position: refs/heads/master@{#435018}
Cr-Commit-Position: refs/heads/master@{#436966}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Naming change. #Patch Set 3 : Register crash key. #Patch Set 4 : Add key to 2 other places. #
Total comments: 2
Patch Set 5 : sky@ comments. #Patch Set 6 : Merge ToT. #
Total comments: 2
Patch Set 7 : sky@ comments. #
Messages
Total messages: 78 (51 generated)
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
georgesak@chromium.org changed reviewers: + siggi@chromium.org
PTAL.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Add crash keys for brand code and domain joined. BUG=660868 ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 ==========
Patchset #1 (id:1) has been deleted
lgtm
georgesak@chromium.org changed reviewers: + sky@chromium.org
+sky@, for owner review.
LGTM if my naming suggestion doesn't make sense. https://codereview.chromium.org/2514483002/diff/20001/chrome/common/crash_keys.h File chrome/common/crash_keys.h (right): https://codereview.chromium.org/2514483002/diff/20001/chrome/common/crash_key... chrome/common/crash_keys.h:99: extern const char kDomainJoined[]; 'domain' is pretty obscure. Would kEnrolledInEnterprisePolicy make more sense?
Thanks, sending to CQ. https://codereview.chromium.org/2514483002/diff/20001/chrome/common/crash_keys.h File chrome/common/crash_keys.h (right): https://codereview.chromium.org/2514483002/diff/20001/chrome/common/crash_key... chrome/common/crash_keys.h:99: extern const char kDomainJoined[]; On 2016/11/17 20:34:18, sky wrote: > 'domain' is pretty obscure. Would kEnrolledInEnterprisePolicy make more sense? This is really to know whether the machine is part of a Windows domain group, not necessarily if it has policies (which can come from other sources). I changed the name to kEnrolledToDomain to match the function name.
The CQ bit was checked by georgesak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, siggi@chromium.org Link to the patchset: https://codereview.chromium.org/2514483002/#ps40001 (title: "Naming change.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/11/17 23:28:04, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) Hey Georges, were you planning to land this? Siggi
The CQ bit was checked by georgesak@chromium.org
Oops, never saw that it failed. Looks like flake, resending to CQ. On Mon, Nov 28, 2016 at 4:52 PM, <siggi@chromium.org> wrote: > On 2016/11/17 23:28:04, commit-bot: I haz the power wrote: > > Try jobs failed on following builders: > > linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux > (JOB_FAILED, > > > http://build.chromium.org/p/tryserver.chromium.linux/ > builders/linux_chromium_chromeos_rel_ng/builds/317405) > > Hey Georges, > > were you planning to land this? > > Siggi > > https://codereview.chromium.org/2514483002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by georgesak@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by georgesak@chromium.org
On 2016/11/29 16:24:30, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... I forgot to register the crash key, hence the problem. Fixed, resubmitting (hopefully for the last time!).
The CQ bit was checked by georgesak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, siggi@chromium.org Link to the patchset: https://codereview.chromium.org/2514483002/#ps60001 (title: "Register crash key.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480437547782900,
"parent_rev": "c214d6e8a2dfce26b352192a095ffdd28c34199b", "commit_rev":
"124042190216dad479e12e2a0c2300b10c6a1a7d"}
Message was sent while issue was closed.
Description was changed from ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:60001) has been created in https://codereview.chromium.org/2536293002/ by iclelland@chromium.org. The reason for reverting is: Reverting this; it appears to be causing 140+ failures in telmetry tests on Win7 (See the failures in https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%... ) As far as I can tell (the log output is being suppressed,) it looks it's hitting the DCHECK in SetCrashKeyValue that would indicate that the key isn't registered as expected. Also filed https://bugs.chromium.org/p/chromium/issues/detail?id=669633 .
Message was sent while issue was closed.
Description was changed from ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ==========
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:80001) has been deleted
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/11/29 20:25:27, iclelland wrote: > A revert of this CL (patchset #3 id:60001) has been created in > https://codereview.chromium.org/2536293002/ by mailto:iclelland@chromium.org. > > The reason for reverting is: Reverting this; it appears to be causing 140+ > failures in telmetry tests on Win7 (See the failures in > https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%... > ) > > As far as I can tell (the log output is being suppressed,) it looks it's hitting > the DCHECK in SetCrashKeyValue that would indicate that the key isn't registered > as expected. > > Also filed https://bugs.chromium.org/p/chromium/issues/detail?id=669633 > . Turns out the key has to be added to 2 other places as well.
georgesak@chromium.org changed reviewers: + rsesek@chromium.org
sky@: PTAnL for sanity check? rsesek@: PTAL for owner review for blimp/engine/app/blimp_engine_crash_keys.cc
lgtm
https://codereview.chromium.org/2514483002/diff/100001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2514483002/diff/100001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:993: // Record whether the machine is domain joined (Win only) in a crash key. This Can this be moved to chrome_browser_main_win?
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
Patchset #5 (id:120001) has been deleted
Patchset #5 (id:140001) has been deleted
Patchset #5 (id:160001) has been deleted
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Patchset #7 (id:220001) has been deleted
Patchset #6 (id:200001) has been deleted
The CQ bit was checked by georgesak@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
sky@, PTAnL. https://codereview.chromium.org/2514483002/diff/100001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/2514483002/diff/100001/chrome/browser/chrome_... chrome/browser/chrome_browser_main.cc:993: // Record whether the machine is domain joined (Win only) in a crash key. This On 2016/12/05 21:59:49, sky wrote: > Can this be moved to chrome_browser_main_win? Done.
LGTM https://codereview.chromium.org/2514483002/diff/240001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2514483002/diff/240001/chrome/browser/chrome_... chrome/browser/chrome_browser_main_win.cc:295: #if defined(OS_WIN) These ifdefs should no longer be necessary (this file is only compiled on windows).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks, sending to CQ. https://codereview.chromium.org/2514483002/diff/240001/chrome/browser/chrome_... File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2514483002/diff/240001/chrome/browser/chrome_... chrome/browser/chrome_browser_main_win.cc:295: #if defined(OS_WIN) On 2016/12/06 23:23:43, sky wrote: > These ifdefs should no longer be necessary (this file is only compiled on > windows). Oops, removed.
The CQ bit was checked by georgesak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from siggi@chromium.org, rsesek@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/2514483002/#ps260001 (title: "sky@ comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 260001, "attempt_start_ts": 1481122828492500,
"parent_rev": "b5e763e9085860300575ffc736fd62c10c19a406", "commit_rev":
"8a31a33ec6f65ec98a87b1492f38d56596861a24"}
Message was sent while issue was closed.
Description was changed from ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ==========
Message was sent while issue was closed.
Committed patchset #7 (id:260001)
Message was sent while issue was closed.
Description was changed from ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Cr-Commit-Position: refs/heads/master@{#435018} ========== to ========== Add a crash key to record whether a windows machine is domain joined. BUG=660868 Committed: https://crrev.com/bf33256aee8db58c30b9d36e991055f2a4ed62a4 Committed: https://crrev.com/51cd41252a2c76b57cc298e8284cead2102ab9ca Cr-Original-Commit-Position: refs/heads/master@{#435018} Cr-Commit-Position: refs/heads/master@{#436966} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/51cd41252a2c76b57cc298e8284cead2102ab9ca Cr-Commit-Position: refs/heads/master@{#436966} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
