SiteInstanceTests failing under Dr. Memory |
|||||
Issue descriptionSiteInstanceTest.* are failing on both the Dr. Memory light and full bots. https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/5150 https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/10611
,
Jun 23 2016
That's concerning. These aren't SiteIsolation tests-- these have been around since Chrome launched. There's probably a CL in build #5150 that caused them to regress. Tests: SiteInstanceTest.CloneNavigationEntry SiteInstanceTest.DefaultSubframeSiteInstance SiteInstanceTest.GetProcess SiteInstanceTest.GetSiteForURL SiteInstanceTest.HasWrongProcessForURL SiteInstanceTest.HasWrongProcessForURLInSitePerProcess SiteInstanceTest.IsSameWebSite SiteInstanceTest.NoProcessPerSiteForEmptySite SiteInstanceTest.OneSiteInstancePerSite SiteInstanceTest.OneSiteInstancePerSiteInBrowserContext SiteInstanceTest.ProcessPerSiteWithWrongBindings SiteInstanceTest.ProcessSharingByType SiteInstanceTest.SetSite SiteInstanceTest.SiteInstanceDestructor
,
Jun 23 2016
That call is in SiteInstanceTest::TearDown, which explains why they're all failing the same way. Something must be causing a RenderProcessHost to stick around after the test is over. Could someone run a bisect? I'm going to be out for a bit.
,
Jun 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e6872f8f8b89bb49bda37577c7e0800ebee2488 commit 8e6872f8f8b89bb49bda37577c7e0800ebee2488 Author: Reilly Grant <reillyg@chromium.org> Date: Thu Jun 23 18:46:00 2016 Disable SiteInstanceTest.* under Dr. Memory. They are failing. BUG= 622793 TBR=nick@chromium.org Review URL: https://codereview.chromium.org/2090393002 . Cr-Commit-Position: refs/heads/master@{#401667} [modify] https://crrev.com/8e6872f8f8b89bb49bda37577c7e0800ebee2488/tools/valgrind/gtest_exclude/content_unittests.gtest-drmemory_win32.txt
,
Jun 23 2016
Disabling these tests isn't ok. There's a CL that should be reverted if it's causing these tests to fail. Is there a way to run a bisect on Dr. Memory so we can find out which CL caused this?
,
Jun 23 2016
These tests are only disabled under Dr. Memory, which is standard procedure when a Dr. Memory-only issue is found. I am working on a bisect now.
,
Jun 23 2016
Ok, thanks. Much appreciated!
,
Jun 23 2016
thestig@ reminded me of something I forgot about which is that the biggest difference between the normal bots and the Dr. Memory bots is that the tests don't run in parallel so one test leaving behind some state can mess up another test hundreds of tests later. Hopefully this should speed up the process of bisecting this.
,
Jun 23 2016
After running a bisect I found https://codereview.chromium.org/2045363002/ to be at fault. Reverting.
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d commit 7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d Author: reillyg <reillyg@chromium.org> Date: Fri Jun 24 01:25:01 2016 Revert of Routing IME Result Calls to the Correct RenderWidgetHost (Aura Only) (patchset #8 id:140001 of https://codereview.chromium.org/2045363002/ ) Reason for revert: This patch added unit tests that leave behind state that will break other tests if content_unittests is run with --single-process-tests (as it is on the Dr. Memory bots). Original issue's description: > Routing IME Result Calls to the Correct RenderWidgetHost (Aura Only) > > Currently, the IME result calls on ui::TextInputClient (RenderWidgetHostViewAura) are only > routed to the RenderWidgetHostImpl corresponding to the tab's RenderWidgetHostView. Therefore, > the IME related IPC arrive at the main frame's RenderWidget only. In the OOPIF structure, > the IME result might be intended to a child frame's RenderWidget. > > This patch will route all such IME calls to the RenderWidgetHost which currently has the > active text input state. > > The patch also adds four unit tests to RenderWidgetHostViewAuraTests to verify that the > routing is done properly. > > BUG= 578168 > > Committed: https://crrev.com/d33fd7859cac797067a2b8a8e55f6dbac59bee19 > Cr-Commit-Position: refs/heads/master@{#401465} TBR=kenrb@chromium.org,creis@chromium.org,ekaramad@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 578168 , 622793 Review-Url: https://codereview.chromium.org/2095813002 Cr-Commit-Position: refs/heads/master@{#401771} [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/browser/renderer_host/render_widget_host_view_aura.cc [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/browser/renderer_host/render_widget_host_view_aura.h [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/browser/renderer_host/text_input_manager.cc [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/browser/renderer_host/text_input_manager.h [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/public/test/text_input_test_utils.cc [modify] https://crrev.com/7225ddf9fd7c32a1de91a9f2c8b755d736b2fd0d/content/test/test_render_view_host.h
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31189d3191c823005b7d09f980953195ccf66765 commit 31189d3191c823005b7d09f980953195ccf66765 Author: reillyg <reillyg@chromium.org> Date: Fri Jun 24 01:26:53 2016 Revert of Disable SiteInstanceTest.* under Dr. Memory. (patchset #1 id:1 of https://codereview.chromium.org/2090393002/ ) Reason for revert: Reverted https://codereview.chromium.org/2045363002/. Original issue's description: > Disable SiteInstanceTest.* under Dr. Memory. > > They are failing. > > BUG= 622793 > TBR=nick@chromium.org > > Committed: https://chromium.googlesource.com/chromium/src/+/8e6872f8f8b89bb49bda37577c7e0800ebee2488 TBR=nick@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 622793 Review-Url: https://codereview.chromium.org/2099573002 Cr-Commit-Position: refs/heads/master@{#401772} [modify] https://crrev.com/31189d3191c823005b7d09f980953195ccf66765/tools/valgrind/gtest_exclude/content_unittests.gtest-drmemory_win32.txt
,
Jun 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8cba7886c3e170ca2dc324dbbd31b28dae623887 commit 8cba7886c3e170ca2dc324dbbd31b28dae623887 Author: ekaramad <ekaramad@chromium.org> Date: Fri Jun 24 19:42:31 2016 [reland] Routing IME Result Calls to the Correct RenderWidgetHost (Aura Only) This is a reland of the original CL: https://codereview.chromium.org/2045363002/ which was reverted by this CL https://codereview.chromium.org/2095813002. The reason for the revert was the regression of several SiteInstanceTests on Dr Memory bots. The cause of the issue was that the newly introduced content unit tests in the original CL were not deleting a RenderWidgetHostImpl which was keeping a RenderProcessHost alive. This was interfering with the SiteInstanceTests which have explicit asserts on the number of RenderProceessHosts alive. BUG= 578168 , 622793 Review-Url: https://codereview.chromium.org/2092103002 Cr-Commit-Position: refs/heads/master@{#401939} [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/browser/renderer_host/render_widget_host_view_aura.cc [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/browser/renderer_host/render_widget_host_view_aura.h [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/browser/renderer_host/text_input_manager.cc [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/browser/renderer_host/text_input_manager.h [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/public/test/text_input_test_utils.cc [modify] https://crrev.com/8cba7886c3e170ca2dc324dbbd31b28dae623887/content/test/test_render_view_host.h
,
Jun 30 2016
Marking bug as fixed since the re-landed patch has not regressed the tests in almost a week. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by reillyg@chromium.org
, Jun 23 2016