Use BaseRect methods instead of directly member variables in xpfe/

RESOLVED FIXED in Firefox 59

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: milan, Assigned: milan)

Tracking

(Blocks: 1 bug)

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

Comment hidden (empty)
Assignee: nobody → milan
Blocks: 1386277
Comment hidden (mozreview-request)
Priority: -- → P3
Whiteboard: [gfx-noted]

Comment 2

a year ago
mozreview-review
Comment on attachment 8934947 [details]
Bug 1423556: Use BaseRect access methods instead of member variables in xpfe/

https://reviewboard.mozilla.org/r/205892/#review211630

r=me with the nits below addressed.

::: xpfe/appshell/nsWebShellWindow.cpp:132
(Diff revision 1)
>    nsCOMPtr<nsIBaseWindow> base(do_QueryInterface(aOpener));
>    if (base) {
> -    rv = base->GetPositionAndSize(&mOpenerScreenRect.x,
> -                                  &mOpenerScreenRect.y,
> -                                  &mOpenerScreenRect.width,
> -                                  &mOpenerScreenRect.height);
> +    auto openerScreenRectX = mOpenerScreenRect.X();
> +    auto openerScreenRectY = mOpenerScreenRect.Y();
> +    auto openerScreenRectW = mOpenerScreenRect.Width();
> +    auto openerScreenRectH = mOpenerScreenRect.Height();

Please use int32_t instead of `auto` here.

Also, the parameters to GetPositionAndSize() are all outparams, so there should be no need to initialize openerScreenRectX et al.

Finally, you could choose shorter names given that these are local variables.

All up, these changes would result in something like this:

> int32_t x, y, width, height;
> rv = base->GetPositionAndSize(&x, &y, &width, &height);
> mOpenerScreenRect.SetRect(x, y, width, height);
Attachment #8934947 - Flags: review?(n.nethercote) → review+

Comment 3

a year ago
mozreview-review
Comment on attachment 8934947 [details]
Bug 1423556: Use BaseRect access methods instead of member variables in xpfe/

https://reviewboard.mozilla.org/r/205892/#review211632

::: xpfe/appshell/nsWebShellWindow.cpp:132
(Diff revision 1)
>    nsCOMPtr<nsIBaseWindow> base(do_QueryInterface(aOpener));
>    if (base) {
> -    rv = base->GetPositionAndSize(&mOpenerScreenRect.x,
> -                                  &mOpenerScreenRect.y,
> -                                  &mOpenerScreenRect.width,
> -                                  &mOpenerScreenRect.height);
> +    auto openerScreenRectX = mOpenerScreenRect.X();
> +    auto openerScreenRectY = mOpenerScreenRect.Y();
> +    auto openerScreenRectW = mOpenerScreenRect.Width();
> +    auto openerScreenRectH = mOpenerScreenRect.Height();

Actually, the mOpenerScreenRect.SetRect() call needs to be moved after the `rv` check.
Blocks: 1423919
No longer blocks: 1386277
Comment hidden (mozreview-request)

Comment 6

a year ago
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3d8b1409f0fe
Use BaseRect access methods instead of member variables in xpfe/ r=njn

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3d8b1409f0fe
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.