Bug 146124 - Spintracer treats the web process as hung when it's showing JavaScript dialogs in the UI process
Summary: Spintracer treats the web process as hung when it's showing JavaScript dialog...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-18 14:42 PDT by Anders Carlsson
Modified: 2015-06-19 10:19 PDT (History)
1 user (show)

See Also:


Attachments
Patch (22.80 KB, patch)
2015-06-18 14:46 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (13.92 KB, patch)
2015-06-18 15:10 PDT, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (15.47 KB, patch)
2015-06-18 15:27 PDT, Anders Carlsson
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2015-06-18 14:42:41 PDT
Spintracer treats the web process as hung when it's showing JavaScript dialogs in the UI process
Comment 1 Anders Carlsson 2015-06-18 14:46:15 PDT
Created attachment 255136 [details]
Patch
Comment 2 Geoffrey Garen 2015-06-18 15:02:11 PDT
Comment on attachment 255136 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255136&action=review

> Source/WebKit2/Shared/mac/HangDetectionDisablerMac.mm:38
> +    if (CGSCopyConnectionProperty(CGSMainConnectionID(), CGSMainConnectionID(), clientsMayIgnoreEventsKey, &value) != kCGErrorSuccess)

Shouldn't we CFRelease value after this copy? Maybe use RetainPtr here?

> Source/WebKit2/Shared/mac/WebMemorySampler.mac.mm:-32
> -#import "config.h"
> -#import "WebMemorySampler.h"
> -
> -#if ENABLE(MEMORY_SAMPLER)  
> -
> -#import <JavaScriptCore/MemoryStatistics.h>
> -#import <mach/mach.h>

Revert?
Comment 3 Anders Carlsson 2015-06-18 15:10:29 PDT
Created attachment 255138 [details]
Patch
Comment 4 Anders Carlsson 2015-06-18 15:27:09 PDT
Created attachment 255139 [details]
Patch
Comment 5 Geoffrey Garen 2015-06-18 15:41:12 PDT
Comment on attachment 255139 [details]
Patch

r=me
Comment 6 Alexey Proskuryakov 2015-06-18 19:00:53 PDT
Nice! Do you plan to adopt this for sync XHR, or would you like us to keep receiving reports about those?
Comment 7 David Kilzer (:ddkilzer) 2015-06-19 08:12:42 PDT
<rdar://problem/21449395>
Comment 8 Anders Carlsson 2015-06-19 08:39:18 PDT
(In reply to comment #6)
> Nice! Do you plan to adopt this for sync XHR, or would you like us to keep
> receiving reports about those?

I talked to Gavin about doing it - it's definitely something we should do. Same thing for window.print.
Comment 9 Anders Carlsson 2015-06-19 09:54:38 PDT
Committed r185749: <http://trac.webkit.org/changeset/185749>
Comment 10 Darin Adler 2015-06-19 10:19:48 PDT
Comment on attachment 255139 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255139&action=review

> Source/WebKit2/Shared/mac/HangDetectionDisablerMac.mm:47
> +    auto value = adoptCF(valuePtr);
> +
> +    if (!value || CFGetTypeID(value.get()) != CFBooleanGetTypeID())
> +        return false;
> +
> +    return value == kCFBooleanTrue;

I think this one line does the same thing as the above four lines of code.

    return adoptCF(valuePtr) == kCFBooleanTrue;