All calls to NS_DispatchTo(Main|Current)Thread under layout/xul/. http://searchfox.org/mozilla-central/search?q=NS_DispatchTo(Main%7CCurrent)Thread&case=false&regexp=true&path=layout%2Fxul

Calls related to TimerCallback. http://searchfox.org/mozilla-central/search?q=TimerCallback&case=false&regexp=false&path=layout%2Fxul

All timer callbacks using nsRepeatService would be labeled in Bug 1348738.

Comment on attachment 8848024 [details] Bug 1342874 - (Part 1) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in XUL frames. https://reviewboard.mozilla.org/r/120986/#review124716 ::: layout/xul/nsImageBoxFrame.cpp:116 (Diff revision 2) > > nsCOMPtr<nsIRunnable> event = new nsImageBoxFrameEvent(aContent, aMessage); > - if (NS_FAILED(NS_DispatchToCurrentThread(event))) > + nsresult rv = aContent->OwnerDoc()->Dispatch("nsImageBoxFrameEvent", > + TaskCategory::Other, > + event.forget()); > + if (NS_FAILED(rv)) While you're replacing this "if" check, you might as well upgrade it to use curly-braces {} around its body, too, per Mozilla coding style. ::: layout/xul/nsListBoxBodyFrame.cpp:831 (Diff revision 2) > - RefPtr<nsPositionChangedEvent> ev = > + RefPtr<nsPositionChangedEvent> event = > new nsPositionChangedEvent(this, aUp, aDelta); > - nsresult rv = NS_DispatchToCurrentThread(ev); > + RefPtr<nsPositionChangedEvent> eventKeeper(event); > + nsresult rv = mContent->OwnerDoc()->Dispatch("nsPositionChangedEvent", > + TaskCategory::Other, > + event.forget()); > if (NS_SUCCEEDED(rv)) { > - if (!mPendingPositionChangeEvents.AppendElement(ev)) { > + if (!mPendingPositionChangeEvents.AppendElement(eventKeeper)) { As noted in bug 1342863, I think this "eventKeeper" extra-variable is an anti-pattern. I think you just want to use do_AddRef in cases like this. ::: layout/xul/tree/nsTreeBodyFrame.cpp:4743 (Diff revision 2) > - RefPtr<ScrollEvent> ev = new ScrollEvent(this); > - if (NS_FAILED(NS_DispatchToCurrentThread(ev))) { > + RefPtr<ScrollEvent> event = new ScrollEvent(this); > + RefPtr<ScrollEvent> eventKeeper(event); > + nsresult rv = mContent->OwnerDoc()->Dispatch("ScrollEvent", > + TaskCategory::Other, > + event.forget()); > + if (NS_FAILED(rv)) { > NS_WARNING("failed to dispatch ScrollEvent"); > } else { > - mScrollEvent = ev; > + mScrollEvent = eventKeeper; s/eventKeeper/do_AddRef/

Comment on attachment 8848025 [details] Bug 1342874 - (Part 2) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in nsXULPopupManager. https://reviewboard.mozilla.org/r/120988/#review124720 ::: layout/xul/nsXULPopupManager.cpp:2763 (Diff revision 2) > - NS_DispatchToCurrentThread(event); > + aPopup->OwnerDoc()->Dispatch("nsXULPopupPositionedEvent", > + TaskCategory::Other, > + event.forget()); Nit: the indentation is off in the last 2 lines here.

Comment on attachment 8848024 [details] Bug 1342874 - (Part 1) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in XUL frames. https://reviewboard.mozilla.org/r/120986/#review125670 r=me with one nit ::: layout/xul/nsMenuBarFrame.cpp:346 (Diff revision 3) > nsCOMPtr<nsIRunnable> event = > new nsMenuBarSwitchMenu(GetContent(), aOldMenu, aNewMenu, aSelectFirstItem); > - return NS_DispatchToCurrentThread(event); > + return mContent->OwnerDoc()->Dispatch("nsPositionChangedEvent", Copypaste typo -- the label is wrong here. s/"nsPositionChangedEvent"/"nsMenuBarSwitchMenu"/, I think

