1691453 - No need to check ObservesTypeOfEntry again in PerformanceObserver::QueueEntry

Status: RESOLVED FIXED

(Core :: Performance, enhancement)

Description

[Sean Feng [:sefeng]]

https://searchfox.org/mozilla-central/rev/d537e47349944c0fbd0100bd52c30e493e748c2e/dom/performance/PerformanceObserver.cpp#125-127 shoud be a MOZ_ASSERT as the caller already does that in https://searchfox.org/mozilla-central/rev/d537e47349944c0fbd0100bd52c30e493e748c2e/dom/performance/Performance.cpp#667

Comment 1

[rohank2502]

Hey, Rohan Here !! Will like to get mentored by you for solving this bug as this is my very first contribution !!

Comment 2

[Sean Feng [:sefeng]]

Hi Rohan, thanks for your interest. The initial comment should give you an idea of where to start. Let me know if you have questions.

Comment 3

[Sneha k]

Hello! I would Like to work on this as my first contribution, should the if block be removed in https://searchfox.org/mozilla-central/rev/d537e47349944c0fbd0100bd52c30e493e748c2e/dom/performance/PerformanceObserver.cpp#125-127
?

Comment 4

[Sean Feng [:sefeng]]

Yes, and it should be replaced with MOZ_ASSERT(ObservesTypeOfEntry(aEntry));.

Comment 5

[achmurali]

Attachment: Bug 1691453 - Remove ObservesTypeOfEntry again in PerformanceObserver::QueueEntry.r?sefeng

Comment 6

[achmurali]

Hi Sean,

Can you please clear some of my questions (Kindly excuse me, if the questions are dumb) -

1. What is the purpose of MOZ_ASSERT ?
2. For bugs like these is running mochitests enough ?
3. And can you please let me know how do i debug locally on my machine.

Thanks

Comment 7

[Sean Feng [:sefeng]]

What is the purpose of MOZ_ASSERT ?

MOZ_ASSERT is just an assertion that'll be evaluated only in debug mode. We added a MOZ_ASSERT to ensure our hypothesis is correct such that the caller indeed called ObservesTypeOfEntry.

For bugs like these is running mochitests enough?

Yeah, mochitests are enough. I actually already queued the patch to land because I am confident with it. However, as you pointed out, we should run mochitests first before landing the patch. We'll see how it goes, if it's got backed out, we can investigate.

And can you please let me know how do i debug locally on my machine.

What do you want to debug? Generally, you want to run with a debug build and start a debugger like rr.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8401142a1ad2
Remove ObservesTypeOfEntry again in PerformanceObserver::QueueEntry.r=sefeng

Comment 9

[achmurali]

What do you want to debug? Generally, you want to run with a debug build and start a debugger like rr.

Like to debug mozilla generally, the address bar, the dom or any such component.

Comment 10

[Sean Feng [:sefeng]]

So to debug the address bar, you probably want to use https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox. For DOM, it's mainly like what I said. You want to use a debugger that can debug C++ code.

Stepping through the code using the debugger is generally helpful. However, most of the time you need to know where to start, and this is the tricky part.

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8401142a1ad2