890938 - Convert AsyncPanZoomController::ZoomToRect to take a CSSRect instead of a gfxRect

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

This should be a decent first bug for Botond. Pretty straightforward - convert APZC::ZoomToRect to take a CSSRect and update callers and so on. See http://mxr.mozilla.org/mozilla-central/ident?i=ZoomToRect&filter= to quickly find places this is called/used so that the appropriate places can be patched. Note that MXR includes false positives (unrelated ZoomToRect identifiers) so you'll have to be careful. Try to propagate the CSSRect as far as it'll go, which should be to TabChild.cpp.

Comment 1

[Botond Ballo [:botond]]

Attachment: patch

Comment 2

[Kartikaya Gupta (email:[e-mail])]

While waiting for Botond to get his L1 commit access, I pushed this to try since it exercises mostly B2G code paths:

https://tbpl.mozilla.org/?tree=Try&rev=d828c691a073

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 772714 [details] [diff] [review]
patch

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

Noticed a style issue (see below) but that should be easily fixable. Please upload a new patch with the comment below addressed and "r=kats" appended to the commit message. The try run is looking good so far and I don't anticipate any problems so we should be able to flag this bug for checkin once the new patch is up.

::: gfx/layers/ipc/AsyncPanZoomController.cpp
@@ +1272,5 @@
>  void AsyncPanZoomController::DetectScrollableSubframe() {
>    mDelayPanning = true;
>  }
>  
> +void AsyncPanZoomController::ZoomToRect(CSSRect zoomToRect) {

In our coding style, arguments to functions should start with "a", so this should be "aZoomToRect" (or just aRect if you don't want to change the name from before).

::: gfx/layers/ipc/AsyncPanZoomController.h
@@ +136,2 @@
>     */
> +  void ZoomToRect(CSSRect zoomToRect);

Ditto here.

Comment 4

[Botond Ballo [:botond]]

Attachment: updated patch

Comment 5

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Comment on attachment 772714 [details] [diff] [review]
> patch
> 
> Review of attachment 772714 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Noticed a style issue (see below) but that should be easily fixable. Please
> upload a new patch with the comment below addressed and "r=kats" appended to
> the commit message. The try run is looking good so far and I don't
> anticipate any problems so we should be able to flag this bug for checkin
> once the new patch is up.
> 
> ::: gfx/layers/ipc/AsyncPanZoomController.cpp
> @@ +1272,5 @@
> >  void AsyncPanZoomController::DetectScrollableSubframe() {
> >    mDelayPanning = true;
> >  }
> >  
> > +void AsyncPanZoomController::ZoomToRect(CSSRect zoomToRect) {
> 
> In our coding style, arguments to functions should start with "a", so this
> should be "aZoomToRect" (or just aRect if you don't want to change the name
> from before).
> 
> ::: gfx/layers/ipc/AsyncPanZoomController.h
> @@ +136,2 @@
> >     */
> > +  void ZoomToRect(CSSRect zoomToRect);
> 
> Ditto here.

Done.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 772895 [details] [diff] [review]
updated patch

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

Perfect, thanks!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3265d1e6ed10

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Backed out for Werror bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/86e71d868f45

https://tbpl.mozilla.org/php/getParsedLog.php?id=25122609&tree=Mozilla-Inbound

Comment 9

[Botond Ballo [:botond]]

Attachment: updated patch to fix Werror bustage

Fixed in updated patch.

Comment 10

[Botond Ballo [:botond]]

Try results: https://tbpl.mozilla.org/?tree=Try&rev=fe91a2c7773e

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b99aac5ea46

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b99aac5ea46