Closed Bug 1265927 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: khuey, Assigned: khuey)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached 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
https://hg.mozilla.org/mozilla-central/rev/fcc0936b576d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Target Milestone: mozilla48 → mozilla49
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.