Issue 289373006: Revert 271416 "Store a stacktrace of PrefService destruction in ..."

Issue 289373006: Revert 271416 "Store a stacktrace of PrefService destruction in ..." (Closed)

Created:
6 years, 7 months ago by Alexei Svitkine (slow)

Modified:
6 years, 7 months ago

Reviewers:
battre

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/

Visibility:
Public.

More Reviews

Description

Revert 271416 "Store a stacktrace of PrefService destruction in ..." Speculative revert, suspecting this CL resulted in timeouts of the following tests on ChromiumOS: ExtensionUpdaterTest.TestGalleryRequestsWithNonOrganicBrand ExtensionUpdaterTest.TestGalleryRequestsWithOrganicBrand Test run time went from ~2s per test to >40s per test (and higher) causing timeouts. > Store a stacktrace of PrefService destruction in PrefChangeRegistrar > > Store a stacktrace of PrefService destruction in PrefChangeRegistrar to debug a > race condition: ~PrefChangeRegistrar causes a crash. The current hypothesis is > that this is caused because the PrefChangeRegistrar is registered to a Profile > that is already destroyed. This code stores stacktrace when the Profile's > PrefService is destroyed to see whether this is correct. > > BUG=373435 > > Review URL: https://codereview.chromium.org/290083006 TBR=battre@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271622

Patch Set 1 #

Created: 6 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+0 lines, -91 lines)			Patch
M	trunk/src/base/prefs/pref_change_registrar.h	View	2 chunks	+0 lines, -5 lines	0 comments	Download
M	trunk/src/base/prefs/pref_change_registrar.cc	View	3 chunks	+0 lines, -21 lines	0 comments	Download
M	trunk/src/base/prefs/pref_member.h	View	2 chunks	+0 lines, -5 lines	0 comments	Download
M	trunk/src/base/prefs/pref_member.cc	View	3 chunks	+0 lines, -20 lines	0 comments	Download
M	trunk/src/base/prefs/pref_notifier.h	View	1 chunk	+0 lines, -6 lines	0 comments	Download
M	trunk/src/base/prefs/pref_notifier_impl.h	View	1 chunk	+0 lines, -3 lines	0 comments	Download
M	trunk/src/base/prefs/pref_notifier_impl.cc	View	1 chunk	+0 lines, -13 lines	0 comments	Download
M	trunk/src/base/prefs/pref_notifier_impl_unittest.cc	View	1 chunk	+0 lines, -3 lines	0 comments	Download
M	trunk/src/base/prefs/pref_observer.h	View	1 chunk	+0 lines, -4 lines	0 comments	Download
M	trunk/src/base/prefs/pref_service.cc	View	2 chunks	+0 lines, -9 lines	0 comments	Download
M	trunk/src/base/prefs/pref_value_store_unittest.cc	View	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Alexei Svitkine (slow)

Committed patchset #1 manually as r271622 (tree was closed).

6 years, 7 months ago (2014-05-20 11:25:15 UTC) #2

Alexei Svitkine (slow)

Example test output: http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/30632 builder: http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29

6 years, 7 months ago (2014-05-20 11:25:42 UTC) #3

Alexei Svitkine (slow)

6 years, 7 months ago (2014-05-20 13:53:49 UTC) #4

Reverting did seem to help - the test times went back to the ~2s range on
that bot (from 40s-70s with this CL).

On Tue, May 20, 2014 at 1:25 PM, <asvitkine@chromium.org> wrote:

> Example test output:
>
> http://build.chromium.org/p/chromium.chromiumos/builders/
> Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/30632
>
> builder:
>
> http://build.chromium.org/p/chromium.chromiumos/builders/
> Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29
>
> https://codereview.chromium.org/289373006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Expand Messages | Collapse Messages