Comment on attachment 8848025 [details] Bug 1342874 - (Part 2) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in nsXULPopupManager. https://reviewboard.mozilla.org/r/120988/#review125676 r=me

Comment on attachment 8849176 [details] Bug 1342874 - (Part 3) DocGroup labeling for timer callbacks in nsListScrollSmoother. https://reviewboard.mozilla.org/r/122010/#review125688 r- for now, because I don't entirely understand why the closure needs to be put into a local RefPtr, and I want to better understand why it does (or doesn't) need that. (see 3rd nit below) ::: layout/xul/nsListBoxBodyFrame.cpp:105 (Diff revision 3) > void > nsListScrollSmoother::Start() > { > + > + nsTimerCallbackFunc Please delete the blank first line of the function here. ::: layout/xul/nsListBoxBodyFrame.cpp:109 (Diff revision 3) > + nsTimerCallbackFunc > + scrollSmootherCallback = [](nsITimer* aTimer, void* aClosure) { > + RefPtr<nsListScrollSmoother> self = I find this wrapping/indentation (with a newline between the type and the variable name) hard to read. How about just wrapping in the lambda's arg list, like we do for normal function signatures? Like this: nsTimerCallbackFunc scrollSmootherCallback = [](nsITimer* aTimer, void* aClosure) { [code, indented by 2 spaces]; }; Note that after this, the function-body should only be indented by 2 spaces with respect to its scope, too -- you've got it indented by 4 (with respect to the scope it's inside of) right now, because the extra linewrap prompted a redundant 2-space indent, I think. ::: layout/xul/nsListBoxBodyFrame.cpp:111 (Diff revision 3) > + RefPtr<nsListScrollSmoother> self = > + static_cast<nsListScrollSmoother*>(aClosure); > + Do we actually need this local var to be a RefPtr (and cause refcount churn)? (I'm not sure, but it's not clear to be that we do.) ::: layout/xul/nsListBoxBodyFrame.cpp:117 (Diff revision 3) > + if (!self->mOuter) return; > + > + // actually do some work. > + self->mOuter->InternalPositionChangedCallback(); Since you're already rewriting this code a bit (it's not a straight copypaste), let's improve things a bit style-wise here and get rid of the "return". (Both because it's on the same line as its if-condition which is evil, and because this is a function-within-a-function so a stray "return" is potentially confusing/ambiguous, control-flow-wise.) Let's just rewrite these lines as: if (self->mOuter) { // actually do some work. self->mOuter->InternalPositionChangedCallback(); }

Comment on attachment 8849177 [details] Bug 1342874 - (Part 4) DocGroup labeling for timer callbacks in nsMenuFrame. https://reviewboard.mozilla.org/r/122012/#review125762 r- since I think we can do away with the member-variable (see below), and that will make this patch a good bit smaller I think. ::: layout/xul/nsMenuFrame.cpp:1522 (Diff revision 3) > + */ > +NS_IMETHODIMP nsMenuTimerMediator::GetName(nsACString& aName) > +{ Wrap after NS_IMETHODIMP please. (This file is a bit messy -- it has a mix of wrap & no-wrap style for this. But wrapping is the standard Mozilla coding style, so you should stick with that for new chunks.) ::: layout/xul/nsMenuFrame.cpp:1523 (Diff revision 3) > +NS_IMETHODIMP nsMenuTimerMediator::GetName(nsACString& aName) > +{ > + aName = mName; > + return NS_OK; > +} > + > +/** > + * Set the name to this timer callback. > + * @param aName the name to set > + */ > +NS_IMETHODIMP nsMenuTimerMediator::SetName(const char * aName) > +{ > + mName.Assign(aName); > + return NS_OK; I don't think we expect anyone to ever adjust our name, right? If not, we should just do away with the member-var, and just make GetName always return the same string, and make SetName return NS_ERROR_NOT_IMPLEMENTED. At least, that's what a few other nsINamed impls seem to do: https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/dom/xhr/XMLHttpRequestMainThread.cpp#3784-3795 https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/dom/media/DecoderDoctorDiagnostics.cpp#652-663 (Not sure how common this pattern is; these are just the first two that I found and they both do that, and it seems reasonable to me assuming we're just using this for logging/reporting.) ::: layout/xul/nsMenuFrame.cpp:1532 (Diff revision 3) > + */ > +NS_IMETHODIMP nsMenuTimerMediator::SetName(const char * aName) > +{ As above, wrap after NS_IMETHODIMP.

