Closed Bug 825866 Opened 11 years ago Closed 11 years ago

Merge nsView into nsIView and clean it up a bit

Categories

(Core :: Layout, defect)

18 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: roc, Assigned: roc)

Details

Attachments

(10 files)

81.79 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
2.39 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
2.47 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.44 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.08 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
2.22 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
17.35 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
10.59 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
227.55 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
1.18 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
There's a lot of cruft here. This bug is about removing some of it. In particular we can remove nsIView's QueryInterface and IID, which will avoid having to rev the IID in future cleanup/removal of nsIView.
Attachment #696990 - Attachment is patch: true
Comment on attachment 696985 [details] [diff] [review]
Part 1: Merge nsView into nsIView, making all references to nsView refer to nsIView instead

Can we make as many functions as possible private in nsIView? Most of the ones added from nsView could be private, no?

It would be nice if we didn't have both nsView and nsIView floating around, even if it's just a filename (nsView.cpp).

>diff --git a/view/public/nsIView.h b/view/public/nsIView.h
>+  // These are defined exactly the same in nsIView, but for now they have to be redeclared
>+  // here because of stupid C++ method hiding rules

I think this comment needs to be changed, are we re-declaring anything?

> nsViewManager::CreateView(const nsRect& aBounds,
>-    v->SetParent(static_cast<nsView*>(const_cast<nsIView*>(aParent)));
>+    v->SetParent(static_cast<nsIView*>(const_cast<nsIView*>(aParent)));

Extra unneeded cast here.

> NS_IMETHODIMP nsViewManager::SetRootView(nsIView *aView)
> {
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

You don't need this cast, it's already the right type.

> NS_IMETHODIMP nsViewManager::InvalidateViewNoSuppression(nsIView *aView,
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

And here too.

>-  nsView* displayRoot = static_cast<nsView*>(GetDisplayRootFor(view));
>-  nsViewManager* displayRootVM = displayRoot->GetViewManager();
>+  nsIView* displayRoot = static_cast<nsIView*>(GetDisplayRootFor(view));
>+  nsViewManager* displayRootVM = displayRoot->GetViewManagerInternal();

And here.

>@@ -807,18 +807,18 @@ void nsViewManager::ReparentChildWidgets
>-  nsView* view = static_cast<nsView*>(aView);
>-  for (nsView *kid = view->GetFirstChild(); kid; kid = kid->GetNextSibling()) {
>+  nsIView* view = static_cast<nsIView*>(aView);
>+  for (nsIView *kid = view->GetFirstChild(); kid; kid = kid->GetNextSibling()) {

And here.

>@@ -827,33 +827,33 @@ void nsViewManager::ReparentWidgets(nsIV
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

And here.

> NS_IMETHODIMP nsViewManager::InsertChild(nsIView *aParent, nsIView *aChild, nsIView *aSibling,
>-  nsView* parent = static_cast<nsView*>(aParent);
>-  nsView* child = static_cast<nsView*>(aChild);
>-  nsView* sibling = static_cast<nsView*>(aSibling);
>+  nsIView* parent = static_cast<nsIView*>(aParent);
>+  nsIView* child = static_cast<nsIView*>(aChild);
>+  nsIView* sibling = static_cast<nsIView*>(aSibling);

Here.

> NS_IMETHODIMP nsViewManager::RemoveChild(nsIView *aChild)
>-  nsView* child = static_cast<nsView*>(aChild);
>+  nsIView* child = static_cast<nsIView*>(aChild);

Here.

> NS_IMETHODIMP nsViewManager::MoveViewTo(nsIView *aView, nscoord aX, nscoord aY)
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

Here.

> NS_IMETHODIMP nsViewManager::ResizeView(nsIView *aView, const nsRect &aRect, bool aRepaintExposedAreaOnly)
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

Here.

> NS_IMETHODIMP nsViewManager::SetViewFloating(nsIView *aView, bool aFloating)
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

Here.

> NS_IMETHODIMP nsViewManager::SetViewVisibility(nsIView *aView, nsViewVisibility aVisible)
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

Here.

> NS_IMETHODIMP nsViewManager::SetViewZIndex(nsIView *aView, bool aAutoZIndex, int32_t aZIndex, bool aTopMost)
>-  nsView* view = static_cast<nsView*>(aView);
>+  nsIView* view = static_cast<nsIView*>(aView);

Here.
Attachment #696986 - Flags: review?(tnikkel) → review+
Attachment #696987 - Flags: review?(tnikkel) → review+
Attachment #696988 - Flags: review?(tnikkel) → review+
Attachment #696989 - Flags: review?(tnikkel) → review+
Attachment #696990 - Flags: review?(tnikkel) → review+
(In reply to Timothy Nikkel (:tn) from comment #7)
> Can we make as many functions as possible private in nsIView? Most of the
> ones added from nsView could be private, no?

I'll do that in a followup patch.

> It would be nice if we didn't have both nsView and nsIView floating around,
> even if it's just a filename (nsView.cpp).

I'll do a mass rename of nsIView to nsView in a followup patch.

> >diff --git a/view/public/nsIView.h b/view/public/nsIView.h
> >+  // These are defined exactly the same in nsIView, but for now they have to be redeclared
> >+  // here because of stupid C++ method hiding rules
> 
> I think this comment needs to be changed, are we re-declaring anything?

Fixed.

I'll attach another followup patch to remove the unnecessary casts.
Comment on attachment 696985 [details] [diff] [review]
Part 1: Merge nsView into nsIView, making all references to nsView refer to nsIView instead

Sounds good.
Attachment #696985 - Flags: review?(tnikkel) → review+
Attachment #697417 - Attachment is patch: true
Attachment #697417 - Flags: review?(tnikkel) → review+
Attachment #697419 - Flags: review?(tnikkel) → review+
Attachment #697421 - Flags: review?(tnikkel) → review+
Comment on attachment 697420 [details] [diff] [review]
Part 8:  Mass-rename of nsIView to nsView

> diff --git a/docshell/base/nsIContentViewer.idl b/docshell/base/nsIContentViewer.idl
> -[ptr] native nsIViewPtr(nsIView);
> +[ptr] native nsIViewPtr(nsView);

nsViewPtr.
Attachment #697420 - Flags: review?(tnikkel) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: