913847 - nsThreadUtils.h is a hub header

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: The low-hanging fruit: stop needlessly including nsThreadUtils.h

A hub header is one that is included by many other headers, and itself includes many other headers. See tracking bug 912735.

This bug is about doing what we can quickly do to make it better.

First, the low-hanging fruit...

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Split NS_IsMainThread and NS_GetMainThread into a new MainThreadUtils.h header

While doing the low-hanging fruit it was clear that many headers still have to include nsThreadUtils.h only because they need NS_IsMainThread. That function can't conveniently be forward-declared because its prototype depends on complex #ifdefs.

And while we're at it, a couple more headers only need NS_GetMainThread. So these two functions go together into a new header, MainThreadUtils.h.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Include MainThreadUtils.h instead of nsThreadUtils.h

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=f9c64588f3eb

Comment 4

[:ehsan akhgari]

Comment on attachment 801147 [details] [diff] [review]
Split NS_IsMainThread and NS_GetMainThread into a new MainThreadUtils.h header

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

::: xpcom/glue/MainThreadUtils.h
@@ +3,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef IsMainThread_h_

Nit: MainThreadUtils_h

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=51c2655a39ca

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=a028687984c6

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=851e5cdd8f67

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=0b2d1d5bb834

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=c6fe3a5f3901

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

https://tbpl.mozilla.org/?tree=Try&rev=960469211b9b

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/554bfe767519
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4eb44a3149ed
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/040522aacba4

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

(was backed out for leaks)

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

@#$#$%^ NS_GetMainThread() addrefs its result!!

Of course! After all, we wouldn't want the main thread to go away under our feet!!

And of course, raw pointers are strong pointers, everyone knows that!

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

Fixes the leak locally... no reason why it shouldn't be green now.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f50b9bd02eae
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8cd6137f030a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d37facef0b

Comment 15

[Trevor Saunders (:tbsaunde)]

(In reply to Benoit Jacob [:bjacob] from comment #13)
> @#$#$%^ NS_GetMainThread() addrefs its result!!

Well, its an xpcom thing and those add ref out args...

Of course we should totally decom it if there isn'ta already such a version.

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f50b9bd02eae
https://hg.mozilla.org/mozilla-central/rev/8cd6137f030a
https://hg.mozilla.org/mozilla-central/rev/e4d37facef0b