1446568 - Move a bit more stuff from nsDocument -> nsIDocument.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Devirtualize nsIDocument::CreateElem.

Review commit: https://reviewboard.mozilla.org/r/228558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228558/

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move Reset and ResetToURI to nsIDocument instead of nsDocument.

Review commit: https://reviewboard.mozilla.org/r/228560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228560/

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Remove nsDocument::UnlockPointer.

And remove some now-unneeded casts.

Review commit: https://reviewboard.mozilla.org/r/228562/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228562/

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move the blocked tracking nodes methods to nsIDocument.

Review commit: https://reviewboard.mozilla.org/r/228564/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228564/

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Remove an unneeded cast.

Review commit: https://reviewboard.mozilla.org/r/228566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228566/

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move a few of the remaining nsDocument members to nsIDocument.

Review commit: https://reviewboard.mozilla.org/r/228568/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228568/

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move a couple more stuff from nsDocument to nsIDocument.

Review commit: https://reviewboard.mozilla.org/r/228570/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228570/

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Make nsIDocument SetContentType / SetDocumentCharacterSet not pure virtual.

No good reason for them to be.

Review commit: https://reviewboard.mozilla.org/r/228572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228572/

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Make SetScriptGlobalObject not pure virtual.

Review commit: https://reviewboard.mozilla.org/r/228574/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228574/

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move UseCounter stuff and IsSynthesized to nsIDocument.

Review commit: https://reviewboard.mozilla.org/r/228576/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228576/

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1446568: Move a couple other methods from nsDocument to nsIDocument.

Need to add an static_cast for nsIDOMDocument, but it'll be temporary, I'd
expect.

Review commit: https://reviewboard.mozilla.org/r/228578/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228578/

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959722 [details]
Bug 1446568: Devirtualize nsIDocument::CreateElem.

https://reviewboard.mozilla.org/r/228558/#review234392

(Do I recall correctly that CreateElem actually had different implementations at some point... one in Document and one in XULDocument... oh well)

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959723 [details]
Bug 1446568: Move Reset and ResetToURI to nsIDocument instead of nsDocument.

https://reviewboard.mozilla.org/r/228560/#review234394

ok, XULDocument does still override those.

::: dom/base/nsDocument.h
(Diff revision 1)
>    //
>    // This can be fixed after updating to rust 1.25 and updating bindgen to
>    // include https://github.com/rust-lang-nursery/rust-bindgen/pull/1271.
>    js::ExpandoAndGeneration mExpandoAndGeneration;
>  
> -  RefPtr<mozilla::EventListenerManager> mListenerManager;

Totally unrelated change, but fine.
Or perhaps tweak the commit message a bit to include also this change.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959724 [details]
Bug 1446568: Remove nsDocument::UnlockPointer.

https://reviewboard.mozilla.org/r/228562/#review234396

::: commit-message-2cdd8:3
(Diff revision 1)
> +Bug 1446568: Remove nsDocument::UnlockPointer. r=smaug
> +
> +And remove some now-unneeded casts.

What is this about. I see no removals

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959724 [details]
Bug 1446568: Remove nsDocument::UnlockPointer.

https://reviewboard.mozilla.org/r/228562/#review234398

(oops, nm my comment. looks fine)

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959725 [details]
Bug 1446568: Move the blocked tracking nodes methods to nsIDocument.

https://reviewboard.mozilla.org/r/228564/#review234400

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959726 [details]
Bug 1446568: Remove an unneeded cast.

https://reviewboard.mozilla.org/r/228566/#review234402

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959727 [details]
Bug 1446568: Move a few of the remaining nsDocument members to nsIDocument.

https://reviewboard.mozilla.org/r/228568/#review234404

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959728 [details]
Bug 1446568: Move a couple more stuff from nsDocument to nsIDocument.

https://reviewboard.mozilla.org/r/228570/#review234406

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959729 [details]
Bug 1446568: Make nsIDocument SetContentType / SetDocumentCharacterSet not pure virtual.

https://reviewboard.mozilla.org/r/228572/#review234408

::: dom/base/nsIDocument.h:791
(Diff revision 1)
>      return mCharacterSet;
>    }
>  
>    /**
> -   * Set the document's character encoding.
> +   * Set the document's character encoding. This will
> +   * trigger a startDocumentLoad if necessary to answer the question.

Looks like the comment in nsDocument.h is wrong, so stick with the comment nsIDocument.h already has

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959730 [details]
Bug 1446568: Make SetScriptGlobalObject not pure virtual.

https://reviewboard.mozilla.org/r/228574/#review234410

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959731 [details]
Bug 1446568: Move UseCounter stuff and IsSynthesized to nsIDocument.

https://reviewboard.mozilla.org/r/228576/#review234412

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8959732 [details]
Bug 1446568: Move a couple other methods from nsDocument to nsIDocument.

https://reviewboard.mozilla.org/r/228578/#review234414

Comment 24

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8959723 [details]
Bug 1446568: Move Reset and ResetToURI to nsIDocument instead of nsDocument.

https://reviewboard.mozilla.org/r/228560/#review234432

::: dom/base/nsDocument.h
(Diff revision 1)
>    //
>    // This can be fixed after updating to rust 1.25 and updating bindgen to
>    // include https://github.com/rust-lang-nursery/rust-bindgen/pull/1271.
>    js::ExpandoAndGeneration mExpandoAndGeneration;
>  
> -  RefPtr<mozilla::EventListenerManager> mListenerManager;

Yeah, this is used by ResetToURI, so it's needed. Will mention it in the commit message.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/71288e94a4e4
Devirtualize nsIDocument::CreateElem. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/de1ebfe4f5b9
Move Reset and ResetToURI to nsIDocument instead of nsDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/339d892ddf7f
Remove nsDocument::UnlockPointer. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/20d621bd59d5
Move the blocked tracking nodes methods to nsIDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc8a03f7e15e
Remove an unneeded cast. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0387ea3bdff
Move a few of the remaining nsDocument members to nsIDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6dec081f644
Move a couple more stuff from nsDocument to nsIDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0740c8e6d169
Make nsIDocument SetContentType / SetDocumentCharacterSet not pure virtual. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f3e5bc6ac99
Make SetScriptGlobalObject not pure virtual. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d7210ef8a29
Move UseCounter stuff and IsSynthesized to nsIDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3508bdff5806
Move a couple other methods from nsDocument to nsIDocument. r=smaug

Comment 26

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/71288e94a4e4
https://hg.mozilla.org/mozilla-central/rev/de1ebfe4f5b9
https://hg.mozilla.org/mozilla-central/rev/339d892ddf7f
https://hg.mozilla.org/mozilla-central/rev/20d621bd59d5
https://hg.mozilla.org/mozilla-central/rev/cc8a03f7e15e
https://hg.mozilla.org/mozilla-central/rev/b0387ea3bdff
https://hg.mozilla.org/mozilla-central/rev/d6dec081f644
https://hg.mozilla.org/mozilla-central/rev/0740c8e6d169
https://hg.mozilla.org/mozilla-central/rev/9f3e5bc6ac99
https://hg.mozilla.org/mozilla-central/rev/5d7210ef8a29
https://hg.mozilla.org/mozilla-central/rev/3508bdff5806