Re: [2/5] winex11: Splitted functions X11DRV_FocusOut and X11DRV_ReparentNotify
Sebastian Lackner sebast...@fds-team.de writes: --- dlls/winex11.drv/event.c | 99 +++--- 1 file changed, 58 insertions(+), 41 deletions(-) These should go in the patches that need them. Also you should start your series with reparent support, the rest doesn't make sense without it. -- Alexandre Julliard julli...@winehq.org
Re: crypt32: Support HCCE_LOCAL_MACHINE.
Hi Ben, thanks for having a whack at this.
Some tests would be nice.
-static HCERTCHAINENGINE CRYPT_defaultChainEngine;
+/* There are two default chain engines which correspond to
HCCE_CURRENT_USER and
+ * HCCE_LOCAL_MACHINE.
+*/
+static HCERTCHAINENGINE CRYPT_defaultChainEngine[2] = { NULL, NULL };
C automatically initializes statics to 0, so the initialization here is
unnecessary. I'm also a little uncertain about the use of an array of two
objects, I'm not sure that two distinct objects wouldn't be easier to
follow, but I'm not religious on this point.
+ if (hChainEngine HCCE_LOCAL_MACHINE
+ InterlockedDecrement(engine-ref) == 0)
I think a function that returns whether an HCERTCHAINENGINE is one of the
special ones would make this easier to read, e.g.:
static int is_special_chain_engine(HCERTCHAINENGINE h)
{
return h == HCCE_CURRENT_USER || h == HCCE_LOCAL_MACHINE;
}
then:
+ if (is_special_chain_engine(hChainEngine)
+ InterlockedDecrement(engine-ref) == 0)
+static HCERTCHAINENGINE CRYPT_GetDefaultChainEngine(HCERTCHAINENGINE h)
{
-if (!CRYPT_defaultChainEngine)
+if (!CRYPT_defaultChainEngine[(int)h])
The constant casting is a little awkward. At least introduce a local
pointer to the one you intend to modify, so we're not constantly having to
re-read that cast.
+if (hChainEngine = HCCE_LOCAL_MACHINE)
+engine = (CertificateChainEngine*)CRYPT_GetDefaultChainEngine(
+ hChainEngine);
if (TRACE_ON(chain))
dump_chain_para(pChainPara);
/* FIXME: what about HCCE_LOCAL_MACHINE? */
See my earlier suggestion on is_special_chain_engine. Also, what about that
comment three lines down? Can't it be removed?
-void default_chain_engine_free(void)
+void default_chain_engine_free(HCERTCHAINENGINE h)
{
-CertFreeCertificateChainEngine(CRYPT_defaultChainEngine);
+CertFreeCertificateChainEngine(CRYPT_defaultChainEngine[(int)h]);
The function default_chain_engine_free is a thin wrapper around
CryptFreeCertificateChainEngine; its intended use is to clear memory at
shutdown. Rather than shift the responsibility of knowing which engines to
free to the caller, just have it free the two engines itself.
Thanks,
--Juan
Re: Wine Gecko 2.24-beta1
On 08/18/13 04:14, Zhenbo Li wrote: I've tested some websites, and found that these pages have problems: http://mail.163.com (it will make gecko crash) http://huaban.com (nothing can be shown) http://douban.com (no pictures) http://html5test.com/ (can't load page, and I'm sure that it's not a regression) I've tested them with Firefox + flashblock, so I think they are not caused by flash. I'm glad to do more tests to contribute Gecko. Thanks for testing. I confirmed all those bugs while preparing the release, but they seem to be caused by MSHTML, not Gecko itself, so the release won't change behaviour here. Cheers, Jacek
Re: (try 4)[1/4] imm32: Move thread data from TLSEntry to an internal list
Aric Stewart a...@codeweavers.com writes: --- dlls/imm32/imm.c | 153 +-- 1 file changed, 116 insertions(+), 37 deletions(-) It breaks the tests: ../../../tools/runtest -q -P wine -M urlmon.dll -T ../../.. -p urlmon_test.exe.so url.c touch url.ok err:ntdll:RtlpWaitForCriticalSection section 0x55ec38e8 imm.c: threaddata_cs wait timed out in thread 005c, blocked by 0031, retrying (60 sec) err:ntdll:RtlpWaitForCriticalSection section 0x7bccc200 loader.c: loader_section wait timed out in thread 0060, blocked by 005c, retrying (60 sec) -- Alexandre Julliard julli...@winehq.org
Re: [PATCH] d3dx9: Use struct d3dx_object for objects.
2013/9/24 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/effect.c | 248 +++-- 1 Datei geändert, 95 Zeilen hinzugefügt(+), 153 Zeilen entfernt(-) I definitely like the direction this patch takes. @@ -5068,6 +5009,9 @@ static HRESULT d3dx9_parse_resource(struct d3dx9_base_effect *base, const char * param = state-parameter; +/* + * TODO: Do we need to create the shader/string here or later when we access them? + */ Here it's fine AFAICS. What are the other options you are thinking about? Something like creating the shaders at the first Begin / BeginPass call?
Re: (try 5) [2/4] imm32: do not let ImmDestroyContext destroy any default contexts
On 9/25/2013 16:03, Aric Stewart wrote: defaultContext = ImmCreateContext(); +if (defaultContext) +((InputContextData*)defaultContext)-threadDefault = TRUE; I think a nicer way is to let ImmCreateContext() set this attribute.
Re: (try 5)[1/4] imm32: Move thread data from TLSEntry to an internal list
On 9/25/2013 16:03, Aric Stewart wrote:
+if (!thread_data-defaultContext)
+{
+HIMC defaultContext;
+LeaveCriticalSection(threaddata_cs);
+defaultContext = ImmCreateContext();
+thread_data = IMM_GetThreadData(0);
Why do you need to unlock/lock around ImmCreateContext()? Is it related
to SendMessage() it calls?
Re: [PATCH] d3dx9: Use struct d3dx_object for objects.
On 25.09.2013 14:02, Matteo Bruni wrote: 2013/9/24 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/effect.c | 248 +++-- 1 Datei geändert, 95 Zeilen hinzugefügt(+), 153 Zeilen entfernt(-) I definitely like the direction this patch takes. @@ -5068,6 +5009,9 @@ static HRESULT d3dx9_parse_resource(struct d3dx9_base_effect *base, const char * param = state-parameter; +/* + * TODO: Do we need to create the shader/string here or later when we access them? + */ Here it's fine AFAICS. What are the other options you are thinking about? Something like creating the shaders at the first Begin / BeginPass call? Yes. Depending on the use case we may create it on the Begin/BeginPass when the shader is needed the first time (this may also be something like isParameterUsed or GetVertexShader). We don't need the shaders for cases like GetPassDesc, where only the shader blob is used and in cases the shaders are not used at all (e.g. if some technique is for shader version 2 and one for shader version 3). That's why I think it may be an option to defer the shader creation. Imho this needs some tests first before an useful decision could be made. The comment was added, because I removed the previously available shader creation (which had some flaws). Cheers Rico
Re: (try 5)[1/4] imm32: Move thread data from TLSEntry to an internal list
On 9/25/13 7:11 AM, Nikolay Sivov wrote:
On 9/25/2013 16:03, Aric Stewart wrote:
+if (!thread_data-defaultContext)
+{
+HIMC defaultContext;
+LeaveCriticalSection(threaddata_cs);
+defaultContext = ImmCreateContext();
+thread_data = IMM_GetThreadData(0);
Why do you need to unlock/lock around ImmCreateContext()? Is it related to
SendMessage() it calls?
Correct, it is related to the SendMessage call.
-aric
Re: (try 5) [2/4] imm32: do not let ImmDestroyContext destroy any default contexts
On 9/25/13 7:09 AM, Nikolay Sivov wrote: On 9/25/2013 16:03, Aric Stewart wrote: defaultContext = ImmCreateContext(); +if (defaultContext) +((InputContextData*)defaultContext)-threadDefault = TRUE; I think a nicer way is to let ImmCreateContext() set this attribute. ImmCreateContext is a win32 api so i am not aware of how I would signal to the call that it is a thread default window. If it is called by an application, not to create a default HIMC then it should not have this set. -aric
Re: wininet: Don't assume that end of chunk means end of stream. (try 2)
Hi Hans,
Sorry for the delay, I wanted to test it a bit more before answering.
On 09/23/13 10:50, Hans Leidekker wrote:
On Thu, 2013-09-19 at 17:38 +0200, Jacek Caban wrote:
I was hoping for a test like the attached one, which shows that there is
still more work to do. But it's a step in the right direction, so I'm
fine with your patch.
The issue here is that HTTP_ReceiveRequestData calls refill_read_buffer, which
calls generic read_http_stream with a read size of 8192 (the read buffer
is still empty). In non-chunked mode this should read the minimum of that size
and the content length. In chunked mode we don't know the content length,
and we should read the minimum of chunk size and read buffer size, as shown by
your test.
Maybe we should add a refill_read_buffer method to the backends and call that
instead of read_http_stream?
We should be able to do that with read semantics that is intended for
READMODE_ASYNC: read anything, not more than buf size and, once any data
is read, don't block. The attached extended tests show the problem with
chunked reads in this case. We'd need to maintain full state of the
stream to be able to do non-blocking chunk headers/tails reads.
Jacek
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index 12eab57..889707c 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -103,7 +103,7 @@ static int expect[MAX_INTERNET_STATUS],
optional[MAX_INTERNET_STATUS],
wine_allow[MAX_INTERNET_STATUS], notified[MAX_INTERNET_STATUS];
static const char *status_string[MAX_INTERNET_STATUS];
-static HANDLE hCompleteEvent, conn_close_event;
+static HANDLE hCompleteEvent, conn_close_event, server_event;
static DWORD req_error;
#define TESTF_REDIRECT 0x01
@@ -2114,6 +2114,36 @@ static DWORD CALLBACK server_thread(LPVOID param)
}
if (strstr(buffer, GET /test_premature_disconnect))
trace(closing connection\n);
+if (strstr(buffer, GET /test_chunked)) {
+static const char chunked_header[] = HTTP/1.1 200
OK\r\nTransfer-Encoding: chunked\r\n\r\n;
+static const char chunk1[] =
20\r\n\r\n;
+static const char chunk2[] = 18\r\n\r\n;
+static const char chunk3[] =
28\r\n\r\n;
+static const char last_chunk[] = 0\r\n\r\n;
+
+send(c, chunked_header, sizeof(chunked_header)-1, 0);
+send(c, chunk1, sizeof(chunk1)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk2, sizeof(chunk2)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk3, sizeof(chunk3)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk1, sizeof(chunk1)-1, 0);
+send(c, chunk2, sizeof(chunk2)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk1, sizeof(chunk1)-1, 0);
+send(c, chunk2, 3, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk2+3, sizeof(chunk2)-4, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, last_chunk, sizeof(last_chunk)-1, 0);
+}
shutdown(c, 2);
closesocket(c);
@@ -2917,6 +2947,181 @@ static void test_no_content(int port)
CloseHandle(hCompleteEvent);
}
+static void test_chunked_stream(int port)
+{
+HINTERNET session, connection, req;
+DWORD res, avail, size;
+BYTE buf[256];
+
+trace(Testing chunked stream...\n);
+
+hCompleteEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+server_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+
+session = InternetOpenA(, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL,
INTERNET_FLAG_ASYNC);
+ok(session != NULL,InternetOpen failed with error %u\n, GetLastError());
+
+pInternetSetStatusCallbackA(session, callback);
+
+SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+connection = InternetConnectA(session, localhost, port,
+NULL, NULL, INTERNET_SERVICE_HTTP, 0x0, 0xdeadbeef);
+ok(connection != NULL,InternetConnect failed with error %u\n,
GetLastError());
+CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+req = HttpOpenRequestA(connection, GET, /test_chunked, NULL, NULL,
NULL,
+INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_RESYNCHRONIZE,
0xdeadbead);
+ok(req != NULL, HttpOpenRequest failed: %u\n, GetLastError());
+CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+trace(sending request...\n);
+
+SET_OPTIONAL(INTERNET_STATUS_COOKIE_SENT);
+SET_EXPECT(INTERNET_STATUS_CONNECTING_TO_SERVER);
+SET_EXPECT(INTERNET_STATUS_CONNECTED_TO_SERVER);
+SET_EXPECT(INTERNET_STATUS_SENDING_REQUEST);
+SET_EXPECT(INTERNET_STATUS_REQUEST_SENT);
+
Re: [PATCH] d3dx9: Use struct d3dx_object for objects.
On 25 September 2013 15:32, Rico Schüller kgbric...@web.de wrote: Yes. Depending on the use case we may create it on the Begin/BeginPass when the shader is needed the first time (this may also be something like isParameterUsed or GetVertexShader). We don't need the shaders for cases like GetPassDesc, where only the shader blob is used and in cases the shaders are not used at all (e.g. if some technique is for shader version 2 and one for shader version 3). That's why I think it may be an option to defer the shader creation. Imho this needs some tests first before an useful decision could be made. The comment was added, because I removed the previously available shader creation (which had some flaws). I think you almost always want to create shaders as early as possible. Shader compilation time and associated stuttering is a real issue. We could do better on that front in wined3d as well, and in some cases in the driver, but if the application doesn't pass us the shader until it's first used there's nothing we can do.
Re: [2/5] winex11: Splitted functions X11DRV_FocusOut and X11DRV_ReparentNotify
Am 25.09.2013 12:19, schrieb Alexandre Julliard: Sebastian Lackner sebast...@fds-team.de writes: --- dlls/winex11.drv/event.c | 99 +++--- 1 file changed, 58 insertions(+), 41 deletions(-) These should go in the patches that need them. Also you should start your series with reparent support, the rest doesn't make sense without it. Well, if its just the ordering of the patches, thats surely something I can change. Concerning reparent support: The specification states that --- The protocol is started by the embedder. The window ID of the client window is passed (by unspecified means) to the embedding application, and the embedder calls XReparentWindow() to reparent the client window into the embedder window. Implementations may choose to support an alternate method of beginning the protocol where the window ID of the embedder is passed to client application and the client creates a window within the embedder, or reparents an existing window into the embedder's window. Which method of starting XEmbed is used a matter up to higher level agreement and outside the scope of this specification. --- This means both methods are valid: * either the client (=wine) passes its own x11 window id to the parent, which does the reparent * or the client itself does the reparent with the x11 window id from the embedder So far Pipelight [1] uses the first method, which at my opinion is definitely preferred, as it wouldn't make that much sense to implement a user32 - winex11 interface, which expects xwindow-ids instead of hwnds. A normal wine application doesn't have any way to know these xwindow ids of foreign applications, so in fact this whole method is only possible, when there is a linux application on the other side. [1] http://fds-team.de/cms/articles/2013-08/pipelight-using-silverlight-in-linux-browsers.html
Re: ntdll: Perform the offset checks also for a serial device.
On Tue, 24 Sep 2013, Dmitry Timoshkov wrote: Marvin test...@winehq.org wrote: === w7pro64 (32 bit comm) === comm.c:863: Test failed: WaitCommEvent failed with a timeout comm.c:875: recovering after WAIT_TIMEOUT... comm.c:886: Test failed: WaitCommEvent error 0 comm.c:888: Test failed: WaitCommEvent: expected EV_TXEMPTY, got 0 comm.c:892: WaitCommEvent for EV_TXEMPTY took 1014 ms (timeout 1000) comm.c:894: Test failed: WaitCommEvent used 1014 ms for waiting comm.c:1561: test_AbortWaitCts timeout 500 handle 00F8 comm.c:1528: Changing CommMask on the fly for handle 00F8 after timeout 500 This VM is broken, How so? That is: what should I do to fix it? -- Francois Gouget fgou...@free.fr http://fgouget.free.fr/ Linux: It is now safe to turn on your computer.
Re: user32: Avoid using CONST.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=2332 Your paranoid android. === build (build) === Patch failed to apply
Re: ntdll: Perform the offset checks also for a serial device.
Francois Gouget fgou...@free.fr wrote: === w7pro64 (32 bit comm) === comm.c:863: Test failed: WaitCommEvent failed with a timeout comm.c:875: recovering after WAIT_TIMEOUT... comm.c:886: Test failed: WaitCommEvent error 0 comm.c:888: Test failed: WaitCommEvent: expected EV_TXEMPTY, got 0 comm.c:892: WaitCommEvent for EV_TXEMPTY took 1014 ms (timeout 1000) comm.c:894: Test failed: WaitCommEvent used 1014 ms for waiting comm.c:1561: test_AbortWaitCts timeout 500 handle 00F8 comm.c:1528: Changing CommMask on the fly for handle 00F8 after timeout 500 This VM is broken, How so? That is: what should I do to fix it? Investigate, find the source of failures and fix it? Note, that all other VMs pass this test just fine, moreover, this same w7pro64 VM doesn't always fail. -- Dmitry.
Re: ntdll: Perform the offset checks also for a serial device.
On Thu, 26 Sep 2013, Dmitry Timoshkov wrote: [...] This VM is broken, How so? That is: what should I do to fix it? Investigate, find the source of failures and fix it? Note, that all other VMs pass this test just fine, moreover, this same w7pro64 VM doesn't always fail. You obviously know more about this API and that test than I do. Further, before claiming the VM is broken I'm sure you have positively determined that the successes you had on your boxes were not just due to luck but that something really is wrong with the VM. So please don't waste my time, at least share your results. -- Francois Gouget fgou...@free.fr http://fgouget.free.fr/ Avoid the Gates of Hell - use Linux.
Re: ntdll: Perform the offset checks also for a serial device.
Francois Gouget fgou...@free.fr wrote: This VM is broken, How so? That is: what should I do to fix it? Investigate, find the source of failures and fix it? Note, that all other VMs pass this test just fine, moreover, this same w7pro64 VM doesn't always fail. You obviously know more about this API and that test than I do. Further, before claiming the VM is broken I'm sure you have positively determined that the successes you had on your boxes were not just due to luck but that something really is wrong with the VM. So please don't waste my time, at least share your results. Obviously you need nothing to know about the test except that it works with a COM-port, so the only thing you'd need to investigate is the difference between VMs in COM-port setup, and try to figure out why this test doesn't always pass. I have no no idea what kind of results you have in mind that I'd need to share. Also I'd appreciate avoiding the wording like please don't waste my time, otherwise it becomes pretty obvious that I should just reply to failure notifications in the testbot with simple it's broken and stop wasting my time explaining why. -- Dmitry.
