826632 - Merge nsIViewManager into nsViewManager and clean up a bit

Status: RESOLVED FIXED

(Core :: Layout, defect)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1: Merge nsIViewManager into nsViewManager

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 2: Use nsRefPtr<nsViewManager> instead of nsCOMPtr

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 3: Create nsViewManagers directly instead of using XPCOM

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 4: nsViewManager does not need to implement nsISupports anymore

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 5: nsViewManager methods do not need to be virtual and most of the ones that return nsresult don't need to

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 6: Remove some unnecessary #includes from nsView.h and nsViewManager.h

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 7: nsView::GetViewManagerInternal is no longer relevant, so remove it

Comment 8

[Timothy Nikkel (:tnikkel)]

Comment on attachment 697850 [details] [diff] [review]
Part 5: nsViewManager methods do not need to be virtual and most of the ones that return nsresult don't need to

>-NS_IMETHODIMP nsViewManager::RemoveChild(nsView *aChild)
>+void
>+nsViewManager::RemoveChild(nsView *aChild)
> {
>-  NS_ENSURE_ARG_POINTER(aChild);
>+  NS_ASSERTION(aChild, "aChild must not be null");

NS_ENSURE_ARG_POINTER returns if the argument is null, so this is a change of behaviour. Intentional?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Timothy Nikkel (:tn) from comment #8)
> NS_ENSURE_ARG_POINTER returns if the argument is null, so this is a change
> of behaviour. Intentional?

Yes. It doesn't matter.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/20df426b6111
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6f3da960161
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fa476152adc
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5d5f0c454c
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c2fc9e57bb5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9a6a2f984f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/0df74b1a4543

Comment 11

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ea909ce6007b for bustage.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=8103d9167be4

So part 1 did not cause the test failures, but the patches up to part 7 did.

Let's try up to part 4: https://tbpl.mozilla.org/?tree=Try&rev=32f1edb2e710

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think I found the bug; in part 5, in nsViewManager::DispatchEvent, I removed a return statement that I shouldn't have.
https://tbpl.mozilla.org/?tree=Try&rev=a29c06d23a80

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1367089491f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b31621af158
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e7027c6242a
https://hg.mozilla.org/integration/mozilla-inbound/rev/92f2f1a27270
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b3fbf63dd27
https://hg.mozilla.org/integration/mozilla-inbound/rev/57f386239178
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb1092d8c447

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1367089491f9
https://hg.mozilla.org/mozilla-central/rev/7b31621af158
https://hg.mozilla.org/mozilla-central/rev/9e7027c6242a
https://hg.mozilla.org/mozilla-central/rev/92f2f1a27270
https://hg.mozilla.org/mozilla-central/rev/0b3fbf63dd27
https://hg.mozilla.org/mozilla-central/rev/57f386239178
https://hg.mozilla.org/mozilla-central/rev/cb1092d8c447