It looks to me like your rebase is correct. I applied your patch locally, and it applied cleanly to a recent mozilla-central. I suspect the "push failed on remote" error that Phabricator is showing is an unrelated error in our automation infrastructure that we can ignore. I did realize in the meantime that I haven't finished reviewing your patch. I wrote some comments in BaseMargin.h (which you have since addressed), but I hadn't looked at later parts of your patch. I'd like to take a more detailed look at those parts, with a view to possibly avoiding some of the other static_casts being added. I might not get a chance to do so until next week, as I'm at a conference right now. In any case, the patch is definitely on the right track, thansk for your work so far!
Bug 1585806 Comment 13 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
It looks to me like your rebase is correct. I applied your patch locally, and it applied cleanly to a recent mozilla-central. I suspect the "push failed on remote" error that Phabricator is showing is an unrelated error in our automation infrastructure that we can ignore. I did realize in the meantime that I haven't finished reviewing your patch. I wrote some comments in BaseMargin.h (which you have since addressed), but I hadn't looked at later parts of your patch. I'd like to take a more detailed look at those parts, with a view to possibly avoiding some of the other static_casts being added. I might not get a chance to do so until next week, as I'm at a conference right now. In any case, the patch is definitely on the right track, thanks for your work so far!