Closed Bug 978544 Opened 7 years ago Closed 7 years ago

Ignore events targetting a nsIMozBrowserFrame in BrowserElementPanning.js

Categories

(Core :: DOM: Device Interfaces, defect, P1)

defect

Tracking

()

RESOLVED FIXED
1.4 S3 (14mar)

People

(Reporter: vingtetun, Assigned: vingtetun)

References

Details

(Keywords: perf, Whiteboard: [c=handeye p= s=2014.03.14 u=])

Attachments

(1 file)

When an event targets a MozBrowserFrame it will likely be handle by the BrowserElementPanning.js instance of that element. Currently I see BrowserElementPanning.js of the master process while profiling panning of apps.
Attached patch bug978544.patchSplinter Review
This patch does not process events for events targeting a MozBrowserFrame, as those will be be handle by the MozBrowserFrame instance of BrowserElementPanning.js.
Attachment #8384239 - Flags: review?(fabrice)
Keywords: perf
Priority: -- → P1
Whiteboard: [c=handeye p= s= u=]
Target Milestone: --- → 1.4 S3 (14mar)
Comment on attachment 8384239 [details] [diff] [review]
bug978544.patch

Review of attachment 8384239 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/chrome/content/content.css
@@ +305,5 @@
>  %endif
> +
> +* {
> + text-rendering: optimizeSpeed;
> +}

left over from another patch I guess ;)

::: dom/browser-element/BrowserElementPanning.js
@@ +77,5 @@
>    handleEvent: function cp_handleEvent(evt) {
> +    // Ignore events targeting a mozbrowser iframe.
> +    try {
> +      evt.target.QueryInterface(Ci.nsIMozBrowserFrame);
> +      return;

would target instanceof Ci.nsIMozBrowserFrame work and be a tad faster?
Attachment #8384239 - Flags: review?(fabrice) → review+
(In reply to Fabrice Desré [:fabrice] from comment #2)
> Comment on attachment 8384239 [details] [diff] [review]
> bug978544.patch
> 
> Review of attachment 8384239 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/chrome/content/content.css
> @@ +305,5 @@
> >  %endif
> > +
> > +* {
> > + text-rendering: optimizeSpeed;
> > +}
> 
> left over from another patch I guess ;)
> 

Oups!

> ::: dom/browser-element/BrowserElementPanning.js
> @@ +77,5 @@
> >    handleEvent: function cp_handleEvent(evt) {
> > +    // Ignore events targeting a mozbrowser iframe.
> > +    try {
> > +      evt.target.QueryInterface(Ci.nsIMozBrowserFrame);
> > +      return;
> 
> would target instanceof Ci.nsIMozBrowserFrame work and be a tad faster?

It works :)
Assignee: nobody → 21
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/fb4819dd57ea
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [c=handeye p= s= u=] → [c=handeye p= s=2014.03.14 u=]
You need to log in before you can comment on or make changes to this bug.