Closed Bug 1069556 Opened 10 years ago Closed 9 years ago

Get rid of local patches, sync back up with Breakpad upstream

Categories

(Toolkit :: Crash Reporting, defect, P1)

Product:
Toolkit
Component:
Crash Reporting
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla47
Tracking Flags:
Tracking Status
firefox44 + wontfix
firefox45 + wontfix
firefox46 + wontfix
firefox47 --- fixed
fennec + ---

People

(Reporter: ted, Assigned: ted)

References

Details

Attachments

(5 files)

MozReview Request: bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB)
40 bytes, text/x-review-board-request
glandium
: review+
Details
MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium
40 bytes, text/x-review-board-request
glandium
: review+
Details
MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium
40 bytes, text/x-review-board-request
BenWa
: review+
glandium
: review+
Details
Fix fileid on mac and android
2.01 KB, patch
ted
: review+
Details | Diff | Splinter Review
Fix fileid to build on mac and android and add a selftest
4.95 KB, patch
Details | Diff | Splinter Review
We took a bunch of local patches to support Breakpad-in-SPS and never got them upstreamed (to be fair they were pretty invasive). Breakpad has been replaced by LUL in SPS now, so we should be able to ditch the local patches and get back to a stock upstream Breakpad. First we need to get rid of the Breakpad-SPS integration code, which is bug 1007145. There are three local patches we're carrying that we'll still need to get upstreamed: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/breakpad-patches/12-bug863475.patch - bug 863475, ARM EXIDX unwind parsing. There's a patch that was up for review upstream for this but it died on the vine. Need to resurrect that. In the short term we can rebase this on top of Breakpad tip and keep it as a local patch. http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/breakpad-patches/19-bug942290-breakpad-exidx-merge.patch - bug 942290 - This patch depends on the previous patch, but it's trivial http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/breakpad-patches/17-bug942407-usersig.patch - bug 942407 - This patch got put up for upstream review but got ignored. Need to poke someone: https://breakpad.appspot.com/984002/
Depends on: 1069558
A wrinkle, putting a note here so I don't forget. Upstream just landed a patch that requires Android NDK version r10c to build: https://code.google.com/p/google-breakpad/source/detail?r=1396
Ted, it sounds like we may need to update breakpad for bug 1164052. Is this something you can look into?
Flags: needinfo?(ted)
No, sorry, there's non-trivial work in bug 1069558 to do to make this feasible. If you can get someone to fix that then I can probably do the rest.
Flags: needinfo?(ted)
Assignee: nobody → nchen
tracking-fennec: --- → ?
tracking-fennec: ? → +
Ted what do you think about moving the current breakpad aside, which will continue to be used for BHR, and then pasting in the new breakpad for crash reports? Jim could then update BHR to use something else at his leisure.
Flags: needinfo?(ted)
No, I think that's an even worse idea. That's how we wind up with two copies of chromium in the tree and never fix it.
Flags: needinfo?(ted)
I think it'd be fine if we rip out the Breakpad code in ThreadStackHelper first, then work on a replacement for later. The BHR native stack hasn't been of much use anyways. I filed bug 1196381 for this.
Blocks: 1196981
This bug is no longer blocked on bug 1069558 because of bug 1196381. Ted, is that all you need to continue working on this bug?
Assignee: nchen → nobody
No longer depends on: 1069558
Flags: needinfo?(ted)
That's great, thanks!
Flags: needinfo?(ted)
That try push was busted in several different ways, but I think I've got those all sorted now. I just need to rebase the two local patches we rely on that haven't been upstreamed and I think we'll be in good shape.
I've got our local patches rebased and applied, and everything seems good. I just need to tweak one more thing that I forgot about to match upstream (The dump_syms binary on Mac changed its commandline arguments slightly.)
Tracking this for 44 since it was brought up in the channel meeting today and seems like a good thing to stay on top of. It sounded like uplifting may be a bit too risky so it would be better riding with 44 to release.
status-firefox44: --- → affected
tracking-firefox44: --- → +
bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB)
bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r?glandium
Attachment #8676747 - Flags: review?(mh+mozilla)
bug 1069556 - local build changes to match up with upstream Breakpad. r?glandium,benwa This commit contains a few things: * Misc build fixup to sync with upstream--adding a few new moz.build files, source files * The final bits of unhooking Breakpad from the profiler: ** Revert to only building toolkit/crashreporter if MOZ_CRASHREPORTER. ** Stop building bits of Breakpad that we only needed for the profiler. ** Remove a few bits of profiler code that were used to interface with Breakpad. ** Remove toolkit/crashreporter/breakpad-logging, which was only used to suppress Breakpad logging for the in-process stackwalker. * Upstream removed their Android-compat sys/ucontext.h because the Android NDK added it, but the bionic we're using for Gonk builds is too old, so add a copy of sys/ucontext.h from bionic master to keep Gonk building.
Attachment #8676748 - Flags: review?(mh+mozilla)
Attachment #8676748 - Flags: review?(bgirard)
I got hung up for a bit because the dump_syms output was different on Mac, but after investigation it looks like the symbol dumper stopped outputting PUBLIC lines for symbols that already had a FUNC entry, so it's not harmful. I'll push this to try again when try is open...
Attachment #8676748 - Flags: review?(bgirard) → review+
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium https://reviewboard.mozilla.org/r/22803/#review20345 Profiler changes look good, we shouldn't be using breakpad to symbolicate anymore.
https://reviewboard.mozilla.org/r/22799/#review20411 ::: toolkit/crashreporter/update-breakpad.sh:8 (Diff revision 1) > +# Usage: update-breakpad.sh <path to breakpad git clone> [rev, defaults to HEAD] FWIW, we tend not to rely on already existing clones.
Comment on attachment 8676747 [details] MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium https://reviewboard.mozilla.org/r/22801/#review20413 If I run update-breakpad.sh /path/to/breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c, I get some weird CRLF related differences from what applying the patch gives. Apart from that, looks good. ::: toolkit/crashreporter/update-breakpad.sh:26 (Diff revision 1) > # remove some extraneous bits > -rm -rf ${crashreporter_dir}/google-breakpad/src/third_party/protobuf ${crashreporter_dir}/google-breakpad/src/testing/ ${crashreporter_dir}/google-breakpad/src/tools/gyp/ > +rm -rf \ > + ${crashreporter_dir}/google-breakpad/docs/ \ > + ${crashreporter_dir}/google-breakpad/src/third_party/protobuf \ > + ${crashreporter_dir}/google-breakpad/src/testing/ \ > + ${crashreporter_dir}/google-breakpad/src/tools/gyp/ \ > + ${crashreporter_dir}/google-breakpad/src/processor/testdata/ \ > + ${crashreporter_dir}/google-breakpad/src/tools/windows/dump_syms/testdata/ Maybe this should be part of the update-breakpad.sh patch?
Attachment #8676747 - Flags: review?(mh+mozilla) → review+
Attachment #8676748 - Flags: review?(mh+mozilla)
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium https://reviewboard.mozilla.org/r/22803/#review20415 ::: toolkit/crashreporter/crashreporter.mozbuild:17 (Diff revision 1) > +if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk' and CONFIG['OS_TARGET'] == 'Android': do we ever have MOZ_WIDGET_TOOLKIT=gonk with OS_TARGET != Android? That would be surprising. ::: toolkit/crashreporter/google-breakpad/src/client/linux/dump_writer_common/moz.build:12 (Diff revision 1) > +Library('breakpad_linux_dump_writer_common_s') You're not using this library directly, so you don't need to give it a name. ::: toolkit/crashreporter/google-breakpad/src/common/Makefile.in:11 (Diff revision 1) > +# memory.h in this dir breaks things if -I$(srcdir) gets added. Can you be more specific? ::: toolkit/crashreporter/google-breakpad/src/common/linux/moz.build:35 (Diff revision 1) > - ] > +] You've been birotted by bug 1175857 ::: toolkit/crashreporter/google-breakpad/src/common/moz.build (Diff revision 1) > - 'arm_ex_reader.cc', > - 'arm_ex_to_module.cc', Those two files come from the 00-arm-exidx-rollup.patch patch. If you're not building them, what does the patch do? This obviously needs to be folded with the previous patch.
https://reviewboard.mozilla.org/r/22801/#review20413 Odd, I produced this changeset by running the script. > Maybe this should be part of the update-breakpad.sh patch? Good call, don't know how that snuck into this patch.
https://reviewboard.mozilla.org/r/22803/#review20415 > do we ever have MOZ_WIDGET_TOOLKIT=gonk with OS_TARGET != Android? That would be surprising. I guess not, I can remove that last bit. > Can you be more specific? Some system headers `#include <memory.h>` (a totally reasonable thing to do) and they wind up including *this* memory.h, which obviously breaks things. I'll update the comment to note that. > Those two files come from the 00-arm-exidx-rollup.patch patch. If you're not building them, what does the patch do? Uh, oops? I generated this patch series iteratively by first updating to stock Breakpad and making things build, and then rebasing that patch, and I guess I missed the part where I actually needed to make these files build again.
https://reviewboard.mozilla.org/r/22799/#review20411 > FWIW, we tend not to rely on already existing clones. Cloning Breakpad is kind of a hassle. It requires you to clone depot_tools first and run their `fetch` script because they're using gclient to handle dependent subrepos.
Comment on attachment 8676746 [details] MozReview Request: bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB)
Comment on attachment 8676747 [details] MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium
Attachment #8676747 - Attachment description: MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r?glandium → MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium
Attachment #8676748 - Flags: review?(mh+mozilla)
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium bug 1069556 - local build changes to match up with upstream Breakpad. r?glandium,benwa This commit contains a few things: * Misc build fixup to sync with upstream--adding a few new moz.build files, source files * The final bits of unhooking Breakpad from the profiler: ** Revert to only building toolkit/crashreporter if MOZ_CRASHREPORTER. ** Stop building bits of Breakpad that we only needed for the profiler. ** Remove a few bits of profiler code that were used to interface with Breakpad. ** Remove toolkit/crashreporter/breakpad-logging, which was only used to suppress Breakpad logging for the in-process stackwalker. * Upstream removed their Android-compat sys/ucontext.h because the Android NDK added it, but the bionic we're using for Gonk builds is too old, so add a copy of sys/ucontext.h from bionic master to keep Gonk building.
Attachment #8676748 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium https://reviewboard.mozilla.org/r/22803/#review20837
Oh good this looks close to landing, new b2g devices Will need this for the aarch64 support.
So I thought I was done, but I had to wrangle B2G builds some more. I just submitted a Try run to check all the various B2G flavors, but this patch builds with B2G for me locally, as well as 64-bit Linux and arm/x86 Android.
Comment on attachment 8676746 [details] MozReview Request: bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22799/diff/2-3/
Comment on attachment 8676747 [details] MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22801/diff/2-3/
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22803/diff/2-3/
Attachment #8676748 - Attachment description: MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r?glandium,benwa → MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=glandium,benwa
Glandium, Ted: I noticed this bug is marked tracking for 44 and that two patches were r+d but don't see a Sheriff landed these patches. Do these patches need to be uplifted to Aurora44 or not?
Flags: needinfo?(ted)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(mh+mozilla)
These patches never landed. They got r+ but I got hung up on making them build for B2G.
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #42) > These patches never landed. They got r+ but I got hung up on making them > build for B2G. Do you want me to get this landed on m-c with a sheriffs help? Should I set the checkin-needed keyword? Or will the patches not apply cleanly now?
They apply cleanly but they don't build on all the B2G variants. I need to spend some more time on it unfortunately.
Given that we are halfway through the beta44 cycle, I would like to suggest wontfixing this for fx44. Tracked for 45.
status-firefox44: affected → wontfix
status-firefox45: --- → affected
tracking-firefox45: --- → +
Blocks: 1187826
Sorry, I got hung up on making this work on B2G and didn't have time to fix it. I spent some time on this yesterday and I made some progress, I'll give it another spin and see how it goes.
Ted, where are we on this? We are bad enough in Android stability that we consider not shipping, and there's a firm belief that this Breakpad update will gives us more actionable data on what to fix to get us back to a better state.
Flags: needinfo?(ted)
Comment on attachment 8676746 [details] MozReview Request: bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22799/diff/3-4/
Comment on attachment 8676747 [details] MozReview Request: bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22801/diff/3-4/
Attachment #8676748 - Attachment description: MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=glandium,benwa → MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium Review request updated; see interdiff: https://reviewboard.mozilla.org/r/22803/diff/3-4/
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #50) > Ted, where are we on this? > We are bad enough in Android stability that we consider not shipping, and > there's a firm belief that this Breakpad update will gives us more > actionable data on what to fix to get us back to a better state. Sorry, fixing the B2G build turned out to be more work than I anticipated, but I believe I have it all working now. I've pushed to try again and asked for re-review on the changes to make that work. If everything looks good on Try this should be landable immediately.
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #51) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7d63e854256 The Windows builds on this push are burning due to bug 1240993, so I re-pushed just Windows builds.
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium I'd like a once-over on the changes here, but Mozreview won't let me unset the r+ there.
Attachment #8676748 - Flags: review+ → review?(mh+mozilla)
Comment on attachment 8676746 [details] MozReview Request: bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) https://reviewboard.mozilla.org/r/22799/#review28811 ::: toolkit/crashreporter/update-breakpad.sh:6 (Diff revision 4) > -# Usage: update-breakpad.sh <path to breakpad SVN> > +set -v -e -x -v -x is nice when debugging, but when simply using, it's kind of overkill. ::: toolkit/crashreporter/update-breakpad.sh:15 (Diff revision 4) > -crashreporter_dir=`dirname $0` > +crashreporter_dir=`realpath $(dirname $0)` You can nest $() instead of mixing with ``: $(realpath $(dirname $0)) ::: toolkit/crashreporter/update-breakpad.sh:61 (Diff revision 4) > -find ${crashreporter_dir}/google-breakpad -name "*.orig" -print0 | xargs -0 rm > +find ${crashreporter_dir}/google-breakpad -name "*.orig" -exec rm '{}' \; -print0 | xargs -0 rm tends to be faster because rm is forked less, but meh.
Attachment #8676746 - Flags: review+
Mozreview is sort of confusing here, but I really just want a once-over on the local build changes patch: https://reviewboard.mozilla.org/r/22803/diff/4#index_header
Comment on attachment 8676748 [details] MozReview Request: bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa, r?glandium https://reviewboard.mozilla.org/r/22803/#review29025 Maybe move the gonk headers to /other-licenses/gonk?
Attachment #8676748 - Flags: review?(mh+mozilla) → review+
https://reviewboard.mozilla.org/r/22803/#review29025 I actually changed tack on this for the final patch and just copied the old version of the Breakpad headers that were removed or changed, so I think they're OK there since they originate from Breakpad.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e628eff02f5cf6fdc118ef0e61f3e17f627e7d2 bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) https://hg.mozilla.org/integration/mozilla-inbound/rev/09fd4f3ab6e764016fe073efb226f03b5969af59 bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/31c98f5e107b9271be88e7c8543c4dbb4a2b6526 bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa,glandium
backed out for multiple test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=20487667&repo=mozilla-inbound
Flags: needinfo?(ted)
Following our conversation on irc, the problem is that the breakpad code used by fileid is using the wrapped STL, which means it uses moz_xmalloc, which is not linked in fileid. In any case, the DISABLE_STL_WRAPPING workaround in toolkit/crashreporter/google-breakpad/src/common/linux/Makefile.in is not enough anymore, and trying to expand it is worthless because the STL *is* used, and we don't want unwrapped code in libxul. That workaround can go away, independently of anything else. There are several solutions to this: - build fileid as a GeckoProgram with linkage=None, but that means on mac there will be a dependency on libmozglue.dylib. - statically link mozalloc to fileid (which requires to assign a LIBRARY_NAME to mozalloc) - build another set of breakpad libraries (only those listed in testing/tools/fileid/moz.build) entirely built without STL wrapping, and use it for fileid, instead of sharing the same as for libxul. The second one is probably the least effort, assuming it works. A cursory test suggests that it does.
https://hg.mozilla.org/mozilla-central/rev/c7b86ad8cd86
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Status: RESOLVED → REOPENED
Flags: needinfo?(ted)
Resolution: FIXED → ---
https://hg.mozilla.org/integration/mozilla-inbound/rev/a319694b46d54d9240842f0c525cf5570cf4649a bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) https://hg.mozilla.org/integration/mozilla-inbound/rev/16f9d3a6d2ef6a6efd088e3b8eff0a4723daef8f bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/24dbe7da737057008fd62dafc1783aedb840dc23 bug 1069556 - local build changes to match up with upstream Breakpad. r=benwa,glandium
This broke on Android and Mac, so I give up.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #71) > This broke on Android and Mac, so I give up. WAIT WHAT
That was a fileid problem too, so I pushed glandium's option one from comment 68 to try (based on when this was on inbound) and it looks ok: https://treeherder.mozilla.org/#/jobs?repo=try&revision=12c0e8322cf0
Attachment #8713894 - Flags: review?(ted)
FYI, I know you are doing your best. We really want this to land asap in m-c. We have a few bugs we cannot act because of the lack of relevant backtrace. We hope the new version of breakpad will help with this.
Comment on attachment 8713894 [details] [diff] [review] Fix fileid on mac and android Review of attachment 8713894 [details] [diff] [review]: ----------------------------------------------------------------- This looks sensible enough, but we ought to verify that fileid still works. Maybe we can tweak testAssertStack in selftest.py to pass a symbols path? It should be as simple as doing: ``` path = os.path.join(build_obj.distdir, 'crashreporter-symbols') if os.path.isdir(path): ... ```
Attachment #8713894 - Flags: review?(ted) → review+
Assignee: nobody → cmanchester
Status: REOPENED → ASSIGNED
Assignee: cmanchester → ted
(In reply to Chris Manchester [:chmanchester] from comment #77) > Created attachment 8715477 [details] [diff] [review] > Fix fileid to build on mac and android and add a selftest > > Patch with updated selftest. Try, in case the automation build environment breaks that for some reason: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7900de044b55
You're the best! I owe you a beer.
[Tracking Requested - why for this release]: We might want to uplift that to 46 when it is ready (and if the developers agree)
tracking-firefox46: --- → ?
Ok, with chmanchester's patch applied, that latest try run looks good!
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9bf35bec8aec0ea8e151097434875f0db0559fa bug 1069556 - change update-breakpad.sh to work with Breakpad's git repo (NPOTB) https://hg.mozilla.org/integration/mozilla-inbound/rev/62a0d8c93da2ca24f3f87642a2989c4741f214d4 bug 1069556 - sync to Breakpad c53ed143108948eb7e2d7ee77dc8c0d92050ce7c. r=glandium, benwa
https://hg.mozilla.org/mozilla-central/rev/c9bf35bec8ae https://hg.mozilla.org/mozilla-central/rev/62a0d8c93da2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Congratulations guys, this will really help us (especially on mobile). Many thanks!
(In reply to Pulsebot from comment #86) > https://hg.mozilla.org/integration/mozilla-inbound/rev/6c98cc47620d This reads: "Followup for bug 1069556: Remove what was removed in bug 1245055. r=me"
Oops, sorry for undoing your changes there!
We probably don't want to uplift this in beta. However, having it in aurora might help. Ted, do you agree? If so, could you fill the uplift request to aurora?
status-firefox45: affected → wontfix
Flags: needinfo?(ted)
I don't really know, I wasn't fixing this to take care of any particular problem, although I know there was hope it would help with some bad Android crash reports. We probably need to take a look at the crash stats for the past few days to see if it made any noticeable improvement.
Flags: needinfo?(ted)
OK, thanks. Kairo, could you verify if it has a positive impact before we uplift it? Danke!
Flags: needinfo?(kairo)
Ted, any thoughts here? Are you still working on this or shall I not worry about it for 46? Tracking for now so that I will come back to it before merge day.
tracking-firefox46: ? → +
Flags: needinfo?(ted)
Like I said in comment 91, this needs Kairo to weigh in on whether this made any difference in Android signatures. From some unittest log investigation in another bug it doesn't look like this fixed the situation.
Flags: needinfo?(ted)
We believed that this would help with bug 1218850 (Crash at hex address on x86 Android). We still see these signatures after the upgrade. I think we should just let this ride the trains.
Kevin is following the signatures more closely than I am, so I trust his judgement on this.
Flags: needinfo?(kairo)
Blocks: 791107
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: