1430450 - [Static Analysis][Logically dead code] NoteDirtyElement: Logically dead code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Sylvestre Ledru [:Sylvestre]]

Here:
https://dxr.mozilla.org/mozilla-central/source/dom/base/Element.cpp?q=if+%28%21existingRoot%29+%7B&redirect_type=single#4655
We are exiting the function when existingRoot is null.

So, the check  with existingRoot on this line: https://dxr.mozilla.org/mozilla-central/source/dom/base/Element.cpp?q=if+%28%21existingRoot%29+%7B&redirect_type=single#4665
is useless

We should remove the check line 4665 to replace it by the allocation.

Comment 1

[Bobby Holley (:bholley)]

> We should remove the check line 4665 to replace it by the allocation.

Not sure what you mean by "the allocation", but I assume you mean we should replace:

uint32_t existingBits = existingRoot ? doc->GetServoRestyleRootDirtyBits() : 0;

with

uint32_t existingBits = doc->GetServoRestyleRootDirtyBits();

Which we should do (I forgot to simplify this in bug 1398119).

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Indeed! 
I reported that as a good first bug to understand our workflows but you can take it if you want

Comment 3

[tanyavedi76]

I'm a beginner, can I take up this issue?

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Sure, just propose a patch for this!

Comment 5

[tanyavedi76]

(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Sure, just propose a patch for this!

Thanks!

Comment 6

[tanyavedi76]

I cannot find the link to the base repo on github, it would be great if someone can provide the same. Thanks!

Comment 7

[Bobby Holley (:bholley)]

(In reply to tanyavedi76 from comment #6)
> I cannot find the link to the base repo on github, it would be great if
> someone can provide the same. Thanks!

I'd recommend having a look at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

You can also find a semi-official github mirror of the firefox repository (which uses mercurial) at https://github.com/mozilla/gecko

Comment 8

[Ashish Daulatabad]

Do we've to just replace
uint32_t existingBits = existingRoot ? doc->GetServoRestyleRootDirtyBits() : 0;

with the line

uint32_t existingBits = doc->GetServoRestyleRootDirtyBits(); ??

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(In reply to Ashish Daulatabad from comment #8)
> Do we've to just replace
> uint32_t existingBits = existingRoot ? doc->GetServoRestyleRootDirtyBits() :
> 0;
> 
> with the line
> 
> uint32_t existingBits = doc->GetServoRestyleRootDirtyBits(); ??

Yes.

Comment 10

[Ashish Daulatabad]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #9)
> Yes.

I'll do it then.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Awesome, thank you!

Comment 12

[Ashish Daulatabad]

Guys... I'm finding trouble in generating the patch file. Can you please help me out?

Comment 13

[Sylvestre Ledru [:Sylvestre]]

here it is:
https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 14

[Ashish Daulatabad]

Attachment: Bug 1430450 - Removal of logically dead code.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8946313 [details] [diff] [review]
Bug 1430450 - Removal of logically dead code.

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

Looks good. You should look for a reviewer. Bobby should be enough.

Comment 16

[Ashish Daulatabad]

(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Looks good. You should look for a reviewer. Bobby should be enough.


I'm really new to this one. Can you help me??

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8946313 [details] [diff] [review]
Bug 1430450 - Removal of logically dead code.

r=me, thanks a lot!

For reference, you can ask for review in a page in the details page of the attachment, like: https://bugzilla.mozilla.org/attachment.cgi?id=8946313&action=edit

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/397ad5120bb0
Remove Logically dead code. r=emilio

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/397ad5120bb0