Closed Bug 1430450 Opened 2 years ago Closed 2 years ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: Sylvestre, Assigned: supersonic12910, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [good first bug][lang=C++][CID 1427555])

Attachments

(1 file)

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.
> 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).
Indeed! 
I reported that as a good first bug to understand our workflows but you can take it if you want
I'm a beginner, can I take up this issue?
Sure, just propose a patch for this!
(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Sure, just propose a patch for this!

Thanks!
I cannot find the link to the base repo on github, it would be great if someone can provide the same. Thanks!
(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
Priority: -- → P3
Do we've to just replace
uint32_t existingBits = existingRoot ? doc->GetServoRestyleRootDirtyBits() : 0;

with the line

uint32_t existingBits = doc->GetServoRestyleRootDirtyBits(); ??
(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.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #9)
> Yes.

I'll do it then.
Awesome, thank you!
Assignee: nobody → supersonic12910
Guys... I'm finding trouble in generating the patch file. Can you please help me out?
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.
(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 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
Attachment #8946313 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/397ad5120bb0
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.