Re: [webkit-dev] Client-based Geolocation
(1) Adds a level of indirection that may not be necessary in these cases. It's true, there's now one more level of indirection. The non-client-based implementation uses Geolocation - GeolocationServiceFoo. The client-based implementation uses Geolocation - GeolocationController - GeolocationServiceFoo. I don't think this is significant, especially as the Controller/Client pattern is well established. (2) Harder to find the back end code, since the common pattern is to have platform-specific implementations in WebCore/platform True, though as I mentioned previously, I'm not sure that Geolocation really fits into the category of 'platform-specific implementations' - there are too many decisions to be made by the embedder. (3) There may not be a one-to-one mapping between back-end implementations and WebKit public APIs. So far, there is a one-to-one mapping. My argument is that this likely to remain the case because embedders want to have close control over their Geolocation implementation (for example, when to enable a GPS if present, which URL to use for network-based lookups, how to handle position caching, how frequently to poll for updates etc). While it might be possible to have a GeolocationServiceLinux in WebCore/platform/linux, for example, that would provide position information for all Linux-based ports, I think that in practice, the embedder wants to have more control than this approach permits. So I think there will be little, if any code duplication between ports. If we later find that multiple ports are duplicating code by using a common platform library, for example, we could consider adding a shared WebCore utility to provide access to this. So that explains the upsides, but you didn't address the downsides I cited. Sorry, I hope I've done so now. Maybe the positives outweigh the negatives, Yes, that's exactly my opinion. Steve -- Google UK Limited Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ Registered in England Number: 3977902 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Atomic read/write operations and thread safety
I have a question about whether WebCore code makes assumptions about the atomicity of certain read/write operations. The particular instance I'm interested in is IconDatabase, where m_threadTerminationRequested is written to from the main thread (in close() for example) and read from the DB thread (in syncThreadMainLoop()) without any locking. It looks like the code is written such that there's no particular synchronization requirement in terms of the order in which the value is set and read. But there's a risk of a garbled read in the case of a simultaneous read and write from the two threads. I assume that the lack of locking is an intentional decision, based on the fact that on all relevant architectures, a boolean read/write is atomic? Is this a general WebCore policy? It would be great if somebody could confirm this, or point out why I'm wrong. Thanks, Steve -- Google UK Limited Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ Registered in England Number: 3977902 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] JSRetainPtr and JSStringRef
Hello I'm looking at adding a method on the LayoutTestController in DumpRenderTree that will return a string. I see that some methods there return JSRetainPtrJSStringRef and some just return JSStringRef. Can anybody comment on when it is appropriate to return JSRetainPtr vs plain JSStringRef? Many thanks John ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Potential patch for building webkit on Visual Studio 2010
On Dec 14, 2010, at 8:24 PM, Jake wrote:
Hello,
I've made some changes to the webkit trunk that allows me to build webkit
(more specifically QtWebkit) with Visual Studio 2010. I had to make several
changes to handle the ambiguous operator = error from NullPtr.h.
To get the build to work I made two primary changes:
First, NullPtr.h
#ifndef NullPtr_h
#define NullPtr_h
// For compilers and standard libraries that do not yet include it, this adds
the
// nullptr_t type and nullptr object. They are defined in the same namespaces
they
// would be in compiler and library that had the support.
#if !defined(_MSC_VER)
#ifndef __has_feature
#define __has_feature(feature) 0
#endif
#if !__has_feature(cxx_nullptr)
namespace std {
class nullptr_t { };
}
extern std::nullptr_t nullptr;
#endif
#elseif _MSC_VER 1600
//to maintain compatibility with previous versions of msvc
#ifndef nullptr
#define nullptr 0
#endif
#endif //_MSC_VER
#endif
And secondly I changed all = 0 errors to = nullptr (approximately 450
instances).
My question is, does this look like a sound approach? If so, I'll submit a
patch.
It would be great to move this discussion to bugs.webkit.org, even if you
aren't sure of the exact correct approach yet. Please file a bug there and
attach your patch so that contributors can discuss it.
-Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] JSRetainPtr and JSStringRef
On Dec 15, 2010, at 9:13 AM, John Knottenbelt wrote: I'm looking at adding a method on the LayoutTestController in DumpRenderTree that will return a string. I see that some methods there return JSRetainPtrJSStringRef and some just return JSStringRef. Can anybody comment on when it is appropriate to return JSRetainPtr vs plain JSStringRef? JSRetainPtrJSStringRef should be used any time it is the responsibility of the caller to release the JSStringRef. Otherwise it is fine to use a plain JSStringRef. -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
On Dec 15, 2010, at 4:28 AM, Steve Block wrote: I have a question about whether WebCore code makes assumptions about the atomicity of certain read/write operations. The particular instance I'm interested in is IconDatabase, where m_threadTerminationRequested is written to from the main thread (in close() for example) and read from the DB thread (in syncThreadMainLoop()) without any locking. It looks like the code is written such that there's no particular synchronization requirement in terms of the order in which the value is set and read. But there's a risk of a garbled read in the case of a simultaneous read and write from the two threads. I assume that the lack of locking is an intentional decision, based on the fact that on all relevant architectures, a boolean read/write is atomic? Correct - or, at least, at the time it was correct. Have you discovered a relevant architecture where it is no longer correct? Is this a general WebCore policy? It would be great if somebody could confirm this, or point out why I'm wrong. I wouldn't be surprised if there were other examples of an unprotected thread shared boolean in either WebCore or any platform's WebKit. I *would* be surprised if there was an example of *any other data type* that was shared between threads unprotected. That said, I don't think we have any general WebCore policies about multithread practices. There might be value in such a document. ~Brady Thanks, Steve -- Google UK Limited Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ Registered in England Number: 3977902 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
On Dec 15, 2010, at 8:47 AM, Brady Eidson wrote: On Dec 15, 2010, at 4:28 AM, Steve Block wrote: I have a question about whether WebCore code makes assumptions about the atomicity of certain read/write operations. The particular instance I'm interested in is IconDatabase, where m_threadTerminationRequested is written to from the main thread (in close() for example) and read from the DB thread (in syncThreadMainLoop()) without any locking. It looks like the code is written such that there's no particular synchronization requirement in terms of the order in which the value is set and read. But there's a risk of a garbled read in the case of a simultaneous read and write from the two threads. I assume that the lack of locking is an intentional decision, based on the fact that on all relevant architectures, a boolean read/write is atomic? Correct - or, at least, at the time it was correct. Have you discovered a relevant architecture where it is no longer correct? Is this a general WebCore policy? It would be great if somebody could confirm this, or point out why I'm wrong. I wouldn't be surprised if there were other examples of an unprotected thread shared boolean in either WebCore or any platform's WebKit. In hindsight I realize my response involved assumptions about this code that were known to me but not to the general audience, so I'll followup more thoroughly. The boolean in question isn't both checked and set at the same time - it's not an acquired resource. One thread sets it, the other checks it. My belief is that this is safe for booleans, but I would love to hear where I'm wrong. Additionally, while there isn't a guard specifically around the boolean, there is the m_syncLock Mutex which is implicitly guarding operations on this bool, so the thread safety of a bool? argument becomes somewhat moot. If there's a specific race condition in this code you've spotted, I'd love to hear how it would manifest! Thanks, ~Brady I *would* be surprised if there was an example of *any other data type* that was shared between threads unprotected. That said, I don't think we have any general WebCore policies about multithread practices. There might be value in such a document. ~Brady Thanks, Steve -- Google UK Limited Registered Office: Belgrave House, 76 Buckingham Palace Road, London SW1W 9TQ Registered in England Number: 3977902 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Potential patch for building webkit on Visual Studio 2010
A more general discussion is how compatible with C++ 0x WebKit intends to be.
The problem Jake mentioned is caused by the RefPtr class. When faced with:
RefPtrNode node;
node = 0;
the VS2010 compiler can't tell whether to cast the integral 0 to
nullptr_t or to T*, e.g. to use either
RefPtr operator=(T*);
RefPtr operator=(std::nullptr_t) { clear(); return *this; }
If WebKit developers intend the code to be tracking C++ 0x, changing
the assignments to use nullptr would be forward thinking. If not, I
expect we could #ifdef out operator=(std::nullptr_t).
I'm new to the WebKit list, so apologies if this broader discussion
should also be done within the bug tracking system.
Jean-Luc
On Wed, Dec 15, 2010 at 8:38 AM, Adam Roben aro...@apple.com wrote:
On Dec 14, 2010, at 8:24 PM, Jake wrote:
Hello,
I've made some changes to the webkit trunk that allows me to build webkit
(more specifically QtWebkit) with Visual Studio 2010. I had to make several
changes to handle the ambiguous operator = error from NullPtr.h.
To get the build to work I made two primary changes:
First, NullPtr.h
#ifndef NullPtr_h
#define NullPtr_h
// For compilers and standard libraries that do not yet include it, this
adds the
// nullptr_t type and nullptr object. They are defined in the same
namespaces they
// would be in compiler and library that had the support.
#if !defined(_MSC_VER)
#ifndef __has_feature
#define __has_feature(feature) 0
#endif
#if !__has_feature(cxx_nullptr)
namespace std {
class nullptr_t { };
}
extern std::nullptr_t nullptr;
#endif
#elseif _MSC_VER 1600
//to maintain compatibility with previous versions of msvc
#ifndef nullptr
#define nullptr 0
#endif
#endif //_MSC_VER
#endif
And secondly I changed all = 0 errors to = nullptr (approximately 450
instances).
My question is, does this look like a sound approach? If so, I'll submit a
patch.
It would be great to move this discussion to bugs.webkit.org, even if you
aren't sure of the exact correct approach yet. Please file a bug there and
attach your patch so that contributors can discuss it.
-Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
Hi, Am 15.12.2010 18:43, schrieb Brady Eidson: On Dec 15, 2010, at 8:47 AM, Brady Eidson wrote: On Dec 15, 2010, at 4:28 AM, Steve Block wrote: I wouldn't be surprised if there were other examples of an unprotected thread shared boolean in either WebCore or any platform's WebKit. In hindsight I realize my response involved assumptions about this code that were known to me but not to the general audience, so I'll followup more thoroughly. The boolean in question isn't both checked and set at the same time - it's not an acquired resource. One thread sets it, the other checks it. My belief is that this is safe for booleans, but I would love to hear where I'm wrong. I have seen this particular technique quite often before. The only thing one needs to watch out for is to declare such a boolean volatile (which I believe this code does, if memory serves). Otherwise the thread which polls the condition may read from a cached location and miss the change. Worst that can happen on a hypothecial architecture where writing a byte is not atomic is that the changed condition takes affect one loop cycle later. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Potential patch for building webkit on Visual Studio 2010
On Dec 15, 2010, at 10:00 AM, Jean-Luc Brouillet wrote: if this broader discussion should also be done within the bug tracking system. Yes, this discussion belongs in a bug. I have some comments on it, and a particular specific suggestion for the short term fix. I am waiting for someone to make the bug report. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
On Dec 15, 2010, at 11:11 AM, Steve Block wrote: Thanks for the reply Brady. The boolean in question isn't both checked and set at the same time - it's not an acquired resource. One thread sets it, the other checks it. I don't follow. If it's set from one thread and checked from another thread without locks, how can you guarantee this (other than with application logic)? I don't understand your question - how can you guarantee this? Additionally, while there isn't a guard specifically around the boolean, there is the m_syncLock Mutex which is implicitly guarding operations on this bool, so the thread safety of a bool? argument becomes somewhat moot. Ah, so you're saying that there's application logic using m_syncLock to prevent the boolean from being written and read at the same time? I didn't spot that. It doesn't matter if it's written and read at the same time in this case - but after every write on the main thread, the background thread is signaled to read, so that read will always happen after the write. ~Brady ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
The boolean in question isn't both checked and set at the same time - it's not an acquired resource. One thread sets it, the other checks it. I don't follow. If it's set from one thread and checked from another thread without locks, how can you guarantee this (other than with application logic)? I don't understand your question - how can you guarantee this? I meant 'how can you guarantee that the boolean isn't both checked and set at the same time, given that you don't use locks'. But it looks like you're saying that this is guaranteed by higher-level application logic, as you describe below. It doesn't matter if it's written and read at the same time in this case - but after every write on the main thread, the background thread is signaled to read, so that read will always happen after the write. OK, makes sense. Thanks, Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
Thanks for the reply Brady. The boolean in question isn't both checked and set at the same time - it's not an acquired resource. One thread sets it, the other checks it. I don't follow. If it's set from one thread and checked from another thread without locks, how can you guarantee this (other than with application logic)? Additionally, while there isn't a guard specifically around the boolean, there is the m_syncLock Mutex which is implicitly guarding operations on this bool, so the thread safety of a bool? argument becomes somewhat moot. Ah, so you're saying that there's application logic using m_syncLock to prevent the boolean from being written and read at the same time? I didn't spot that. I have seen this particular technique quite often before. The only thing one needs to watch out for is to declare such a boolean volatile (which I believe this code does, if memory serves). Otherwise the thread which polls the condition may read from a cached location and miss the change. The boolean in question isn't volatile. This isn't a problem because as I mentioned, there's no requirement for a given write to be picked up by a particular read. If a read 'misses' a write, we'll pick it up later. Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Potential patch for building webkit on Visual Studio 2010
https://bugs.webkit.org/show_bug.cgi?id=51122 created. Thanks Darin. On Wed, Dec 15, 2010 at 11:12 AM, Darin Adler da...@apple.com wrote: On Dec 15, 2010, at 10:00 AM, Jean-Luc Brouillet wrote: if this broader discussion should also be done within the bug tracking system. Yes, this discussion belongs in a bug. I have some comments on it, and a particular specific suggestion for the short term fix. I am waiting for someone to make the bug report. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
On Dec 15, 2010, at 8:47 AM, Brady Eidson wrote: On Dec 15, 2010, at 4:28 AM, Steve Block wrote: I have a question about whether WebCore code makes assumptions about the atomicity of certain read/write operations. The particular instance I'm interested in is IconDatabase, where m_threadTerminationRequested is written to from the main thread (in close() for example) and read from the DB thread (in syncThreadMainLoop()) without any locking. It looks like the code is written such that there's no particular synchronization requirement in terms of the order in which the value is set and read. But there's a risk of a garbled read in the case of a simultaneous read and write from the two threads. I assume that the lack of locking is an intentional decision, based on the fact that on all relevant architectures, a boolean read/write is atomic? Correct - or, at least, at the time it was correct. Have you discovered a relevant architecture where it is no longer correct? Is this a general WebCore policy? It would be great if somebody could confirm this, or point out why I'm wrong. I wouldn't be surprised if there were other examples of an unprotected thread shared boolean in either WebCore or any platform's WebKit. I *would* be surprised if there was an example of *any other data type* that was shared between threads unprotected. That said, I don't think we have any general WebCore policies about multithread practices. There might be value in such a document. One possibility is to check the perf impact os using atomic read/write primitives; if it doesn't have a performance cost, it might be safer to do it that way. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Atomic read/write operations and thread safety
15.12.2010, в 10:36, Stephan Aßmus написал(а): The only thing one needs to watch out for is to declare such a boolean volatile (which I believe this code does, if memory serves). Otherwise the thread which polls the condition may read from a cached location and miss the change. Worst that can happen on a hypothecial architecture where writing a byte is not atomic is that the changed condition takes affect one loop cycle later. The behavior of volatile depends on the compiler - there is no guarantee that a memory barrier will be emitted. Historically, volatile existed not for multiprocessor systems, but for special addresses in address space (like memory mapped I/O), where it is important to actually access the address each time, and not than keep a local variable in a boolean. There is code in WebKit that doesn't care about the other thread immediately seeing the change of a boolean value, and just hopes that it will propagate soon enough in practice. I remember implementing worker thread termination in that way. This code is of course formally wrong. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
