Closed Bug 1423556 Opened 6 years ago Closed 6 years ago

Use BaseRect methods instead of directly member variables in xpfe/

Categories

(Core :: Graphics, enhancement, P3)

Product:
Core
Component:
Graphics
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: milan, Assigned: milan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

Bug 1423556: Use BaseRect access methods instead of member variables in xpfe/
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details 
      No description provided.
Assignee: nobody → milan
Blocks: 1386277
Priority: -- → P3
Whiteboard: [gfx-noted]
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 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
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
https://hg.mozilla.org/mozilla-central/rev/3d8b1409f0fe
Status: NEW → RESOLVED
Closed: 6 years 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.