Re: [webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)
[(dev time of maintaining comments) + (risk of outdated comments causing bugs X dev time of fixing resulting bugs)] (dev time gained by more contributors each being more knowledgable) No? On Wed, Jul 11, 2012 at 8:53 AM, Ryosuke Niwa rn...@webkit.org wrote: On Jul 11, 2012 8:43 AM, John Mellor joh...@chromium.org wrote: On Wed, Jul 11, 2012 at 4:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Jul 11, 2012 at 6:54 AM, John Mellor joh...@chromium.org wrote: Even obvious (to some) concepts like InlineBox have subtleties, for example not all inline-level elements have inline boxes. An unambiguous class-level comment could make this clearer, for example: // An inline box represents a rectangle that occurs on a line, corresponding to // all or part of some RenderObject. It must be inline-level and its contents // must participate in its containing inline formatting context. For example a // non-replaced element with a 'display' value of 'inline' generates an inline // box, as does an anonymous inline element (text directly contained inside a // block container element, not inside an inline element). But atomic // inline-level boxes (such as replaced inline-level elements, inline-block // elements, inline-table elements, and ruby elements) are not inline boxes // since they participate in their inline formatting context as a single // opaque box; these are handled by insert class that deals with these. // http://www.w3.org/TR/2011/REC-CSS2-20110607/visuren.html#inline-boxes What's the point of adding this comment when the URL contains all the information? All we need is the URL. If anything, we should be describing the difference between the inline boxes in CSS2.1 and our implementation instead. That would be great! I agree that there's probably limited value in just copy/pasting from specs like I did. Linking to the spec something is based on and describing the differences would add a lot of value. The problem is that we'll then incur the maintenace cost of keeping comments up-to-date and the risk of them getting out-of-date as we have previously discussed. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Misplaced files
Regarding renaming files: The .cpp and .h file names need to correspond with .idl names, which in turn correspond with the interfaces specified in these .idl. The later are standard, user-facing strings. This means that you can't change them without fixing a lot of generation and build rules. If your goal is reducing complexity, this might not be a good idea. On Tue, Aug 31, 2010 at 3:02 AM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Aug 30, 2010 at 5:17 PM, Darin Fisher da...@chromium.org wrote: On Mon, Aug 30, 2010 at 9:11 AM, Maciej Stachowiak m...@apple.com wrote: On Aug 30, 2010, at 8:36 AM, Darin Fisher wrote: On Mon, Aug 30, 2010 at 12:18 AM, Adam Barth aba...@webkit.org wrote: On Fri, Aug 27, 2010 at 8:12 PM, Maciej Stachowiak m...@apple.com wrote: Yes. The file-related stuff should all be in one directory, I think. Ok. I moved the files from WebCore/html to WebCore/fileapi. On Aug 27, 2010, at 6:19 PM, Kinuko Yasuda wrote: We have bunch of FileSystem (which is a part of File API) related files in WebCore/storage/. Maybe we should move them to the new directory too? Are these the files you're talking about? WebCore/storage/DOMFilePath.cpp WebCore/storage/DOMFilePath.h WebCore/storage/DOMFileSystem.cpp WebCore/storage/DOMFileSystem.h WebCore/storage/DOMFileSystem.idl WebCore/storage/FileEntry.cpp WebCore/storage/FileEntry.h WebCore/storage/FileEntry.idl WebCore/storage/FileSystemCallback.h WebCore/storage/FileSystemCallback.idl WebCore/storage/FileSystemCallbacks.cpp WebCore/storage/FileSystemCallbacks.h WebCore/storage/LocalFileSystem.cpp WebCore/storage/LocalFileSystem.h I'm happy to move them to WebCore/fileapi, but I'm also happy for you to do it. Thanks, Adam How about just moving everything into WebCore/storage? This is all storage-related stuff. I think the File API is large enough to deserve its own directory. In fact, it might be worth splitting up the remaining contents of the storage directory too. It is confusing to have large but almost entirely separate APIs all piled into one directory. It is true they are all storage-related, but that is a pretty broad theme, and LocalStorage, SQL Storage, Indexed DB and File API have little or no interaction with each other. Regards, Maciej That's fair. Plus, there are a lot of files in there already. What names should we use? WebSQLDatabase: Like WebSockets, I think the web part is pretty important to keep people from getting confused. 'websqldatabase' seems a bit long though. 'websqldb' maybe? WebStorage: Currently we call this Dom Storage throughout the codebase (including in the ENABLE macro), so we may want to call it domstorage. Like WebSockets and WebSQLDatabase, I think storage with no prefix seems like a generic storage directory so we should probably call it webstorage if domstorage isn't acceptable. Indexed Database API: IndexedDB is what it's commonly called, so a directory of indexeddb seems like the way to go. J ___ 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] Testing changes to CodeGenerator*.pm
I see run-bindings-tests is primary a productivity tool: Faster development, easy debugging, better reviews. But it only works if the golden copies don't fall into disrepair = Hence the need to bake this tool into the general testing framework (run-webkit-tests, pre-submit checks, etc.). It will become a useful (but not perfect) regression tool as a side effect of that. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Introducing the Chromium Build Bots
Hi, We have recently set up 3 new builds bots at build.webkit.org: Chromium-Win-Release, Chromium-Mac-Release Chromium-Linux-Release. These bots build the Chromium Webkit Port http://trac.webkit.org/wiki/Chromium. They don't run layout tests yet, but will in the future. These bots should be used to detect if your webkit commit broke the chromium build, so you can fix it like you'd do with any other port. The Chromium Port's underlying build procedure is very similar to webkit's: *update-webkit --chromium* and *build-webkit --chromium*. These commands should work on a standard checkout of webkit on Mac, Linux or Windows and won't interfere with any other webkit port using the same checkout. The following contributors should be able to help you with hands-on assistance for build-bot boost-age: y...@chromium.org, dglaz...@chromium.org, e...@webkit.org, le...@chromium.org. Please email them or find them on #webkit. Feel free to let us know if there is anything we can improve to make the this collaboration seamless and productive. Best regards, Yaar ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
On Thu, Dec 3, 2009 at 11:36 AM, Jeremy Orlow jor...@chromium.org wrote: On Thu, Dec 3, 2009 at 11:20 AM, Alexey Proskuryakov a...@webkit.orgwrote: On 02.12.2009, at 23:35, Maciej Stachowiak wrote: One possible conservative modification to the rule is that if you have a multi-block if ... else if ... else chain, then if even one body has braces, all should. I think that would tend to reduce rather than increase visual noise, as all the else keywords would line up where right now they look ragged. That would be my preference, too. I would like this. +1 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] trac.webkit.org links via Google.com
Robots.txt can exclude most of the trac site, and then include the sitemap.xml. This way you block most of the junk and only give permission to the important file. All major search engine support sitemap.xml, and those that don't will be blocked by robots.txt. A script could generate sitemap.xml from a local svn checkout of trunk. It will produce one url for each source file (frequency=daily) and one url for every revision (frequency=year). That will cover most of the search requirements. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] trac.webkit.org links via Google.com
The urls in sitemap.xml are not patterns - there are exact urls the search
engine will retrieve.
So, you would blacklist most urls with blanket rules in robots.txt and
whitelist explicit urls in sitemap.xml. e.g. in robots.txt, blacklist
/changeset/*, and in sitemap.xml whitelist all
http://trac.webkit.org/changeset/1http://trac.webkit.org/changeset/#%7Brevision%7D
to http://trac.webkit.org/changeset/6 (It's going to be a big file
alright).
On Tue, Dec 1, 2009 at 11:33 AM, Mark Rowe mr...@apple.com wrote:
On 2009-12-01, at 11:04, Yaar Schnitman wrote:
Robots.txt can exclude most of the trac site, and then include the
sitemap.xml. This way you block most of the junk and only give permission to
the important file. All major search engine support sitemap.xml, and those
that don't will be blocked by robots.txt.
A script could generate sitemap.xml from a local svn checkout of trunk. It
will produce one url for each source file (frequency=daily) and one url for
every revision (frequency=year). That will cover most of the search
requirements.
Forgive me, but this doesn't seem to address the issues that I raised in my
previous message.
To reiterate: We need to allow only an explicit set of URLs to be crawled.
Sitemaps *do not* provide this ability. They expose information about set
of URLs to a crawler, they do not limit the set of URLs that it can crawl.
A robots.txt file *does* provide the ability to limit the set of URLs
that can be crawled.
However, the semantics of robots.txt seem to make it incredibly unwieldy to
expose *only *the content of interest, if it is possible at all. For
instance, to expose
http://trac.webkit.org/changeset/#{revision}http://trac.webkit.org/changeset/#%7Brevision%7D
while preventing http://trac.webkit.org/changeset/#{revision}/#{path} or
http://trac.webkit.org/changeset/#{revision}?http://trac.webkit.org/changeset/#%7Brevision%7D?format=zipnew=#{revision}
from
being crawled. Another example would be exposing
http://trac.webkit.org/browser/#{path}http://trac.webkit.org/browser/#%7Bpath%7D
while preventing http://trac.webkit.org/browser/#{path}?rev=#{revision}
from being crawled.
Is there something that I'm missing?
- Mark
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] trac.webkit.org links via Google.com
A sitemap.xml file is a more modern way of telling Google how to crawl a site and the traffic can be throttled in Google's webmaster tools ( http://www.google.com/webmasters/tools/). Creating a daily script that generates sitemap.xml for webkit's SVN repo should trivial. There are probably trac plugins that do that already. If done right, google crawler shouldn't produce much more load than an average developer doing a daily svn sync. On Mon, Nov 30, 2009 at 11:00 PM, Eric Seidel e...@webkit.org wrote: On Tue, Dec 1, 2009 at 1:52 AM, Mark Rowe mr...@apple.com wrote: rel=nofollow doesn't do what you think it doeshttp://en.wikipedia.org/wiki/Nofollow#What_nofollow_is_not_for. It prevents a link from implying influence. It doesn't prevent the link from being followed and the destination content from being indexed. Good to know. git grep is hard to beat. I totally agree! I just often want trac urls for sharing with others, and assembling them from file paths is annoying sometimes. :) I looked briefly at google.com/codesearch but it doesn't seem to have found svn.webkit.org yet. It claims we should ideally have a codesearch sitemap http://www.google.com/support/webmasters/bin/topic.py?topic=12640but I don't really know much about sitemaps or if that would even be a good idea. I don't see a sitemap listed in robots.txt ( http://www.sitemaps.org/protocol.php#submit_robots), but maybe there is one tucked away somewhere, but I'm pretty clueless on the whole hosting a website thing. :) Thanks again. -eric On Tue, Dec 1, 2009 at 1:41 AM, Mark Rowe mr...@apple.com wrote: On 2009-11-30, at 22:36, Eric Seidel wrote: It's bothered me for a while that I can't just type trac webkit Document.cpp into Google and have it give me a trac link to our Document.cpp page. http://trac.webkit.org/browser/trunk/WebCore/dom/Document.cpp I checked http://trac.macosforge.org/robots.txt tonight and low and behold we disallow browser/ (which is where all these links live). Curious if this is intentional, and if we should change this setting? Web crawler indexing of Trac is seriously painful for the servers involved. The entire SVN history of the repository is accessible. File content. Changes. Annotations. Everything. That's not cheap to compute and serve up. - Mark ___ 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] The Chromium WebKit API
Hi, WebKit/chromium is now a live directory. We have completed committing the bulk of the Chromium WebKit API into WebKit/chromium and this code is now integrated to Chromium. Our next steps, as described below, are to commit a 2nd wave (much smaller) of API additions, and to port DumpRenderTree to the Chromium API. Best regards, Yaar On Thu, Nov 5, 2009 at 12:02 AM, Darin Fisher da...@chromium.org wrote: *Please ignore this if you are not interested in the Chromium WebKit API...* I'm writing to announce that we have finished decoupling the Chromium WebKit API http://src.chromium.org/viewvc/chrome/trunk/src/webkit/api/from the rest of the Chromium repository, and so we are now ready to move it to svn.webkit.org. The plan is for it to live under WebKit/WebKit/chromium/http://trac.webkit.org/browser/trunk/WebKit/chromium/ . *Some background:* Chromium began life using WebCore directly. A layer (named webkit/glue) was added to the Chromium repository to help insulate most of the Chromium repository from the fast moving WebCore codebase. However, that layer grew to have many dependencies on lower layers in the Chromium repository (base, net, etc.), and it was also coded using Google C++ style. For much of the past year (since Feb!), we have been working furiously to eliminate those dependencies and convert to WebKit C++ style so that this glue layer could live in the WebKit repository and thereby provide a clean and stable API to WebCore for consumption by Chromium. The result is something we have been calling our WebKit API. Over the past year, we also upstreamed all of our modifications to WebCore. However, without the corresponding WebKit layer in plain sight, it is often hard to understand some of the PLATFORM(CHROMIUM) code that lives in WebCore. It is long overdue that we contribute our WebKit API layer into svn.webkit.org. *Next steps:* Within the coming days, we plan to commit the Chromium WebKit API into WebKit/WebKit/chromium, and then throw all the requisite switches in the Chromium repository to point the Chromium build at this code. Dimitri Glazkov and Eric Seidel are going to be driving this effort. Thanks guys!! *Future steps:* After the dust has settled with this move, we will still have some chores left to do. There remain a number of WebCore dependencies in the Chromium repository that we plan to eliminate. These will be eliminated by introducing additional interfaces in the Chromium WebKit API. (We did not want to delay the initial commit of the Chromium WebKit API waiting on these changes.) It will now be possible to port DumpRenderTree to the Chromium WebKit API, and this is a task we will undertake in the following months. It'll be great for the Chromium project to move to the same testing infrastructure for layout tests as the rest of the WebKit community! Regards, -Darin ___ 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] git.webkit.org/WebKit.git out of sync from svn repository?
Hi all, I think we have this problem again. Webkit svn is on r50761, but I can only git pull up to r50738. -Yaar On Sun, Nov 8, 2009 at 5:11 PM, Fumitoshi Ukai (鵜飼文敏) u...@chromium.orgwrote: On Sat, Nov 7, 2009 at 6:10 AM, William Siegrist wsiegr...@apple.comwrote: On Nov 5, 2009, at 8:32 PM, Fumitoshi Ukai (鵜飼文敏) wrote: I think svn has r50586 (accodring to trac.webkit.org), but git only pulls r50565now. Even I clone git repository from git://git.webkit.org/WebKit.git again, it only has r50565. Is it out of sync? We had syncing issues yesterday, but they look fixed now. Can you retry? It seems working now. Thanks! ukai -Bill ___ 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] Performance of NamedAttrMap
I encountered a similar (potential) performance problem with style properties (see CSSMutableStyleDeclaration::findPropertyWithId), which are stored in an unordered vector too. A potential solution would be to create a HashMap only for elements / style properties with more than K (5+?) attributes, and only when they are first accessed. Such a hashmap will not replace the vector, but just provide an index to it. On Thu, Oct 29, 2009 at 2:33 PM, Darin Adler da...@apple.com wrote: On Oct 29, 2009, at 2:32 PM, Darin Adler wrote: On Oct 29, 2009, at 2:30 PM, Jens Alfke wrote: Is there any reason this couldn't be optimized to use a HashMap Memory consumption is much greater. or at least binary search? Would make lookups faster but parsing slower. I forgot to mention: I believe the common case for attributes is a very small number of attributes. Having one element with many attributes is quite uncommon. This is one consideration when making improvements and optimizations here. Making sure the pathological case is not terribly slow is good, but we also want the normal case to be super-fast. -- Darin ___ 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
