978544 - Ignore events targetting a nsIMozBrowserFrame in BrowserElementPanning.js

Status: RESOLVED FIXED

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug978544.patch

This patch does not process events for events targeting a MozBrowserFrame, as those will be be handle by the MozBrowserFrame instance of BrowserElementPanning.js.

Comment 2

[[:fabrice] Fabrice Desré]

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?

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 :)

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/b2g-inbound/rev/fb4819dd57ea

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fb4819dd57ea