Move nsRunnable to mozilla::Runnable, CancelableRunnable to mozilla::

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

(Blocks 1 bug)

unspecified
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

Posted patch PatchSplinter Review
This is largely mechanical.
Attachment #8743051 - Flags: review?(nfroyd)
fyi comm-central will need some simple adjustments here.
Comment on attachment 8743051 [details] [diff] [review]
Patch

Review of attachment 8743051 [details] [diff] [review]:
-----------------------------------------------------------------

I'm going to assume this was essentially:

find . -name \*.h | xargs perl -pie 's/\bnsRunnable\b/mozilla::Runnable/g'
find . -name \*.cpp | xargs perl -pie 's/\bnsRunnable\b/Runnable/g'

with a bit of smarts on the header side to not add qualifiers when necessary.  Picking a couple of files at random suggests this is the case.
Attachment #8743051 - Flags: review?(nfroyd) → review+
It was a little more complicated than that, but yeah, that's pretty much what it was.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1)
> fyi comm-central will need some simple adjustments here.

Thanks Kyle for the heads-up!
Blocks: 1265951
Depends on: 1267577

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fcc0936b576d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Target Milestone: mozilla48 → mozilla49
Depends on: 1268822
Hi Nathan!

Sorry to uncover this old bug, but does the changes here imply that the docs at [1] (and other places?) need to be updated?

[1] - https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Cross-thread_calls_using_runnables
Flags: needinfo?(nfroyd)
(In reply to Alessio Placitelli [:Dexter] from comment #7)
> Sorry to uncover this old bug, but does the changes here imply that the docs
> at [1] (and other places?) need to be updated?

Yes!  Thanks for ferreting that out.
Flags: needinfo?(nfroyd)
(In reply to Nathan Froyd [:froydnj] from comment #8)
> (In reply to Alessio Placitelli [:Dexter] from comment #7)
> > Sorry to uncover this old bug, but does the changes here imply that the docs
> > at [1] (and other places?) need to be updated?
> 
> Yes!  Thanks for ferreting that out.

Cool! Who is going to do that? Should we simply mark this with dev-doc-needed?
Flags: needinfo?(nfroyd)
(In reply to Alessio Placitelli [:Dexter] from comment #9)
> (In reply to Nathan Froyd [:froydnj] from comment #8)
> > (In reply to Alessio Placitelli [:Dexter] from comment #7)
> > > Sorry to uncover this old bug, but does the changes here imply that the docs
> > > at [1] (and other places?) need to be updated?
> > 
> > Yes!  Thanks for ferreting that out.
> 
> Cool! Who is going to do that? Should we simply mark this with
> dev-doc-needed?

I know that's what you do for web-facing things; I'm less certain about what to do for Gecko internals.  I will put this on my list of things to do.  Thanks!
Flags: needinfo?(nfroyd)
You need to log in before you can comment on or make changes to this bug.