1114345 - Slow script dialog pops up after hibernating my machine

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

If I leave ff open and hibernate my computer then I see the slow script dialog sometimes when I turn it back on. I'm guessing that this is fallout from bug 1111412 since I've only seen this in the last few days.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yeah, I guess we probably need to the the usual "check twice" thing here. I wonder why this didn't happen before, though.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: slow-script-laptop

I think this should do the trick. Not sure how to test this though.

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8543460 [details] [diff] [review]
slow-script-laptop

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

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1301,5 @@
>          // been running long enough that we might show the slow script dialog.
>          // Triggering the callback from off the main thread can be expensive.
> +        // We wait half the allotted time because the interrupt callback must
> +        // fire twice before it's considered a slow script. This avoids problems
> +        // with laptops going to sleep or time changes.

Can you explain the problematic situation in more detail?

@@ +1378,2 @@
>          return true;
>      }

While you're here (possibly in a separate patch underneath this one), can you remove this whole block? I believe it was obsoleted by bug 1111412, and it confused the heck out of my while I was trying to page this stuff back in.

::: js/xpconnect/src/xpcprivate.h
@@ +674,5 @@
>      JS::PersistentRootedObject mCompilationScope;
>      nsRefPtr<AsyncFreeSnowWhite> mAsyncSnowWhiteFreer;
>  
>      mozilla::TimeStamp mSlowScriptCheckpoint;
> +    bool mSlowScriptSecondHalf;

Please comment profusely as to what each of these is about.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10b0f190e1ef

> While you're here (possibly in a separate patch underneath this one), can you remove this
> whole block? I believe it was obsoleted by bug 1111412, and it confused the heck out of my
> while I was trying to page this stuff back in.

I don't think we can take this out. It's still possible to run JS code outside of an event handler. In that case we want to initialize the checkpoint there. I did update the comment to reflect this though.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/10b0f190e1ef