Comment on attachment 8849177 [details] Bug 1342874 - (Part 4) DocGroup labeling for timer callbacks in nsMenuFrame. https://reviewboard.mozilla.org/r/122012/#review125770 ::: commit-message-4f3c5:3 (Diff revision 3) > +Make nsMenuTimerMediator inherit nsINamed. Because using `InitWithCallback()` > +would add ref count for mTimerMediator, it can prevent mTimerMediator being > +null. I'm not sure what the second sentence is saying here. It doesn't really seem to be true -- InitWithCallback does not actually "prevent mTimerMediator being null". mTimerMediator is just a variable (of type RefPtr) -- anybody could set it to null, regardless of what InitWithCallback does with the copy it got. I think you really mean to talk about the *lifetime* of mTimerMediator, and you're trying to say that InitWithCallback will keep it alive until the callback fires, right? Please rephrase along those lines. ::: commit-message-4f3c5:7 (Diff revision 3) > +If using `InitWithNamedFuncCallback()`, mTimerMediator is possible to be > +accessed with null. Because mTimerMediator would be destroyed when nsMenuFrame > +destroyed, and the timer callback is possible to be ran after nsMenuFrame > +destroyed. > + This paragraph doesn't make sense to me at all, really. Could you reword a bit for clarity? In particular: - "accessed with null" doesn't make sense - The second sentence here isn't a complete sentence. - "to be ran" --> to be run" (s/a/u/) - The mention of InitWithNamedFuncCallback() comes out of nowhere, because you're not actually using it in this patch. (Maybe you're intending this comment to explain *why* you're not using InitWithNamedFuncCallback()? If so, please state that more clearly -- e.g. start this whole idea out with "Note that we still use InitWithCallback() instead of upgrading to InitWithNamedFuncCallback. This is because they have different lifetime implications for their callback arg, mTimerMediator. [etc]"

Comment on attachment 8849178 [details] Bug 1342874 - (Part 5) DocGroup labeling for timer callbacks in nsXULPopupManager. https://reviewboard.mozilla.org/r/122014/#review125774 r=me

