Remove some unnecessary null checks in dom/

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: poiru, Assigned: poiru)

Tracking

Trunk
mozilla45
Points:
---

Firefox Tracking Flags

(firefox44 affected, firefox45 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

3 years ago
These patches have been gathering dust for a few months. I lost a bunch of hunks because I didn't feel like resolving so many conflicts, but this is better than nothing.
(Assignee)

Comment 1

3 years ago
Created attachment 8677267 [details] [diff] [review]
Remove some unnecessary null checks in dom/base/
Attachment #8677267 - Flags: review?(n.nethercote)
(Assignee)

Comment 2

3 years ago
Created attachment 8677272 [details] [diff] [review]
Remove some unnecessary null checks in dom/xslt/
Attachment #8677272 - Flags: review?(n.nethercote)
(Assignee)

Comment 3

3 years ago
Created attachment 8677273 [details] [diff] [review]
Remove some unnecessary null checks in rest of dom/xul/
Attachment #8677273 - Flags: review?(n.nethercote)
(Assignee)

Comment 4

3 years ago
Created attachment 8677275 [details] [diff] [review]
Remove some unnecessary null checks in rest of dom/
Attachment #8677275 - Flags: review?(n.nethercote)
Attachment #8677267 - Flags: review?(n.nethercote) → review+
Comment on attachment 8677272 [details] [diff] [review]
Remove some unnecessary null checks in dom/xslt/

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

::: dom/xslt/xpath/txNodeSetAdaptor.cpp
@@ +25,5 @@
>  {
>      if (!mValue) {
>          mValue = new txNodeSet(nullptr);
>      }
> +    return NS_OK;

Bonus points if you change the return type of this function to |void| and avoid additional redundant checks up the call chain.

::: dom/xslt/xpath/txXPCOMExtensionFunction.cpp
@@ +255,5 @@
>  #ifdef TX_TO_STRING
>                                                    aName,
>  #endif
>                                                    aState);
> +    return NS_OK;

Ditto.

::: dom/xslt/xslt/txStylesheetCompiler.cpp
@@ +1086,5 @@
>      nsresult rv = findFunction(aName, aID, this, aFunction);
>      if (rv == NS_ERROR_XPATH_UNKNOWN_FUNCTION &&
>          (aID != kNameSpaceID_None || fcp())) {
>          *aFunction = new txErrorFunctionCall(aName);
> +        rv = NS_OK;

Ditto.

::: dom/xslt/xslt/txXSLTNumberCounters.cpp
@@ +97,5 @@
>          // if we don't recognize the token then use '1'
>          aCounter = new txDecimalCounter(1, aGroupSize, aGroupSeparator);
>      }
> +    MOZ_ASSERT(aCounter);
> +    return NS_OK;

Ditto.
Attachment #8677272 - Flags: review?(n.nethercote) → review+
Attachment #8677273 - Flags: review?(n.nethercote) → review+
Comment on attachment 8677275 [details] [diff] [review]
Remove some unnecessary null checks in rest of dom/

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

::: dom/plugins/base/nsPluginNativeWindow.cpp
@@ +40,5 @@
>  nsresult PLUG_NewPluginNativeWindow(nsPluginNativeWindow ** aPluginNativeWindow)
>  {
>    NS_ENSURE_ARG_POINTER(aPluginNativeWindow);
>    *aPluginNativeWindow = new nsPluginNativeWindowPLATFORM();
> +  return NS_OK;

Can you now change the return type of this function to |void|? Maybe; I haven't checked all the instances to see if any can return something other than NS_OK.
Attachment #8677275 - Flags: review?(n.nethercote) → review+
Nice changes. Thank you for persevering.

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/66eba1425dbe
https://hg.mozilla.org/mozilla-central/rev/f05d1adcc902
https://hg.mozilla.org/mozilla-central/rev/7924b7209be3
https://hg.mozilla.org/mozilla-central/rev/8f9c8e490cab
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.