DeCOMtaminate nsIWidget for the last time

RESOLVED FIXED in Firefox 53

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks 1 bug)

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(10 attachments)

5.56 KB, patch
jimm
: review+
Details | Diff | Splinter Review
21.40 KB, patch
jimm
: review+
Details | Diff | Splinter Review
23.24 KB, patch
mstange
: review+
Details | Diff | Splinter Review
23.97 KB, patch
mstange
: review+
Details | Diff | Splinter Review
19.14 KB, patch
jimm
: review+
Details | Diff | Splinter Review
14.39 KB, patch
jimm
: review+
Details | Diff | Splinter Review
2.83 KB, patch
mstange
: review+
Details | Diff | Splinter Review
16.29 KB, patch
jimm
: review+
Details | Diff | Splinter Review
34.41 KB, patch
mstange
: review+
Details | Diff | Splinter Review
10.69 KB, patch
mstange
: review+
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
Continuing on from bug 1299335.
(Assignee)

Comment 1

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual nsresult|. The callsites
were a mix of checked and unchecked so using |MOZ_MUST_USE| didn't feel
appropriate.
Attachment #8820952 - Flags: review?(jmathies)
(Assignee)

Comment 2

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual void|. The return value was
only checked in one low-value assertion so I decided it wasn't needed.
Attachment #8820954 - Flags: review?(jmathies)
(Assignee)

Comment 3

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual void|. The return value was
only checked in one low-value assertion and one other place where the check had
no useful effect (nsCocoaWindow::HideWindowChrome()).
Attachment #8820955 - Flags: review?(mstange)
(Assignee)

Comment 4

2 years ago
This patch changes them from |NS_IMETHOD| to |virtual nsresult|.
Attachment #8820956 - Flags: review?(mstange)
(Assignee)

Comment 5

2 years ago
This patch changes one from |NS_IMETHOD| to |virtual nsresult| and the other to
|virtual void|.
Attachment #8820959 - Flags: review?(jmathies)
(Assignee)

Comment 6

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual nsresult| because some call
sites are checked and others aren't.
Attachment #8820960 - Flags: review?(jmathies)
(Assignee)

Comment 7

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual MOZ_MUST_USE nsresult|.
Attachment #8820962 - Flags: review?(mstange)
(Assignee)

Comment 8

2 years ago
This patch changes it from |NS_IMETHOD| to |virtual nsresult| because some
callsites are checked and some aren't.
Attachment #8820963 - Flags: review?(jmathies)
(Assignee)

Comment 9

2 years ago
|virtual T| is clearer than |NS_IMETHOD_(T)|.
Attachment #8820964 - Flags: review?(mstange)
(Assignee)

Comment 10

2 years ago
This patch converts some NS_IMETHOD and NS_IMETHODIMP occurrences that I missed
in previous bugs.

The patch also removes the Android
nsWindow::{Get,Set}HasTransparentBackground() functions because they're unused.
Attachment #8820965 - Flags: review?(mstange)
Attachment #8820955 - Flags: review?(mstange) → review+
Attachment #8820956 - Flags: review?(mstange) → review+
Attachment #8820962 - Flags: review?(mstange) → review+
Comment on attachment 8820964 [details] [diff] [review]
(part 9) - Remove remaining NS_IMETHOD_ occurrences from nsIWidget

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

I had no idea NS_IMETHOD_(T) even existed. That's wild.
Attachment #8820964 - Flags: review?(mstange) → review+
Attachment #8820965 - Flags: review?(mstange) → review+

Updated

2 years ago
Attachment #8820952 - Flags: review?(jmathies) → review+

Updated

2 years ago
Attachment #8820954 - Flags: review?(jmathies) → review+

Updated

2 years ago
Attachment #8820959 - Flags: review?(jmathies) → review+

Updated

2 years ago
Attachment #8820960 - Flags: review?(jmathies) → review+
Comment on attachment 8820963 [details] [diff] [review]
(part 8) - Streamline nsIWidget::DispatchEvent

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

::: widget/cocoa/nsCocoaWindow.mm
@@ +1887,5 @@
>    return NS_OK;
>  }
>  
>  // Invokes callback and ProcessEvent methods on Event Listener object
> +nsresult 

nit - trailing whitespace
Attachment #8820963 - Flags: review?(jmathies) → review+

Updated

2 years ago
Whiteboard: tpi:+
(Assignee)

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9fc4ad67f3ddcd876962cb9fa8f4f950b5abeec
Bug 1325234 (part 1) - Streamline nsIWidget::NotifyIME. r=jimm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8da1583fcd48bc11ff425300a31ed220b96fb415
Bug 1325234 (part 2) - Streamline nsIWidget::Invalidate(). r=jimm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4181e317c8df04f18dd85eac8d8922b1d4e41f6
Bug 1325234 (part 3) - Streamline nsIWidget::Show(). r=mstange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/445cae8045dec19f197ea08c80f6e4d17fc336d7
Bug 1325234 (part 4) - Streamline nsIWidget::{SetFocus,SetNonClientMargins}(). r=mstange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/75544504b0012d30bbee73fd73fea1ac5c85de03
Bug 1325234 (part 5) - Streamline nsIWidget::SetCursor (both versions). r=jimm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/407b4fc54cd5009d3c434565b7a249390c7c5005
Bug 1325234 (part 6) - Streamline nsIWidget::SetTitle. r=jimm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/48f5a494d7edf85ee57f329ee75d83d874042294
Bug 1325234 (part 7) - Streamline nsIWidget::GetSelectionAsPlaintext. r=mstange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad2e4c9148888815661208134b21bd22f34986b
Bug 1325234 (part 8) - Streamline nsIWidget::DispatchEvent. r=jimm.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e51e167058e06429deb642480c0c5691715601e
Bug 1325234 (part 9) - Remove remaining NS_IMETHOD_ occurrences from nsIWidget. r=mstange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c419cd68dcc95fb63f43bcaf117995ae77f638f
Bug 1325234 (part 10) - Final nsIWidget deCOMtamination clean-ups. r=mstange.
You need to log in before you can comment on or make changes to this bug.