Comment on attachment 8849179 [details] Bug 1342874 - (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. https://reviewboard.mozilla.org/r/122016/#review125778 r- on this one because I'm not clear that SystemGroup is actually correct. ::: layout/xul/nsXULTooltipListener.cpp:199 (Diff revision 3) > mTooltipTimer = do_CreateInstance("@mozilla.org/timer;1"); > + mTooltipTimer->SetTarget(SystemGroup::EventTargetFor(TaskCategory::Other)); Are you sure SystemGroup is appropriate here? The documentation for it says it's "for dispatching runnables that don't need to touch web content" https://dxr.mozilla.org/mozilla-central/source/xpcom/threads/SystemGroup.h#13 ...but at first glance, the callback *does* seem to work with web content. At least, sTooltipCallback calls "instance->ShowTooltip()" which starts out with: nsCOMPtr<nsIContent> sourceNode = do_QueryReferent(mSourceNode); ...which looks like maybe web content. https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsXULTooltipListener.cpp#392-395 Maybe you should really be dispatching this to the document for one of the nodes involved here? (Maybe using the local variable "sourceContent", which seems to come from the arg aEvent? I'm not sure, this is just a first guess from skimming the code above this.)

Comment on attachment 8849180 [details] Bug 1342874 - (Part 7) DocGroup labeling for timer callbacks in nsTreeBodyFrame. https://reviewboard.mozilla.org/r/122018/#review125780 r=me

Comment on attachment 8849181 [details] Bug 1342874 - (Part 8) DocGroup labeling for timer callbacks in nsTreeSelection. https://reviewboard.mozilla.org/r/122020/#review125790 r- on this one since I'd like to see the final version, but I think it's nearly there. ::: layout/xul/tree/nsTreeSelection.cpp:857 (Diff revision 3) > +nsIContent* > +nsTreeSelection::GetContent() > +{ > + if (!mTree) { > + return nullptr; > + } > + > + nsCOMPtr<nsIDOMElement> element; > + nsCOMPtr<nsIBoxObject> boxObject = do_QueryInterface(mTree); > + boxObject->GetElement(getter_AddRefs(element)); > + nsCOMPtr<nsIContent> content = do_QueryInterface(element); > + return content; A few things: (1) Please change the return type from a raw pointer to already_AddRefed<nsIContent>, and change the return statement to "return content.forget()", and be sure all the callsites capture the result in a nsCOMPtr. (In the current patch, the raw pointer return statement here is bogus/dangerous -- we don't have any strong guarantees about the lifetime of the thing it points to, once this local nsCOMPtr goes out of scope. For all we know, that's the last nsCOMPtr and the thing gets deleted right then!! I'm willing to believe it's probably still alive, but that's impossible to know without a bunch of research/assumptions into the implementation of mTree & friends.) (2) Please restore the line-ordering and blank spaces between lines from the original version of this code (as it previously existed in nsTreeSelection::GetSingle). In particular: - Declare variables as close to their usage as possible -- so "element" should be declared 1 line further down, as it was in the original code. - The blank spaces between the steps (from the original version of this code) make it easier (for me at least) to separate this into a 3-part process and grok the pieces separately, amongst all the obscure COM interface-conversions.

Comment on attachment 8849179 [details] Bug 1342874 - (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. https://reviewboard.mozilla.org/r/122016/#review125778 > Are you sure SystemGroup is appropriate here? The documentation for it says it's "for dispatching runnables that don't need to touch web content" > https://dxr.mozilla.org/mozilla-central/source/xpcom/threads/SystemGroup.h#13 > > ...but at first glance, the callback *does* seem to work with web content. At least, sTooltipCallback calls "instance->ShowTooltip()" which starts out with: > nsCOMPtr<nsIContent> sourceNode = do_QueryReferent(mSourceNode); > ...which looks like maybe web content. > https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsXULTooltipListener.cpp#392-395 > > > Maybe you should really be dispatching this to the document for one of the nodes involved here? (Maybe using the local variable "sourceContent", which seems to come from the arg aEvent? I'm not sure, this is just a first guess from skimming the code above this.) Oh thanks. I missed the usage of sourcNode.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9b9f0d05e7f22157e210826d2bbe2c478cbbfb4

Comment on attachment 8849176 [details] Bug 1342874 - (Part 3) DocGroup labeling for timer callbacks in nsListScrollSmoother. https://reviewboard.mozilla.org/r/122010/#review126526 ::: layout/xul/nsListBoxBodyFrame.cpp:110 (Diff revision 4) > + // nsListScrollSmoother would not be nullptr here. mRepeatTimer->Stop() > + // would be called when nsListScrollSmoother destroyed. > + auto self = static_cast<nsListScrollSmoother*>(aClosure); Similar to comment 33 -- you seem to be writing comments about "null" incorrectly. The real thing to worry about here is *not* about nullness (though you could assert that aClosure is non-null, if you like). The real thing to worry about is whether aClosure is pointing to something non-null which has been deleted. (That's the sort of thing that turns into security exploits). Put another way: here's the question that someone reading this code should wonder: "Is it possible that 'void* aClosure' is a pointer to a nsListScrollSmoother object which was originally alive when this callback was created, BUT which has been deleted by the time this callback actually runs? i.e. how do we know that the refcounted object that aClosure points to is still alive?") Your comment here should anticipate that question and answer it. (The current text is on the right track, but it's confused because it talks about nsListScrollSmoother being "nullptr" which is an unrelated concern.)

Comment on attachment 8849177 [details] Bug 1342874 - (Part 4) DocGroup labeling for timer callbacks in nsMenuFrame. https://reviewboard.mozilla.org/r/122012/#review126528 r=me on this part with nits addressed: ::: commit-message-4f3c5:5 (Diff revision 4) > +Bug 1342874 - (Part 4) DocGroup labeling for timer callbacks in nsMenuFrame. r?dholbert > + > +Note that we still use InitWithCallback() instead of upgrading to > +InitWithNamedFuncCallback(). This is because they have different lifetime > +implications for their callback arg, mTimerMediator. s/their callback arg/the callback arg that nsMenuFrame passes in,/ (This is my own fault; my original suggested text was too hand-wavy. Sorry about that.) ::: commit-message-4f3c5:7 (Diff revision 4) > +So we make nsMenuTimerMediator inherit nsINamed. Using InitWithCallback() > +will keep mMenuTimerMediator alive until the callback fires. Two nits: (1) The first sentence here, "So we make nsMenuTimerMediator inherit nsINamed", kinda comes out of nowhere. (The text right before it was talking about lifetimes, but nsINamed doesn't have anything to do with lifetimes.) Probably replace that sentence with something which more clearly explains the motivation, like this: "So to label the callback, we make nsMenuTimerMediator inherit from nsINamed, and we provide the label via that API's GetName method." (2) The second sentence also feels like it comes out of nowhere. (It's really about the existing code & lifetimes and not about nsINamed.) I think that sentence wants to join the first paragraph, probably, since it's an extra thought about what that first paragraph is already talking about. Also, s/will keep/keeps/ -- since you're talking about something that we already do in existing code. ::: layout/xul/nsMenuFrame.cpp:1535 (Diff revision 4) > +/** > + * Set the name to this timer callback. > + * @param aName the name to set > + */ > +NS_IMETHODIMP > +nsMenuTimerMediator::SetName(const char * aName) nit: delete the space between "char" and "*"

Comment on attachment 8849179 [details] Bug 1342874 - (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. https://reviewboard.mozilla.org/r/122016/#review126534 ::: layout/xul/nsXULTooltipListener.cpp:199 (Diff revision 4) > + nsCOMPtr<nsIContent> content = do_QueryReferent(mSourceNode); > + if (content) { > + mTooltipTimer->SetTarget( > + content->OwnerDoc()->EventTargetFor(TaskCategory::Other)); > + } In comment 35, I suggested using the local variable "sourceContent" here, but you're using a different variable which you're deriving from "mSourceNode" via a few extra steps. Is there a reason you can't just directly use "sourceContent"? In particular: - It looks like sourceContent is the correct type (nsCOMPtr<nsIContent>) - It's already known to be non-null (we've already dereferenced it in code above this) - It's actually the same thing you're deriving in |content| here, since you're starting with "mSourceNode" which itself is set from "sourceContent" earlier. :) So I think you do really want to use sourceNode (and you already are, but just in a roundabout way...)

Comment on attachment 8849181 [details] Bug 1342874 - (Part 8) DocGroup labeling for timer callbacks in nsTreeSelection. https://reviewboard.mozilla.org/r/122020/#review126538 r=me on this part, with one comment nit.) ::: layout/xul/tree/nsTreeSelection.h:40 (Diff revision 4) > + // Helper function to get content > + already_AddRefed<nsIContent> GetContent(); This comment needs a bit more information to become useful. (Right now it's just restating the function signature, without adding any explanatory value.) Let's add a bit more so that it's useful. How about: // Helper function to get the content node associated with mTree. (I'm not really familiar with this file, but I think this is an accurate description based on the implementation; and it's more helpful as a quick summary of what the function might do.)

Comment on attachment 8849179 [details] Bug 1342874 - (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. https://reviewboard.mozilla.org/r/122016/#review126534 > In comment 35, I suggested using the local variable "sourceContent" here, but you're using a different variable which you're deriving from "mSourceNode" via a few extra steps. > > Is there a reason you can't just directly use "sourceContent"? > > In particular: > - It looks like sourceContent is the correct type (nsCOMPtr<nsIContent>) > - It's already known to be non-null (we've already dereferenced it in code above this) > - It's actually the same thing you're deriving in |content| here, since you're starting with "mSourceNode" which itself is set from "sourceContent" earlier. :) > > So I think you do really want to use sourceNode (and you already are, but just in a roundabout way...) I didn't notice sourceContent in the same scope with the timer callback. Fixed it now.

Comment on attachment 8849176 [details] Bug 1342874 - (Part 3) DocGroup labeling for timer callbacks in nsListScrollSmoother. https://reviewboard.mozilla.org/r/122010/#review126662 r=me with one null-check added. ::: layout/xul/nsListBoxBodyFrame.cpp:110 (Diff revision 5) > + // The passed-in nsListScrollSmoother is always alive here. Because if > + // nsListScrollSmoother died, mRepeatTimer->Stop() would be called during > + // the destruction and this callback would never be invoked. > + auto self = static_cast<nsListScrollSmoother*>(aClosure); This comment is much clearer! Thanks for fixing it up. ::: layout/xul/nsListBoxBodyFrame.cpp:125 (Diff revision 5) > mRepeatTimer = do_CreateInstance("@mozilla.org/timer;1"); > - mRepeatTimer->InitWithCallback(this, SMOOTH_INTERVAL, nsITimer::TYPE_ONE_SHOT); > + nsIContent* content = mOuter->GetContent(); > + if (content) { > + mRepeatTimer->SetTarget( > + content->OwnerDoc()->EventTargetFor(TaskCategory::Other)); One final thing I'm just noticing -- based on the assertion-and-early-return further up in the callback, it looks like the "mOuter" field can occasionally be a null pointer. (This was noted 16 years ago in bug 68365, and we added that early-return-and-assertion as a band-aid, and apparently we never tracked down *why* it unexpectedly ends up null... Too bad.) So, you probably need a null-check for mOuter here before you use it, or else you'll crash in whatever scenario produces null "mOuter" pointers here. I'd suggest that you replace this line: nsIContent* content = mOuter->GetContent(); ...with the following: nsIContent* content = nullptr; if (mOuter) { content = mOuter->GetContent(); }

Comment on attachment 8849179 [details] Bug 1342874 - (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. https://reviewboard.mozilla.org/r/122016/#review126670 r=me. Thanks!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83e7501f86b5dfe77e54608d19fd5a7703fef7ad&selectedJob=86896773

Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d3e4854a0bca (Part 1) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in XUL frames. r=dholbert https://hg.mozilla.org/integration/autoland/rev/dc73fede9e2e (Part 2) DocGroup labeling for runnables dispatched by NS_DispatchTo(Main|Current)Thread in nsXULPopupManager. r=dholbert https://hg.mozilla.org/integration/autoland/rev/ff081e160157 (Part 3) DocGroup labeling for timer callbacks in nsListScrollSmoother. r=dholbert https://hg.mozilla.org/integration/autoland/rev/beffca8b62d1 (Part 4) DocGroup labeling for timer callbacks in nsMenuFrame. r=dholbert https://hg.mozilla.org/integration/autoland/rev/9880010b77e3 (Part 5) DocGroup labeling for timer callbacks in nsXULPopupManager. r=dholbert https://hg.mozilla.org/integration/autoland/rev/3b363b82f9d0 (Part 6) DocGroup labeling for timer callbacks in nsXULTooltipListener. r=dholbert https://hg.mozilla.org/integration/autoland/rev/b16051c50387 (Part 7) DocGroup labeling for timer callbacks in nsTreeBodyFrame. r=dholbert https://hg.mozilla.org/integration/autoland/rev/fdfa03bfe8c5 (Part 8) DocGroup labeling for timer callbacks in nsTreeSelection. r=dholbert

https://hg.mozilla.org/mozilla-central/rev/d3e4854a0bca https://hg.mozilla.org/mozilla-central/rev/dc73fede9e2e https://hg.mozilla.org/mozilla-central/rev/ff081e160157 https://hg.mozilla.org/mozilla-central/rev/beffca8b62d1 https://hg.mozilla.org/mozilla-central/rev/9880010b77e3 https://hg.mozilla.org/mozilla-central/rev/3b363b82f9d0 https://hg.mozilla.org/mozilla-central/rev/b16051c50387 https://hg.mozilla.org/mozilla-central/rev/fdfa03bfe8c5