Closed Bug 1529735 Opened 11 months ago Closed 11 months ago

How should sCCRunner and sICCRunner interact?

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: bzbarsky, Assigned: smaug)

References

Details

Attachments

(1 file)

Before the patch for bug 1377131 the code in nsJSContext::RunNextCollectorTimer looked like this:

  if (sCCRunner) {
    if (ReadyToTriggerExpensiveCollectorTimer()) {
      CCRunnerFired(TimeStamp());
    }
    return;
  }
 
  if (sICCRunner) {
    ICCRunnerFired(TimeStamp());
    return;
  }

but after that bug (and on tip) it looks like this:

  if (sCCRunner) {
    sCCRunner->SetDeadline(aDeadline);
    runnable = sCCRunner;
  }

  if (sICCRunner) {
    sICCRunner->SetDeadline(aDeadline);
    runnable = sICCRunner;
  }

  if (runnable) {
    runnable->Run();
  }

In particular, of both sCCRunner and sICCRunner are non-null, we used to do CCRunnerFired() and return. But now we'll run sICCRunner. Is that an intended change in behavior?

Flags: needinfo?(bugs)

sICCRunner should be null when sCCRunner is non-null.
sCCRunner does all the forget skippable handling, and eventually may trigger iCC, which then
sets sCCRunner to null and set sICCRunner to non-null
https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/dom/base/nsJSEnvironment.cpp#1534-1548.

Status: NEW → RESOLVED
Closed: 11 months ago
Flags: needinfo?(bugs)
Resolution: --- → INVALID

sICCRunner should be null when sCCRunner is non-null.

Is that worth asserting?

Flags: needinfo?(bugs)
Status: RESOLVED → REOPENED
Flags: needinfo?(bugs)
Resolution: INVALID → ---
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3dc67a87eb4
assert that only either sCCRunner or sICCRunner is active, not both, r=mccr8
Status: REOPENED → RESOLVED
Closed: 11 months ago11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.