Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2244733002:
Settings Sync: Fix CloseUI logic in both Settings and MD Settings. (Closed)
|
Created:
4 years, 4 months ago by tommycli Modified:
4 years, 4 months ago CC:
chromium-reviews, dbeam+watch-options_chromium.org, michaelpg+watch-md-settings_chromium.org, dbeam+watch-settings_chromium.org, michaelpg+watch-options_chromium.org, stevenjb+watch-md-settings_chromium.org, Dan Beam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSettings Sync: Fix CloseUI logic in both Settings and MD Settings.
Previously, the Sync Settings overlay was only properly marked closed if
it was the last-opened instance of Sync Settings.
This patch fixes that.
BUG=636693
Committed: https://crrev.com/0e9a88ea078bd8dbb4f095f8fbc7699716b30f63
Cr-Commit-Position: refs/heads/master@{#412020}
Patch Set 1 #Patch Set 2 : fix guest mode crash #
Messages
Total messages: 29 (11 generated)
tommycli@chromium.org changed reviewers: + stevenjb@chromium.org
stevenjb: PTAL, thanks!
lgtm
The CQ bit was checked by tommycli@chromium.org
On 2016/08/12 23:27:35, stevenjb wrote: > lgtm Thanks!
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_...)
The CQ bit was checked by tommycli@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 checked by tommycli@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 tommycli@chromium.org
The CQ bit was checked by tommycli@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stevenjb@chromium.org Link to the patchset: https://codereview.chromium.org/2244733002/#ps20001 (title: "fix guest mode crash")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Settings Sync: Fix CloseUI logic in both Settings and MD Settings. Previously, the Sync Settings overlay was only properly marked closed if it was the last-opened instance of Sync Settings. This patch fixes that. BUG=636693 ========== to ========== Settings Sync: Fix CloseUI logic in both Settings and MD Settings. Previously, the Sync Settings overlay was only properly marked closed if it was the last-opened instance of Sync Settings. This patch fixes that. BUG=636693 Committed: https://crrev.com/0e9a88ea078bd8dbb4f095f8fbc7699716b30f63 Cr-Commit-Position: refs/heads/master@{#412020} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/0e9a88ea078bd8dbb4f095f8fbc7699716b30f63 Cr-Commit-Position: refs/heads/master@{#412020}
Message was sent while issue was closed.
michaelpg@chromium.org changed reviewers: + michaelpg@chromium.org
Message was sent while issue was closed.
stevenjb/dbeam: why do we have no stack trace or error messages for these crashes (seg faults presumably)? https://codereview.chromium.org/2244733002/#ps1
Message was sent while issue was closed.
On 2016/08/15 19:41:51, michaelpg wrote: > stevenjb/dbeam: why do we have no stack trace or error messages for these > crashes (seg faults presumably)? https://codereview.chromium.org/2244733002/#ps1 What crashes? That link just points to this CL.
Message was sent while issue was closed.
On 2016/08/15 19:56:09, stevenjb wrote: > On 2016/08/15 19:41:51, michaelpg wrote: > > stevenjb/dbeam: why do we have no stack trace or error messages for these > > crashes (seg faults presumably)? > https://codereview.chromium.org/2244733002/#ps1 > > What crashes? That link just points to this CL. The browsertest failures here: https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium... They failed due to a segfault, not a test expectation failure.
Message was sent while issue was closed.
I don't think we maintain the tools required to produce symbolic crash dumps on trybots, but I could be wrong. Generally I would try to reproduce this locally. On Mon, Aug 15, 2016 at 2:06 PM, <tommycli@chromium.org> wrote: > On 2016/08/15 19:56:09, stevenjb wrote: > > On 2016/08/15 19:41:51, michaelpg wrote: > > > stevenjb/dbeam: why do we have no stack trace or error messages for > these > > > crashes (seg faults presumably)? > > https://codereview.chromium.org/2244733002/#ps1 > > > > What crashes? That link just points to this CL. > > The browsertest failures here: > > https://build.chromium.org/p/tryserver.chromium.linux/ > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > They failed due to a segfault, not a test expectation failure. > > https://codereview.chromium.org/2244733002/ > -- 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.
Message was sent while issue was closed.
On 2016/08/15 20:08:20, stevenjb wrote: > I don't think we maintain the tools required to produce symbolic crash > dumps on trybots, but I could be wrong. Generally I would try to reproduce > this locally. > > > On Mon, Aug 15, 2016 at 2:06 PM, <mailto:tommycli@chromium.org> wrote: > > > On 2016/08/15 19:56:09, stevenjb wrote: > > > On 2016/08/15 19:41:51, michaelpg wrote: > > > > stevenjb/dbeam: why do we have no stack trace or error messages for > > these > > > > crashes (seg faults presumably)? > > > https://codereview.chromium.org/2244733002/#ps1 > > > > > > What crashes? That link just points to this CL. > > > > The browsertest failures here: > > > > https://build.chromium.org/p/tryserver.chromium.linux/ > > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > > > They failed due to a segfault, not a test expectation failure. > > > > https://codereview.chromium.org/2244733002/ > > > > -- > 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 mailto:chromium-reviews+unsubscribe@chromium.org. It was odd, but even locally I didn't get a segfault message or stacktrace. I eventually figured out what was going on, but was confused for a while.
Message was sent while issue was closed.
Were you running / able to produce with a Debug build? I've seen release builds crash with no stack info. Sometimes you can get a little information if you're attached to a debugger at the time, but usually I try reproducing with Debug. On Mon, Aug 15, 2016 at 2:09 PM, <tommycli@chromium.org> wrote: > On 2016/08/15 20:08:20, stevenjb wrote: > > I don't think we maintain the tools required to produce symbolic crash > > dumps on trybots, but I could be wrong. Generally I would try to > reproduce > > this locally. > > > > > > On Mon, Aug 15, 2016 at 2:06 PM, <mailto:tommycli@chromium.org> wrote: > > > > > On 2016/08/15 19:56:09, stevenjb wrote: > > > > On 2016/08/15 19:41:51, michaelpg wrote: > > > > > stevenjb/dbeam: why do we have no stack trace or error messages for > > > these > > > > > crashes (seg faults presumably)? > > > > https://codereview.chromium.org/2244733002/#ps1 > > > > > > > > What crashes? That link just points to this CL. > > > > > > The browsertest failures here: > > > > > > https://build.chromium.org/p/tryserver.chromium.linux/ > > > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > > > > > They failed due to a segfault, not a test expectation failure. > > > > > > https://codereview.chromium.org/2244733002/ > > > > > > > -- > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. > > It was odd, but even locally I didn't get a segfault message or stacktrace. > > I eventually figured out what was going on, but was confused for a while. > > https://codereview.chromium.org/2244733002/ > -- 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.
Message was sent while issue was closed.
On 2016/08/15 20:19:53, stevenjb wrote: > Were you running / able to produce with a Debug build? I've seen release > builds crash with no stack info. Sometimes you can get a little information > if you're attached to a debugger at the time, but usually I try reproducing > with Debug. > > > On Mon, Aug 15, 2016 at 2:09 PM, <mailto:tommycli@chromium.org> wrote: > > > On 2016/08/15 20:08:20, stevenjb wrote: > > > I don't think we maintain the tools required to produce symbolic crash > > > dumps on trybots, but I could be wrong. Generally I would try to > > reproduce > > > this locally. > > > > > > > > > On Mon, Aug 15, 2016 at 2:06 PM, <mailto:tommycli@chromium.org> wrote: > > > > > > > On 2016/08/15 19:56:09, stevenjb wrote: > > > > > On 2016/08/15 19:41:51, michaelpg wrote: > > > > > > stevenjb/dbeam: why do we have no stack trace or error messages for > > > > these > > > > > > crashes (seg faults presumably)? > > > > > https://codereview.chromium.org/2244733002/#ps1 > > > > > > > > > > What crashes? That link just points to this CL. > > > > > > > > The browsertest failures here: > > > > > > > > https://build.chromium.org/p/tryserver.chromium.linux/ > > > > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > > > > > > > They failed due to a segfault, not a test expectation failure. > > > > > > > > https://codereview.chromium.org/2244733002/ > > > > > > > > > > -- > > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. > > > > It was odd, but even locally I didn't get a segfault message or stacktrace. > > > > I eventually figured out what was going on, but was confused for a while. > > > > https://codereview.chromium.org/2244733002/ > > > > -- > 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 mailto:chromium-reviews+unsubscribe@chromium.org. stevenjb: Okay thanks, I will try a Debug build for local repro next time. Thanks for the tip. Tommy
Message was sent while issue was closed.
On 2016/08/15 20:19:53, stevenjb wrote: > Were you running / able to produce with a Debug build? I've seen release > builds crash with no stack info. Sometimes you can get a little information > if you're attached to a debugger at the time, but usually I try reproducing > with Debug. Still a shame, though. Looks like the crash happens in InProcessBrowserTest::QuitBrowsers, maybe that has something to do with it? OTOH, adding a signal handler for SIGSEGV here: https://cs.chromium.org/chromium/src/content/public/test/browser_test_base.cc... works to produce a stack trace. Worth investigating? > > > On Mon, Aug 15, 2016 at 2:09 PM, <mailto:tommycli@chromium.org> wrote: > > > On 2016/08/15 20:08:20, stevenjb wrote: > > > I don't think we maintain the tools required to produce symbolic crash > > > dumps on trybots, but I could be wrong. Generally I would try to > > reproduce > > > this locally. > > > > > > > > > On Mon, Aug 15, 2016 at 2:06 PM, <mailto:tommycli@chromium.org> wrote: > > > > > > > On 2016/08/15 19:56:09, stevenjb wrote: > > > > > On 2016/08/15 19:41:51, michaelpg wrote: > > > > > > stevenjb/dbeam: why do we have no stack trace or error messages for > > > > these > > > > > > crashes (seg faults presumably)? > > > > > https://codereview.chromium.org/2244733002/#ps1 > > > > > > > > > > What crashes? That link just points to this CL. > > > > > > > > The browsertest failures here: > > > > > > > > https://build.chromium.org/p/tryserver.chromium.linux/ > > > > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > > > > > > > They failed due to a segfault, not a test expectation failure. > > > > > > > > https://codereview.chromium.org/2244733002/ > > > > > > > > > > -- > > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. > > > > It was odd, but even locally I didn't get a segfault message or stacktrace. > > > > I eventually figured out what was going on, but was confused for a while. > > > > https://codereview.chromium.org/2244733002/ > > > > -- > 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 mailto:chromium-reviews+unsubscribe@chromium.org.
Message was sent while issue was closed.
On 2016/08/15 20:55:56, michaelpg wrote: > On 2016/08/15 20:19:53, stevenjb wrote: > > Were you running / able to produce with a Debug build? I've seen release > > builds crash with no stack info. Sometimes you can get a little information > > if you're attached to a debugger at the time, but usually I try reproducing > > with Debug. > > Still a shame, though. Looks like the crash happens in > InProcessBrowserTest::QuitBrowsers, maybe that has something to do with it? > > OTOH, adding a signal handler for SIGSEGV here: > https://cs.chromium.org/chromium/src/content/public/test/browser_test_base.cc... > > works to produce a stack trace. Worth investigating? Discussion moved to crbug.com/637942. > > > > > > > On Mon, Aug 15, 2016 at 2:09 PM, <mailto:tommycli@chromium.org> wrote: > > > > > On 2016/08/15 20:08:20, stevenjb wrote: > > > > I don't think we maintain the tools required to produce symbolic crash > > > > dumps on trybots, but I could be wrong. Generally I would try to > > > reproduce > > > > this locally. > > > > > > > > > > > > On Mon, Aug 15, 2016 at 2:06 PM, <mailto:tommycli@chromium.org> wrote: > > > > > > > > > On 2016/08/15 19:56:09, stevenjb wrote: > > > > > > On 2016/08/15 19:41:51, michaelpg wrote: > > > > > > > stevenjb/dbeam: why do we have no stack trace or error messages for > > > > > these > > > > > > > crashes (seg faults presumably)? > > > > > > https://codereview.chromium.org/2244733002/#ps1 > > > > > > > > > > > > What crashes? That link just points to this CL. > > > > > > > > > > The browsertest failures here: > > > > > > > > > > https://build.chromium.org/p/tryserver.chromium.linux/ > > > > > builders/linux_chromium_chromeos_rel_ng/builds/261327 > > > > > > > > > > They failed due to a segfault, not a test expectation failure. > > > > > > > > > > https://codereview.chromium.org/2244733002/ > > > > > > > > > > > > > -- > > > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. > > > > > > It was odd, but even locally I didn't get a segfault message or stacktrace. > > > > > > I eventually figured out what was going on, but was confused for a while. > > > > > > https://codereview.chromium.org/2244733002/ > > > > > > > -- > > 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 mailto:chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
