Don't check for chrome.exe when initializing crash reporting

Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
Committed: https://crrev.com/d3b1599cda75c5ff8793d0eef641d36b47b3528f
Cr-Commit-Position: refs/heads/master@{#414861}

[scottmg, 2016-08-25 23:39:56]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 23:40:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-25 23:40:50]

Description was changed from

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
==========

to

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
==========

[scottmg, 2016-08-25 23:40:50]

scottmg@chromium.org changed reviewers:
+ bcwhite@chromium.org

[ananta, 2016-08-25 23:42:40]

lgtm

[bcwhite, 2016-08-25 23:43:11]

lgtm

[scottmg, 2016-08-25 23:43:55]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 23:44:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 01:02:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 01:02:59]

Dry run: 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_...)

[robertshield, 2016-08-26 02:04:16]

lgtm

[scottmg, 2016-08-26 05:06:06]

Hmm, looks like chrome_elf_unittests.exe is depending on chrome_elf.dll (and so
trying to initialize crash handling).

[scottmg, 2016-08-26 15:12:00]

On 2016/08/26 05:06:06, scottmg wrote:
> Hmm, looks like chrome_elf_unittests.exe is depending on chrome_elf.dll (and
so
> trying to initialize crash handling).

I haven't been able to reproduce what's happening on the bot locally though. No
imports of chrome_elf into the unittests binary, and no spurious relaunch of
chrome as handler. Occassionally I get a flaky failure of

[ RUN      ] ELFImportsTest.ChromeExeSanityCheck
Backtrace:
        (No symbol) [0x61E85018]
        MapViewOfFileExNuma [0x75C0382C+140]
        MapViewOfFile [0x75C0378D+29]
        base::MemoryMappedFile::MapFileRegionToMemory [0x01364F18+360]
(d:\src\cr3\src\base\files\memory_mapped_file_win.cc:86)
        base::MemoryMappedFile::Initialize [0x01365087+87]
(d:\src\cr3\src\base\files\memory_mapped_file.cc:56)
        `anonymous namespace'::ELFImportsTest::GetImports [0x01360B98+73]
(d:\src\cr3\src\chrome_elf\elf_imports_unittest.cc:42)
        `anonymous
namespace'::ELFImportsTest_ChromeExeSanityCheck_Test::TestBody [0x01360454+141]
(d:\src\cr3\src\chrome_elf\elf_imports_unittest.cc:133)
       
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
[0x013B231E+32] (d:\src\cr3\src\testing\gtest\src\gtest.cc:2460)
        testing::Test::Run [0x013B91CD+93]
(d:\src\cr3\src\testing\gtest\src\gtest.cc:2474)
        testing::TestInfo::Run [0x013B934E+126]
(d:\src\cr3\src\testing\gtest\src\gtest.cc:2660)
        testing::TestCase::Run [0x013B9279+133]
(d:\src\cr3\src\testing\gtest\src\gtest.cc:2773)
        testing::internal::UnitTestImpl::RunAllTests [0x013B95F8+433]
(d:\src\cr3\src\testing\gtest\src\gtest.cc:4645)
       
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>
[0x013B2362+32] (d:\src\cr3\src\testing\gtest\src\gtest.cc:2460)
        testing::UnitTest::Run [0x013B9422+133]
(d:\src\cr3\src\testing\gtest\src\gtest.cc:4255)
        base::TestSuite::Run [0x013A8371+95]
