Closed Bug 1299335 Opened 4 years ago Closed 4 years ago

DeCOMtaminate nsIWidget even more

Categories

(Core :: Widget, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 --- wontfix
firefox53 --- fixed

People

(Reporter: njn, Assigned: njn)

References

(Blocks 1 open bug)

Details

(Whiteboard: tpi:+)

Attachments

(8 files, 1 obsolete file)

Continuing on from bug 1296993.
Priority: -- → P2
Whiteboard: tpi:+
Priority: P2 → P3
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.
Attachment #8819714 - Flags: review?(jmathies)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
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.
Attachment #8819715 - Flags: review?(mstange)
This patch removes its return value, because none of the call sites check it
except for one non-vital assertion.
Attachment #8819716 - Flags: review?(mstange)
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.
Attachment #8819717 - Flags: review?(mstange)
This patch changes it from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|.
Attachment #8819718 - Flags: review?(mstange)
This patch changes them from |NS_IMETHOD| to |virtual void| because the return
values are never used.
Attachment #8819734 - Flags: review?(mstange)
This patch changes them from |NS_IMETHOD| to |virtual void| because the return
values are never used.
Attachment #8819816 - Flags: review?(mstange)
This patch changes them from |NS_IMETHOD| to |virtual void| because every
implementation of these functions always returns |NS_OK|.
Attachment #8819817 - Flags: review?(mstange)
This patch changes it from |NS_IMETHOD| to |virtual void| because every
implementation of these functions always returns |NS_OK|.
Attachment #8819818 - Flags: review?(mstange)
Attachment #8819816 - Attachment is obsolete: true
Attachment #8819816 - Flags: review?(mstange)
Attachment #8819715 - Flags: review?(mstange) → review+
Attachment #8819716 - Flags: review?(mstange) → review+
Attachment #8819717 - Flags: review?(mstange) → review+
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
Attachment #8819718 - Flags: review?(mstange) → review+
Attachment #8819734 - Flags: review?(mstange) → review+
Attachment #8819817 - Flags: review?(mstange) → review+
Attachment #8819818 - Flags: review?(mstange) → review+
> (part 5) - Streamline nsIWidget::StartPluginIME
> 
> Is there a caller that checks the result?

Good question. There is one checked call, in nsPluginInstanceOwner::ProcessEvent().
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
Attachment #8819714 - Flags: review?(jmathies) → review+
Pushed by nnethercote@mozilla.com:
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.
Blocks: 1325234
We've built 51 RC. Mark 51 won't fix.
You need to log in before you can comment on or make changes to this bug.