1299335 - DeCOMtaminate nsIWidget even more

Status: RESOLVED FIXED

(Core :: Widget, defect, P3)

Description

[Nicholas Nethercote [inactive]]

Continuing on from bug 1296993.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Change some nsIWidget function return values from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|

Specifically: OnDefaultButtonLoaded, AttachNativeKeyEvent, BeginMoveDrag,
BeginResizeDrag, GetAttention. These are all fallible functions whose result is
always checked.

The patch also moves some trivial function definitions from nsBaseWidget.cpp to
nsBaseWidget.h, and removes the android BeginResizeDrag() because it can
use the nsBaseWidget one.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Streamline nsIWidget::SetIcon

This patch does the following.

- Removes the return value, because none of the call sites check it.

- Removes the empty implementations from several nsIWidget instances, because
  they can use the nsBaseWidget one.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Streamline nsIWidget::SetParent

This patch removes its return value, because none of the call sites check it
except for one non-vital assertion.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Streamline nsIWidget::HideWindowChrome

This patch does the following.

- Removes the return value, because none of the call sites check it.

- Removes the empty implementations from the android nsIWidget instance,
  because it can use the nsBaseWidget one.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Streamline nsIWidget::StartPluginIME

This patch changes it from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Streamline nsIWidget::{Move,Resize}Client()

This patch changes them from |NS_IMETHOD| to |virtual void| because the return
values are never used.

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Streamline nsIWidget::{Move,Resize}Client()

This patch changes them from |NS_IMETHOD| to |virtual void| because the return
values are never used.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: (part 7) - Streamline nsIWidget::{Move,Resize}

This patch changes them from |NS_IMETHOD| to |virtual void| because every
implementation of these functions always returns |NS_OK|.

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: (part 8) - Streamline nsIWidget::Enable

This patch changes it from |NS_IMETHOD| to |virtual void| because every
implementation of these functions always returns |NS_OK|.

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 8819718 [details] [diff] [review]
(part 5) - Streamline nsIWidget::StartPluginIME

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

Is there a caller that checks the result? If not, why did you choose to make this one MOZ_MUST_USE nsresult instead of void?

r=me either way, it's an improvement regardless

Comment 11

[Nicholas Nethercote [inactive]]

> (part 5) - Streamline nsIWidget::StartPluginIME
> 
> Is there a caller that checks the result?

Good question. There is one checked call, in nsPluginInstanceOwner::ProcessEvent().

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 8819714 [details] [diff] [review]
(part 1) - Change some nsIWidget function return values from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|

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

lgtm

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/87e9a25f3d53
(part 1) - Change some nsIWidget function return values from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|. r=jimm.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a75e4a674e23
(part 2) - Streamline nsIWidget::SetIcon. r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe8165b5dbe9
(part 3) - Streamline nsIWidget::SetParent. r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/19292462b6f5
(part 4) - Streamline nsIWidget::HideWindowChrome. r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e0238046725
(part 5) - Streamline nsIWidget::StartPluginIME. r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf2c019eaf93
(part 6) - Streamline nsIWidget::{Move,Resize}Client(). r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/1da69ae6f724
(part 7) - Streamline nsIWidget::{Move,Resize}. r=mstange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0922b81af274
(part 8) - Streamline nsIWidget::Enable. r=mstange.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/87e9a25f3d53
https://hg.mozilla.org/mozilla-central/rev/a75e4a674e23
https://hg.mozilla.org/mozilla-central/rev/fe8165b5dbe9
https://hg.mozilla.org/mozilla-central/rev/19292462b6f5
https://hg.mozilla.org/mozilla-central/rev/6e0238046725
https://hg.mozilla.org/mozilla-central/rev/bf2c019eaf93
https://hg.mozilla.org/mozilla-central/rev/1da69ae6f724
https://hg.mozilla.org/mozilla-central/rev/0922b81af274

Comment 15

[Gerry Chang [:gchang]]

We've built 51 RC. Mark 51 won't fix.