(d:\src\cr3\src\base\test\test_suite.cc:246)
        base::`anonymous namespace'::LaunchUnitTestsInternal [0x013A6A49+659]
(d:\src\cr3\src\base\test\launcher\unit_test_launcher.cc:206)
        base::LaunchUnitTests [0x013A67A2+74]
(d:\src\cr3\src\base\test\launcher\unit_test_launcher.cc:445)
        main [0x01360FC4+81] (d:\src\cr3\src\chrome_elf\run_all_unittests.cc:19)
        __scrt_common_main_seh [0x01415E02+249]
(f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
        BaseThreadInitThunk [0x75DE62C4+36]
        RtlSubscribeWnfStateChangeNotification [0x77720609+1081]
        RtlSubscribeWnfStateChangeNotification [0x777205D4+1028]

but it looks unrelated and just in the test code I think (?)

[scottmg, 2016-08-26 15:55:02]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 15:55:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-26 15:56:09]

On 2016/08/26 15:12:00, scottmg wrote:
> On 2016/08/26 05:06:06, scottmg wrote:
> > Hmm, looks like chrome_elf_unittests.exe is depending on chrome_elf.dll (and
> so
> > trying to initialize crash handling).
> 
> I haven't been able to reproduce what's happening on the bot locally though.
No
> imports of chrome_elf into the unittests binary, and no spurious relaunch of
> chrome as handler. Occassionally I get a flaky failure of
> 
> [ RUN      ] ELFImportsTest.ChromeExeSanityCheck
> Backtrace:
>         (No symbol) [0x61E85018]
>         MapViewOfFileExNuma [0x75C0382C+140]
>         MapViewOfFile [0x75C0378D+29]
>         base::MemoryMappedFile::MapFileRegionToMemory [0x01364F18+360]
> (d:\src\cr3\src\base\files\memory_mapped_file_win.cc:86)
>         base::MemoryMappedFile::Initialize [0x01365087+87]
> (d:\src\cr3\src\base\files\memory_mapped_file.cc:56)
>         `anonymous namespace'::ELFImportsTest::GetImports [0x01360B98+73]
> (d:\src\cr3\src\chrome_elf\elf_imports_unittest.cc:42)
>         `anonymous
> namespace'::ELFImportsTest_ChromeExeSanityCheck_Test::TestBody
[0x01360454+141]
> (d:\src\cr3\src\chrome_elf\elf_imports_unittest.cc:133)
>        
> testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
> [0x013B231E+32] (d:\src\cr3\src\testing\gtest\src\gtest.cc:2460)
>         testing::Test::Run [0x013B91CD+93]
> (d:\src\cr3\src\testing\gtest\src\gtest.cc:2474)
>         testing::TestInfo::Run [0x013B934E+126]
> (d:\src\cr3\src\testing\gtest\src\gtest.cc:2660)
>         testing::TestCase::Run [0x013B9279+133]
> (d:\src\cr3\src\testing\gtest\src\gtest.cc:2773)
>         testing::internal::UnitTestImpl::RunAllTests [0x013B95F8+433]
> (d:\src\cr3\src\testing\gtest\src\gtest.cc:4645)
>        
>
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>
> [0x013B2362+32] (d:\src\cr3\src\testing\gtest\src\gtest.cc:2460)
>         testing::UnitTest::Run [0x013B9422+133]
> (d:\src\cr3\src\testing\gtest\src\gtest.cc:4255)
>         base::TestSuite::Run [0x013A8371+95]
> (d:\src\cr3\src\base\test\test_suite.cc:246)
>         base::`anonymous namespace'::LaunchUnitTestsInternal [0x013A6A49+659]
> (d:\src\cr3\src\base\test\launcher\unit_test_launcher.cc:206)
>         base::LaunchUnitTests [0x013A67A2+74]
> (d:\src\cr3\src\base\test\launcher\unit_test_launcher.cc:445)
>         main [0x01360FC4+81]
(d:\src\cr3\src\chrome_elf\run_all_unittests.cc:19)
>         __scrt_common_main_seh [0x01415E02+249]
> (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
>         BaseThreadInitThunk [0x75DE62C4+36]
>         RtlSubscribeWnfStateChangeNotification [0x77720609+1081]
>         RtlSubscribeWnfStateChangeNotification [0x777205D4+1028]
> 
> but it looks unrelated and just in the test code I think (?)

Oh, got it. ChromeElfLoadSanityTest force loads chrome_elf.dll into
chrome_elf_unittests.exe, which causes it to run chrome_elf_unittests
--type=crashpad-handler. The test launcher ignores that and so runs all the
tests again. (It doesn't forkbomb because the second time around it has
--type=crashpad-handler on the command line, so it doesn't reinitialize).

I think the failing test above is because there's probably two processes trying
to map that binary at the same time (maybe, not sure about that). I think the
only reason the bots fail is that the sub-crashpad-handler process is trying to
access c:\users\... to create a crashpad database, so all very flukey.

Anyway, I think relaunching with that flag before running the test is probably
an OK workaround for this one case, since we're just trying to confirm that
user32 is not in the import table.

Please take another look to see how you feel about the test.

[commit-bot: I haz the power, 2016-08-26 16:55:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 16:55:34]

Dry run: 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_...)

[scottmg, 2016-08-26 17:06:26]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 17:06:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-26 18:23:53]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 18:24:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 19:31:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 19:31:09]

Dry run: 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_...)

[scottmg, 2016-08-26 19:51:57]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 19:53:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-26 19:53:25]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 19:54:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 21:11:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 21:11:27]

Dry run: 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_...)

[scottmg, 2016-08-26 21:46:13]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 21:47:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 22:40:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 22:40:33]

Dry run: This issue passed the CQ dry run.

[scottmg, 2016-08-26 22:52:12]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-08-26 22:52:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
bcwhite@chromium.org, ananta@chromium.org, robertshield@chromium.org
Link to the patchset: https://codereview.chromium.org/2279943002/#ps180001
(title: ".")

[commit-bot: I haz the power, 2016-08-26 22:53:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 23:48:08]

Description was changed from

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
==========

to

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
==========

[commit-bot: I haz the power, 2016-08-26 23:48:10]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-08-26 23:51:19]

Description was changed from

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664
==========

to

==========
Don't check for chrome.exe when initializing crash reporting

This was added so that chrome_elf wouldn't try to initialize crash
reporting when loaded in browser_tests, etc. But subsequently the
dependencies have been cleaned up, and browser_tests doesn't link
chrome_elf, so shouldn't be necessary any more.

Also messes up the obscure case when people rename chrome.exe.

R=robertshield@chromium.org, ananta@chromium.org
BUG=604923, crashpad:106, 568664

Committed: https://crrev.com/d3b1599cda75c5ff8793d0eef641d36b47b3528f
Cr-Commit-Position: refs/heads/master@{#414861}
==========

[commit-bot: I haz the power, 2016-08-26 23:51:21]

Patchset 10 (id:??) landed as
https://crrev.com/d3b1599cda75c5ff8793d0eef641d36b47b3528f
Cr-Commit-Position: refs/heads/master@{#414861}

[robertshield, 2016-08-27 03:39:36]

sorry for the radio silence, was out today, nice sleuthing and here's a belated
still lgtm.