Bug 116464

Summary: PPT: Closing tab that is hung or chewing 100% CPU leaves abandoned WebProcess.
Product: WebKit Reporter: Andreas Kling <kling>
Component: WebKit2Assignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, kling, rniwa, webkit-ews
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 116513    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch
andersca: review+, webkit-ews: commit-queue-
Patch for landing
kling: commit-queue+
Patch for landing
commit-queue: commit-queue-
Simpler, gentler, better(?) none

Description Andreas Kling 2013-05-20 14:37:44 PDT
<rdar://problem/10103795>
Comment 1 Andreas Kling 2013-05-20 14:38:33 PDT
Created attachment 202321 [details]
Proposed patch
Comment 2 Early Warning System Bot 2013-05-20 14:44:20 PDT
Comment on attachment 202321 [details]
Proposed patch

Attachment 202321 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/457113
Comment 3 Anders Carlsson 2013-05-20 15:05:20 PDT
Comment on attachment 202321 [details]
Proposed patch

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

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:208
> +    if (m_context->usesNetworkProcess() && !m_suddenTerminationCounter && canTerminateChildProcess())
>          requestTermination();

Can you make this an early return instead?

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:210
> +        static const double timeBeforeForcefullyKillingWebProcessInSeconds = 10;

I don’t think this needs to be static.

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:662
> +    WTFLogAlways("Killing runaway web process, PID=%d\n", processIdentifier());

Hue. Maybe lowercase pid?
Comment 4 Andreas Kling 2013-05-20 15:08:47 PDT
Created attachment 202323 [details]
Patch for landing
Comment 5 Andreas Kling 2013-05-20 15:10:42 PDT
Created attachment 202324 [details]
Patch for landing
Comment 6 Andreas Kling 2013-05-20 15:56:07 PDT
Committed r150384: <http://trac.webkit.org/changeset/150384>
Comment 7 WebKit Commit Bot 2013-05-20 21:39:22 PDT
Re-opened since this is blocked by bug 116513
Comment 8 WebKit Commit Bot 2013-05-20 21:41:02 PDT
Comment on attachment 202324 [details]
Patch for landing

Rejecting attachment 202324 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 202324, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
k #2 FAILED at 202.
Hunk #3 succeeded at 685 with fuzz 2 (offset 24 lines).
2 out of 3 hunks FAILED -- saving rejects to file Source/WebKit2/UIProcess/WebProcessProxy.cpp.rej
patching file Source/WebKit2/UIProcess/WebProcessProxy.h
Hunk #1 FAILED at 185.
Hunk #2 FAILED at 211.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebKit2/UIProcess/WebProcessProxy.h.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/528029
Comment 9 Andreas Kling 2013-05-21 14:38:31 PDT
Created attachment 202473 [details]
Simpler, gentler, better(?)
Comment 10 WebKit Commit Bot 2013-05-21 17:06:37 PDT
Comment on attachment 202473 [details]
Simpler, gentler, better(?)

Clearing flags on attachment: 202473

Committed r150491: <http://trac.webkit.org/changeset/150491>
Comment 11 WebKit Commit Bot 2013-05-21 17:06:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Ryosuke Niwa 2013-05-21 19:27:38 PDT
It seems like this patch broke two TestWebKitAPI tests:
http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/7944

http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/7944/steps/run-api-tests/logs/stdio

Tests that failed:
  WebKit2.CloseThenTerminate
Tests that timed out:
  WebKit2.DOMWindowExtensionNoCache
Comment 13 Andreas Kling 2013-05-22 11:13:44 PDT
(In reply to comment #12)
> It seems like this patch broke two TestWebKitAPI tests:
> http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/7944
> 
> http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/7944/steps/run-api-tests/logs/stdio
> 
> Tests that failed:
>   WebKit2.CloseThenTerminate

Fixed with <http://trac.webkit.org/changeset/150500>.

> Tests that timed out:
>   WebKit2.DOMWindowExtensionNoCache

Tracked on bug 116595.