Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(241)

Issue 2431483004: Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain (Closed)

Created:
4 years, 2 months ago by scottmg
Modified:
4 years, 2 months ago
Reviewers:
penny, ananta, Will Harris
CC:
chromium-reviews
Target Ref:
refs/pending/branch-heads/2840
Project:
chromium
Visibility:
Public.

Description

Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain Currently, initialization crash reporting indirectly calls CreateProcess() from chrome_elf's DllMain(), which causes deadlock in some configurations (showing up in M54). Instead, signal to chrome_elf that it should initialize crash reporting later in WinMain(). This means we lose a little coverage that we were hoping to gain from the move to chrome_elf, but a localized fix is required for merge to stable. Follow up bug is https://crbug.com/656800. BUG=655788, 656800 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng TEST=enforce certificate rules per https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to launch chrome. Review-Url: https://codereview.chromium.org/2428703002 Cr-Commit-Position: refs/heads/master@{#425840} (cherry picked from commit f3a5670dd8d42b045d31625dde4a2561b471138f) Committed: https://chromium.googlesource.com/chromium/src/+/0aa889d7b478450c71133081216ccefbd645ba30

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -5 lines) Patch
M chrome/app/chrome_exe_main_win.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome_elf/chrome_elf.def View 1 chunk +1 line, -0 lines 0 comments Download
M chrome_elf/chrome_elf_main.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome_elf/chrome_elf_main.cc View 2 chunks +7 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (3 generated)
scottmg
Apparently this function has been renamed since 54 :( InitializeCrashReportingForProcess -> elf_crash::InitializeCrashReporting. Hopefully the sameish.
4 years, 2 months ago (2016-10-18 23:10:49 UTC) #3
scottmg
4 years, 2 months ago (2016-10-18 23:18:18 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
0aa889d7b478450c71133081216ccefbd645ba30 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698