Closed
Bug 703260
Opened 13 years ago
Closed 13 years ago
Reduce view usage in event handling
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: enndeakin, Assigned: enndeakin)
References
Details
Attachments
(3 files)
(deleted),
patch
|
smaug
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
Attached will be a few miscellaneous patches which remove some view usage from event handling / presshell / popup manager.
Assignee | ||
Comment 1•13 years ago
|
||
This patch:
- nsEventStateManager methods don't use the passed in view
- change GetClientData to GetFrame as it only ever gets set to a frame.
- pass the view's frame to nsPresShell::HandleEvent instead
Could break these out into separate patches if desired.
This passes all the tests, but am asking for sr because I'm not sure if I've missed some special event case that wants a view
Attachment #575179 -
Flags: superreview?(roc)
Attachment #575179 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #575179 -
Flags: review? → review?(bugs)
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #575180 -
Flags: review?(matspal)
Assignee | ||
Comment 3•13 years ago
|
||
Only presshell implements it so just move the methods into nsIPresShell.
Attachment #575181 -
Flags: review?(matspal)
Updated•13 years ago
|
Attachment #575179 -
Attachment is patch: true
Comment 4•13 years ago
|
||
Comment on attachment 575179 [details] [diff] [review]
Patch 1 - remove view from HandleEvent
Nice.
I think you could now get rid of nsWeakView.
Attachment #575179 -
Flags: review?(bugs) → review+
Comment on attachment 575179 [details] [diff] [review]
Patch 1 - remove view from HandleEvent
Review of attachment 575179 [details] [diff] [review]:
-----------------------------------------------------------------
::: view/public/nsIView.h
@@ +251,5 @@
> mNextSibling = reinterpret_cast<nsView*>(aSibling);
> }
>
> /**
> + * Set the view's root frame.
It might not be a root frame, so fix the comment
@@ +258,3 @@
>
> /**
> + * Retrieve the view's root frame.
Ditto
::: view/public/nsIViewObserver.h
@@ +95,5 @@
> * @param aHandled - whether the correct frame was found to
> * handle the event
> * @return error status
> */
> + NS_IMETHOD HandleEvent(nsIFrame* aRootFrame,
Again, not necessarily a root frame.
Attachment #575179 -
Flags: superreview?(roc) → superreview+
Updated•13 years ago
|
Attachment #575180 -
Flags: review?(matspal) → review+
Comment 6•13 years ago
|
||
Comment on attachment 575181 [details] [diff] [review]
Patch 3 - remove nsIViewObserver
In view/src/nsViewManager.cpp
> case NS_SYSCOLORCHANGED:
> {
> // Hold a refcount to the observer. The continued existence of the observer will
> // delay deletion of this view hierarchy should the event want to cause its
> // destruction in, say, some JavaScript event handler.
>- nsCOMPtr<nsIViewObserver> obs = GetViewObserver();
>- if (obs) {
>- obs->HandleEvent(aView->GetFrame(), aEvent, false, aStatus);
>+ if (mPresShell) {
>+ mPresShell->HandleEvent(aView->GetFrame(), aEvent, false, aStatus);
This needs to hold a strong ref on mPresShell before calling HandleEvent.
And s/observer/pres shell/ in the comment?
Attachment #575181 -
Flags: review?(matspal) → review+
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to Mats Palmgren [:mats] from comment #6)
> This needs to hold a strong ref on mPresShell before calling HandleEvent.
> And s/observer/pres shell/ in the comment?
I'll change this, but the presshell/prescontext just fires off a nsIRunnable so it shouldn't matter.
Comment 8•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9298bebc43bf
https://hg.mozilla.org/mozilla-central/rev/e4a32b4ffd93
https://hg.mozilla.org/mozilla-central/rev/7e2695c9d94e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Comment 9•13 years ago
|
||
Comment on attachment 575181 [details] [diff] [review]
Patch 3 - remove nsIViewObserver
>--- a/layout/base/nsPresShell.h
>+++ b/layout/base/nsPresShell.h
>@@ -309,40 +308,35 @@ public:
> virtual void SetIgnoreViewportScrolling(bool aIgnore);
>
> virtual void SetDisplayPort(const nsRect& aDisplayPort);
>
> virtual nsresult SetResolution(float aXResolution, float aYResolution);
>
> //nsIViewObserver interface
Missed one :)
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•