950386 - Move mozilla::Selection to mozilla::dom::Selection

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[(no longer active)]

Follow-up for bug 949445.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

It seems like unified builds make this annoying, because files that refer to just "Selection" incorrectly compile locally due to stealing a "using namespace mozilla::dom;" from a neighbor and then break on try.  I don't suppose there's any clever way to get unified builds to not do this, is there?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, boy.  Arguably, with unified builds the same rules that apply to "using namespace" in headers should apply to .cpp files...

But anyway, --disable-unified-compilation locally might do what you want here?

Comment 3

[(no longer active)]

(In reply to comment #2)
> Oh, boy.  Arguably, with unified builds the same rules that apply to "using
> namespace" in headers should apply to .cpp files...
> 
> But anyway, --disable-unified-compilation locally might do what you want here?

Well, we still need to make sure that the code builds in both configurations.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: patch

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

Will do an extra try before pushing to make sure it hasn't bitrotted.  I wasn't sure when to do "using" vs. explicitly invoking the namespace, so tell me if you want me to change it:

* In accessible/ and layout/ I didn't use "using namespace mozilla::dom;", but in one selection-heavy file I did "using mozilla::dom::Selection;".  Obviously in nsSelection.cpp itself I used "using namespace mozilla::dom;".
* In editor/ I used "using namespace mozilla::dom;" because of how heavily it uses DOM stuff.
* In editor/libeditor/html/nsWSRunObject.h I had to explicitly qualify ::DOMPoint for it to compile.  I suspect this is because I added "using namespace mozilla::dom;" to some file that got unified with a file including this one, so it became ambiguous in the unified file.  Has anyone noted that unified builds are a bit of a hack?

Comment 5

[(no longer active)]

(In reply to :Aryeh Gregor from comment #4)
> Has anyone noted that unified builds are a bit of a hack?

Yes, indeed (and they're mostly my fault.)  The trade-off is between these kinds of hacks and making everybody more productive when they're not fighting these kinds of issues by giving them 2x faster builds.

Also, note that the try push only builds in unified mode, in order to test non-unified compilation you need to modify http://mxr.mozilla.org/mozilla-central/source/build/mozconfig.common.override and add ac_add_options --disable-unified-compilation there.

Comment 6

[(no longer active)]

Comment on attachment 8411018 [details] [diff] [review]
patch

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

::: dom/bindings/Bindings.conf
@@ +973,5 @@
>      'nativeType': 'nsScreen',
>  },
>  
>  'Selection': {
> +    'nativeType': 'mozilla::dom::Selection',

I think you can just drop this, this is the name that the WebIDL compiler picks by default.

Comment 7

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Trying a non-unified compilation:

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

And yes, I know unified compilation gives huge speed wins, and am totally in favor.  Just saying it's a hack.  :)  (I guess there's no way to magically scope "using" statements.)

Comment 8

[(no longer active)]

(In reply to comment #7)
> And yes, I know unified compilation gives huge speed wins, and am totally in
> favor.  Just saying it's a hack.  :)  (I guess there's no way to magically
> scope "using" statements.)

Not that I know of, unfortunately!

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/be6f50f0f583

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be6f50f0f583