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)
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.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #696985 -
Flags: review?(tnikkel)
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #696986 -
Flags: review?(tnikkel)
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #696987 -
Flags: review?(tnikkel)
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #696988 -
Flags: review?(tnikkel)
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #696989 -
Flags: review?(tnikkel)
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #696990 -
Flags: review?(tnikkel)
Updated•11 years ago
|
Attachment #696990 -
Attachment is patch: true
Comment 7•11 years ago
|
||
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.
Updated•11 years ago
|
Attachment #696986 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #696987 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #696988 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #696989 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #696990 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 8•11 years ago
|
||
(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 9•11 years ago
|
||
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+
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #697417 -
Flags: review?(tnikkel)
Assignee | ||
Comment 11•11 years ago
|
||
Attachment #697419 -
Flags: review?(tnikkel)
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #697420 -
Flags: review?(tnikkel)
Assignee | ||
Comment 13•11 years ago
|
||
Attachment #697421 -
Flags: review?(tnikkel)
Assignee | ||
Comment 14•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=9238274c76fb
Updated•11 years ago
|
Attachment #697417 -
Attachment is patch: true
Updated•11 years ago
|
Attachment #697417 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #697419 -
Flags: review?(tnikkel) → review+
Updated•11 years ago
|
Attachment #697421 -
Flags: review?(tnikkel) → review+
Comment 15•11 years ago
|
||
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+
Assignee | ||
Comment 16•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b53bba1107 https://hg.mozilla.org/integration/mozilla-inbound/rev/4e644276c092 https://hg.mozilla.org/integration/mozilla-inbound/rev/a428bfec3539 https://hg.mozilla.org/integration/mozilla-inbound/rev/638d614c55a5 https://hg.mozilla.org/integration/mozilla-inbound/rev/53707172d724 https://hg.mozilla.org/integration/mozilla-inbound/rev/dcb03b79fd15 https://hg.mozilla.org/integration/mozilla-inbound/rev/5f2b030f0b74 https://hg.mozilla.org/integration/mozilla-inbound/rev/fbcf7d62c988 https://hg.mozilla.org/integration/mozilla-inbound/rev/504d70c1ca9c https://hg.mozilla.org/integration/mozilla-inbound/rev/042de8e1b27c
Comment 17•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b4b53bba1107 https://hg.mozilla.org/mozilla-central/rev/4e644276c092 https://hg.mozilla.org/mozilla-central/rev/a428bfec3539 https://hg.mozilla.org/mozilla-central/rev/638d614c55a5 https://hg.mozilla.org/mozilla-central/rev/53707172d724 https://hg.mozilla.org/mozilla-central/rev/dcb03b79fd15 https://hg.mozilla.org/mozilla-central/rev/5f2b030f0b74 https://hg.mozilla.org/mozilla-central/rev/fbcf7d62c988 https://hg.mozilla.org/mozilla-central/rev/504d70c1ca9c https://hg.mozilla.org/mozilla-central/rev/042de8e1b27c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•