Bug 46714

Summary: WebKitTestRunner needs layoutTestController.setCallCloseOnWebViews
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Tools / TestsAssignee: Balazs Kelemen <kbalazs>
Status: NEW ---    
Severity: Normal CC: abecsi, andersca, eric, hugo.lima, kbalazs, ossy, sam, webkit.review.bot
Priority: P2 Keywords: InRadar, LayoutTestFailure
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Adam Roben (:aroben) 2010-09-28 04:03:38 PDT
WebKitTestRunner needs layoutTestController.setCallCloseOnWebViews
Comment 1 Adam Roben (:aroben) 2010-09-28 04:30:15 PDT
<rdar://problem/8485908>
Comment 2 Balazs Kelemen 2011-06-27 06:35:45 PDT
Currently it is on the win-wk2 list and also on the mac-snowleopard list with other reason. It should be skipped via the mac-wk2 list to make it skipped on every wk2 platform.
Comment 3 Balazs Kelemen 2011-06-27 06:37:49 PDT
Created attachment 98714 [details]
Patch
Comment 4 Csaba Osztrogon√°c 2011-06-27 06:44:36 PDT
Comment on attachment 98714 [details]
Patch

r=me. Please reopen the bug after CQ closed it. :)
Comment 5 WebKit Review Bot 2011-06-27 06:47:54 PDT
Comment on attachment 98714 [details]
Patch

Rejecting attachment 98714 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 385, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 441, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/8944341
Comment 6 Andras Becsi 2011-06-27 06:52:05 PDT
(In reply to comment #5)
> (From update of attachment 98714 [details])
> Rejecting attachment 98714 [details] from commit-queue.
> 
> Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1
> 
> Last 500 characters of output:
> cripts/webkitpy/common/system/executive.py", line 385, in run_command
>     close_fds=self._should_close_fds())
>   File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 441, in popen
>     return subprocess.Popen(*args, **kwargs)
>   File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
>     errread, errwrite)
>   File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
>     raise child_exception
> TypeError: execv() arg 2 must contain only strings
> 
> Full output: http://queues.webkit.org/results/8944341

Seems, that CQ does not like UTF-8 characters in reviewers names, like in bug
https://bugs.webkit.org/show_bug.cgi?id=61890
Comment 7 Balazs Kelemen 2011-06-27 07:22:26 PDT
Created attachment 98719 [details]
Patch

One more test is affected by this and need to be skipped.
Comment 8 Eric Seidel (no email) 2011-06-27 10:13:29 PDT
(In reply to comment #6)
> Seems, that CQ does not like UTF-8 characters in reviewers names, like in bug
> https://bugs.webkit.org/show_bug.cgi?id=61890

I suspect it's a config issue on these new CQ bots, due to setting LANG=ascii, etc.
Comment 9 Csaba Osztrogon√°c 2011-06-27 22:48:19 PDT
Comment on attachment 98719 [details]
Patch

rs=me, but it seems your patch needs update to be appliable.
Comment 10 Balazs Kelemen 2011-06-28 00:35:28 PDT
Committed r89900: <http://trac.webkit.org/changeset/89900>
Comment 11 Andras Becsi 2011-09-02 03:31:25 PDT
(In reply to comment #10)
> Committed r89900: <http://trac.webkit.org/changeset/89900>

Can this bug be closed now?
Comment 12 Andras Becsi 2011-09-02 03:33:26 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > Committed r89900: <http://trac.webkit.org/changeset/89900>
> 
> Can this bug be closed now?

Please disregard me, this was just skipping the affected tests.
Please make it more obvious next time, that the patch only skips tests.
Comment 13 Hugo Parente Lima 2012-05-24 12:21:50 PDT
Is this function really needed?

Only two layouttests uses it:

./plugins/open-and-close-window-with-plugin.html
./plugins/geturlnotify-during-document-teardown.html


The chromium implementation does nothing:

void LayoutTestController::setCallCloseOnWebViews(const CppArgumentList& arguments, CppVariant* result)
{
    result->setNull();
}

The Qt implementation also does nothing.

DumpRenderTree/LayoutTestController.{h|cpp} just sets the variable m_callCloseOnWebViews that isn't used anywhere.

The other ports didn't have any reference to it.

So it's fine to remove it and modify the only two layout tests using it?

Regards
Comment 14 Balazs Kelemen 2012-05-24 15:03:06 PDT
> So it's fine to remove it and modify the only two layout tests using it?
 
I think's it's definitely an improvement to remove such a non-sense feature.
Comment 15 Hugo Parente Lima 2012-05-28 11:22:49 PDT
(In reply to comment #14)
> > So it's fine to remove it and modify the only two layout tests using it?
> 
> I think's it's definitely an improvement to remove such a non-sense feature.

I was wrong, some ports including Qt-WK1 really do something on this function, so it's still valid.

Sorry for the noise.