[chromium-dev] Re: Concerning HelpWanted tags
There's actually a lot of useful things in this email for someone to learn if they're seeking to become a more regular contributor. On Thu, Apr 30, 2009 at 10:29 PM, Mohamed Mansour m0.interact...@gmail.comwrote: I usually browse the issue tracker for HelpWanted tags, and try to solve those bugs. It would be great if more HelpWanted tags would be tagged, so we can have more variety of bugs to fix. You've been contributing for a while. Have you fixed enough bugs that you can start tackling more significant issues more aggressively? HelpWanted is generally used as a starter bug tag, so the hope is that people begin really learning an area deeply enough to take important things. It would be nice if anyone in the team could go through them and decide whether that bug is _really_ needed. Some of them which I implemented took hours to figure out, and turned out to be as a not needed patch. The lesson is, don't implement things without coordinating ahead of time with the right people. Presence in the bug tracker has little correlation with desirability, since anyone can file anything. It would, indeed, be nice if things in the bug tracker were only things we were sure we wanted, and had good descriptions, and lots of other stuff. We are highly resource constrained, so triaging all 1+ bugs (plus the couple thousand left over in our internal database) is just not feasible. If you want to work on something with a high chance of not being rejected, try to pick bugs marked P1 (or P0...). We definitely have looked at and care about those. A couple of us have vowed to be more aggressive about closing bugs we don't want instead of ignoring them, if we see them. And some of them, the reviewer doesn't have time to review the code because he/she are busy, and then forget to actually review it. I don't believe it is professional for myself to keep spamming the review for an update :) That's what the rest of us do. Don't feel bad. A lot of time spent for just a small request. I sympathize to some degree, but the best way to overcome this is to be so useful that we desire you to become a contributor with commit access. Some people will always review you quickly; some people always slowly; and some will stick you in their queue based on how worthwhile they think it's likely to be. Can't do much about the first two groups, but becoming a committer says a lot to the third. I am just wondering if it is worth contributing patches anymore. We all have day jobs, and external contributors are usually students or people who come home after work and code some more (like me). I would like it if there was a mentor for dedicated newcomers, think of it as a free developer for the chromium engineer. That way, when we do patches (iterations) we don't feel left out. It's really a cost-benefit thing. We have a couple external people who have rapidly become contributors due to very high-quality work, a lot of initiative, and good judgment. We have other people who are not moving along that path as rapidly. Are there people in the second group that (a) are good enough that it would be valuable to have them as committers, but we'll lose them if we don't do something, and (b) are _more_ valuable as committers than the mentor was doing whatever job he was doing before he started doing more mentoring work? The answer to this is not clear to me. Our take so far seems to have been we have our heads down, and we'll try to be helpful to a point, but you need to be self-starting to really thrive here. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Setting Default Search Engine
On Apr 30, 10:16 pm, Mohamed Mansour m0.interact...@gmail.com wrote:
Chromium doesn't support window.external.AddSearchProvider , it supports
the OpenSearch specification.
Except that IS the OpenSearch specification:
Aside from using autodiscovery links, both IE7 and Firefox 2 can
be pointed to your OpenSearch Description document using a javascript
call: window.external.AddSearchProvider(http://mysite.org/odd.xml;);
See http://www.opensearch.org/Documentation/Developer_best_practices_guide
It is also the same function being called on http://mycroft.mozdev.org
where Chrome is working and popping up a window to add an engine:
function addOpenSearch(name,ext,cat,pid,meth)
{
if ((typeof window.external == object) ((typeof
window.external.AddSearchProvider == unknown) || (typeof
window.external.AddSearchProvider == function))) {
// See bugs 430058/430067/430070 for Camino
if (((typeof window.external.AddSearchProvider == unknown) ||
(window.navigator.vendor == 'Camino')) meth == p) {
alert(This plugin uses POST which is not currently supported by
your browser's implementation of OpenSearch.);
} else {
window.external.AddSearchProvider(
http://mycroft.mozdev.org/installos.php/; + pid + / + name
+ .xml);
}
} else {
alert(You will need a browser which supports OpenSearch to
install this plugin.);
}
}
(I tested locally on Chrome and it gets to the line with
window.external.AddSearchProvider.)
So what javascript works with Chrome???
Or what is wrong with my xml file on Chrome???
Can someone explicitly spell it out? This isn't Apple; it's not a big
secret is it? ;-)
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Nomination for chromium account?
Hi, I'm Shinichiro, working on chromium recently, and fixing chromium bugs to learn the code base of chromium. As I finished 13 patches (http://dev.chromium.org/getting-involved/become-a-committer says that 10-20 patches are required), I'm sending this email to be a committer or a provisional committer. So, I'd like some of you to nominate me as a chromium developer or support the nomination. Some of my changes were relatively short. You may think I need more works to be a committer. If so, I want to be a provisional committer. Asking chromium developers to check my changes in is a bit uncomfortable as I feel I'm spoiling productivity of them. I worked on 10 bugs, fixed 9 of them, and finished chromium side change of the last one bug (we need a small change for webkit to finish this bug. Brett is saying that he will do this with his change). I finished 13 reviews by fixing these bugs. Here is the list of my changes I've done. If any other information is needed, please let me know. * Emphasize https for URL like view-source:https://example.com in Omnibox - Bug: https:// http://crbug.com/2349 - Review: http://codereview.chromium.org/62094 * Trivial typo fix - Review: http://codereview.chromium.org/67123 * Implement file_util::CountFilesCreatedAfter for POSIX - Bug: http://crbug.com/9833 - Review: -- http://codereview.chromium.org/75033 -- http://codereview.chromium.org/87003 -- http://codereview.chromium.org/87039 * Allow users to copy javascript: links -- Bug: http://crbug.com/2725 -- Review: http://codereview.chromium.org/91002 * Selection right click - Bug: http://crbug.com/7297 - Review: http://codereview.chromium.org/92047 * Fix canvas's 'lighter' composite operator - Bug: http://crbug.com/1619 - Review: http://codereview.chromium.org/93093 NOTE: WebKit change is needed to fix this bug, but not yet done. * Make JS's alert box copyable - Bug: http://crbug.com/5879 - Review: http://codereview.chromium.org/93112 * Stop debugger by ESC - Bug: http://crbug.com/6890 - Review: http://codereview.chromium.org/92116 * Kill tasks by 'E' - Bug: http://crbug.com/7229 - Review: http://codereview.chromium.org/93135 * Word wrapping in JS message box - Bug: http://crbug.com/2441 - Review: http://codereview.chromium.org/100013 * Fix FocusManager to handle accelerators properly - Bug: http://crbug.com/11073 - Review: http://codereview.chromium.org/99161 Thanks! --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Chrome closes the window without warning
Hi, I am not sure this is the right forum, but I think its a bug. If there are multiple tabs open and you hit the window close button, it does not warn and closes the window. Cannot call it a feature as there are tabs where I would be working actively and may hit the close button accidently or call CTRL+f4 - amitabh --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Discussion to take over #chrome on irc
On Fri, May 1, 2009 at 1:02 AM, Peter Kasting pkast...@chromium.org wrote: ... And since there seems little benefit in registering #chrome just to redirect it to #chromium I don't see why we'd do this. Frankly anyone able to get on IRC is able to find the right place for support. ... Some people don't even know what Chromium is, and want to just type /join #chrome on Freenode and have it work. A member of #chrome told me that lots of people try to join, mistakenly thinking it's a Google Chrome support channel. We don't need to make people to look on the Web to find out what channel to join. We should make #chrome Just Work, especially since it is so easy to do. -- Jason Spiro: software/web developer, packager, trainer, IT consultant. I support Linux, UNIX, Windows, and more. Contact me to discuss your needs. +1 (416) 992-3445 / www.jspiro.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Chrome closes the window without warning
humour I'd suggest to stop using a mouse and only use the shortcut keys: alt-tab, ctrl-tab, ctrl-f4. Never ever use alt-f4! /humour Feature requests like this are best sent to chromium-discuss@ unless you actually want to implement it. But instead, please star this feature request; you'll save everyone's time: http://code.google.com/p/chromium/issues/detail?id=1820 Thanks for your comprehension, M-A On Thu, Apr 30, 2009 at 9:29 PM, Amitabh sai...@gmail.com wrote: Hi, I am not sure this is the right forum, but I think its a bug. If there are multiple tabs open and you hit the window close button, it does not warn and closes the window. Cannot call it a feature as there are tabs where I would be working actively and may hit the close button accidently or call CTRL+f4 - amitabh --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Discussion to take over #chrome on irc
I think part of Peter's point is that we *don't want* people looking for support in #chromium. It's a developer channel. Our ad-hoc filtering to keep support-seekers out seems to be working fine. I have no desire to change that. On Fri, May 1, 2009 at 1:41 AM, Jason A. Spiro jasonspi...@gmail.com wrote: We don't need to make people to look on the Web to find out what channel to join. We should make #chrome Just Work, especially since it is so easy to do. -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Need help in finding a performance problem...
Salut again, I got a bit further, but not quite there yet... The (0, 0, 17, 17) invalidation request came from the fact that WebKit invalidates the scroll bars when their enabled state is set, and it was done before the frame rect of the scroll bar is set... So I fixed that (by simply reversing the call order) and so we save a few extraneous invalidations less... But that didn't make much of a difference in the timing unfortunately. Then I tried to pass an array of bitmaps and rects (as opposed to a union of rects) to the paint rect IPC message, and it cut off 100ms of my 3.2 seconds scenario (so down to 3.1s), but it also cuts off some time off of the scenario without the resizer rect (which went from about 2.72s to roughly 2.68s)... So these don't seem to be THE reason for this slow down... So I keep digging... But I wonder if it would be worth it or not to commit these changes anyway, since they do provide a small performance improvement (at least in the scenario I'm working with, would be interesting to compare to other scenarios, I may try that later today)... Again, thanks for any hints if you have some... ;-) BYE MAD On Thu, Apr 30, 2009 at 2:50 PM, Marc-Andre Decoste m...@chromium.orgwrote: As an intermediate point of reference, looking at the calls to RenderWidget::DidInvalidateRect() and tracing the cases where the new rect doesn't intersect with the current paint_rect, I get the following results: Without the resizer rect: (0, 0, 743, 633) not in (0, 0, 0, 0) (362, 8, 1, 1) not in (0, 0, 0, 0) (726, 0, 17, 609) not in (0, 0, 0, 0) (0, 0, 17, 17) not in (0, 0, 0, 0) With the resizer rect: (0, 0, 743, 633) not in (0, 0, 0, 0) (362, 8, 1, 1) not in (0, 0, 0, 0) (726, 0, 17, 592) not in (0, 0, 0, 0) (726, 592, 17, 17) not in (0, 0, 0, 0) (0, 0, 17, 17) not in (726, 592, 17, 17) (21, 149, 12, 25) not in (0, 0, 0, 0) So we do get a few more, and the scariest is the second last one, where we get the two extreme corners of the window... I'll try to see if this could actually be a bug where the position of the invalidation is wrong and the intent was to get it at (726, 592) but it wasn't offset properly... But since we also have that same (0, 0, 17, 17) request in the no resizer rect scenario, I doubt it... Otherwise, I'll try to compute the timings with a new paint message that receives an array of rects instead of a union... BYE MAD On Thu, Apr 30, 2009 at 1:59 PM, Marc-Andre Decoste m...@chromium.orgwrote: Salut Adam, this is a theory that I'm currently validating... And I will try to change the IPC message code to confirm that it will resolve the problem... So I guess you don't see any problem in this approach... So if I succeed, now I know who to ask for a code review :-) Thanks! BYE MAD On Thu, Apr 30, 2009 at 1:44 PM, Adam Langley a...@chromium.org wrote: On Thu, Apr 30, 2009 at 7:51 AM, Marc-Andre Decoste m...@chromium.org wrote: An alternative could be to send a bitmap the size of the union rect, but only paint the individual rects in it, and extract them individually on the other side of the IPC... But I wonder if it would be worth the added complexity and risk... Unless I missed something (which is most probably the case :-)... Forgive me if I'm misunderstanding here. But is the problem that the area of the union rectangle is significantly greater than the areas of the actually damaged regions, thus we're painting too much? If that's the case, we could well change the PaintRect and ScrollRect messages to carry a vector of rects and have them arranged in sequence in the TransportDIB. Since I'm currently to blame for much of the IPC painting code, I can do this if it'll be of benefit. AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: No more commits to third_party/WebKit
Not yet. There's a small bunch of people still landing unforkages. :DG On Fri, May 1, 2009 at 8:40 AM, Marc-Antoine Ruel mar...@chromium.org wrote: svn lock? On Fri, May 1, 2009 at 11:35 AM, Dimitri Glazkov dglaz...@chromium.org wrote: We are very, very close to total unforking. In order to facilitate the completion of this process, please refrain from landing any changes in trunk/deps/third_party/WebKit. This holds true even if your patch is already approved upstream. So, to put it simply: NO MOAR THIRD_PARTY/WEBKIT COMMEETS. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: No more commits to third_party/WebKit
svn lock? On Fri, May 1, 2009 at 11:35 AM, Dimitri Glazkov dglaz...@chromium.org wrote: We are very, very close to total unforking. In order to facilitate the completion of this process, please refrain from landing any changes in trunk/deps/third_party/WebKit. This holds true even if your patch is already approved upstream. So, to put it simply: NO MOAR THIRD_PARTY/WEBKIT COMMEETS. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Need help in finding a performance problem...
On Fri, May 1, 2009 at 8:36 AM, Marc-Andre Decoste m...@chromium.org wrote: So these don't seem to be THE reason for this slow down... So I keep digging... But I wonder if it would be worth it or not to commit these changes anyway, since they do provide a small performance improvement (at least in the scenario I'm working with, would be interesting to compare to other scenarios, I may try that later today)... If you have improved page load performance as measured by the page cyclers, it would seem very reasonable to commit it. Though you probably want to run it by Darin first as there are a lot of subtle details to how painting works. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Need help in finding a performance problem...
Salut Evan, thanks, I will do that... And the results seems better than I initially thought... Now I need a better way to validate the results, because I think the high level evaluation I did previously was misleading... Looking at the data a little closer, I seem to have reached a much better performance than I thought, and the scenarios of with and without the resize corner are much closer than I initially thought with these two changes... So I'm going to work a bit more on making sure I have my numbers straight, and then get the gurus to validate my fixes :-) Thanks again! BYE MAD On Fri, May 1, 2009 at 11:54 AM, Evan Martin e...@chromium.org wrote: On Fri, May 1, 2009 at 8:36 AM, Marc-Andre Decoste m...@chromium.org wrote: So these don't seem to be THE reason for this slow down... So I keep digging... But I wonder if it would be worth it or not to commit these changes anyway, since they do provide a small performance improvement (at least in the scenario I'm working with, would be interesting to compare to other scenarios, I may try that later today)... If you have improved page load performance as measured by the page cyclers, it would seem very reasonable to commit it. Though you probably want to run it by Darin first as there are a lot of subtle details to how painting works. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Notification when a WebFrame is destroyed
Hi All, Is there currently a way to be notified when a WebFrame instance is about to be destroyed? The Chromium Embedded Framework needs this notification to properly clean up memory associated with specific WebFrame instances (for example, custom NPObject types bound to the WebFrame via BindToWindowObject()). If this capability doesn't currently exist I propose adding a BeforeDestroyFrame(WebFrame*) method to WebViewDelegate that will be called from WebFrameLoaderClient::frameLoaderDestroyed(). Thanks, Marshall --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Notification when a WebFrame is destroyed
Perhaps WebViewDelegate::WindowObjectCleared works? -Darin On Fri, May 1, 2009 at 9:10 AM, Marshall Greenblatt magreenbl...@gmail.comwrote: Hi All, Is there currently a way to be notified when a WebFrame instance is about to be destroyed? The Chromium Embedded Framework needs this notification to properly clean up memory associated with specific WebFrame instances (for example, custom NPObject types bound to the WebFrame via BindToWindowObject()). If this capability doesn't currently exist I propose adding a BeforeDestroyFrame(WebFrame*) method to WebViewDelegate that will be called from WebFrameLoaderClient::frameLoaderDestroyed(). Thanks, Marshall --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Need help in finding a performance problem...
On Fri, May 1, 2009 at 9:06 AM, Marc-Andre Decoste m...@chromium.org wrote: Salut Evan, thanks, I will do that... And the results seems better than I initially thought... If you get performance improvements, please do commit :) Evan is correct that Darin needs to check this over, but I'll happy code review everything where I can. AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Unpacking Extensions and the Sandbox
Right now, we are unpacking extensions in the browser process. This basically consists of unzipping the package into a directory structure and parsing a JSON manifest. Both of these things feel like things we should not be doing in the browser. Additionally, extensions can contains PNG images that will be used in the browser process, for example, for themes. Decoding these images also shouldn't be done in the browser process. I'm looking for advice on how best to sandbox all of this. Here are my current thoughts: To me, the conceptually simplest solution would be to do all of the unpacking in whichever renderer happened to be the one that the user clicked Install in. In the case of autoupdate, we'd use the extension's own process, which are also just renderers. The browser would tell the renderer about the zip file that needed to be unpacked, and the renderer would unzip it, parse it, and decode images into bitmaps, which would all be shipped back to the browser. The immediate practical problem with this approach is that the zip library we use works in terms of files, not memory. This could be changed, but I am not sure how good an idea that is since packages could be large. Average Firefox extensions are ~300k, but we are planning for a max of 1M. Maybe the renderers could be allowed to have a temporary directory they are allowed to do work in? The browser could put the zip file there and they could be unpacked in place? Another orthogonal idea I have heard kicked around is a separate utility process. This seems like it would have the same problems with how to get the data in and out, though, and I don't see why bother having a new process when we already have a renderer we could use. Looking forward to your brilliant ideas, - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Setting Default Search Engine
Do you have a working page you can point me at so that I can
definitively tell you why your script isn't working?
Here's some reasons why it might fail:
. You already have a search engine for http://url.com/.
. The name assigned to the keyword is already in use and the user has
modified the existing keyword with the same name.
. Your url uses POST.
-Scott
On Fri, May 1, 2009 at 2:45 AM, stylo~ scootrs@gmail.com wrote:
On Apr 30, 10:16 pm, Mohamed Mansour m0.interact...@gmail.com wrote:
Chromium doesn't support window.external.AddSearchProvider , it supports
the OpenSearch specification.
Except that IS the OpenSearch specification:
Aside from using autodiscovery links, both IE7 and Firefox 2 can
be pointed to your OpenSearch Description document using a javascript
call: window.external.AddSearchProvider(http://mysite.org/odd.xml;);
See http://www.opensearch.org/Documentation/Developer_best_practices_guide
It is also the same function being called on http://mycroft.mozdev.org
where Chrome is working and popping up a window to add an engine:
function addOpenSearch(name,ext,cat,pid,meth)
{
if ((typeof window.external == object) ((typeof
window.external.AddSearchProvider == unknown) || (typeof
window.external.AddSearchProvider == function))) {
// See bugs 430058/430067/430070 for Camino
if (((typeof window.external.AddSearchProvider == unknown) ||
(window.navigator.vendor == 'Camino')) meth == p) {
alert(This plugin uses POST which is not currently supported by
your browser's implementation of OpenSearch.);
} else {
window.external.AddSearchProvider(
http://mycroft.mozdev.org/installos.php/; + pid + / + name
+ .xml);
}
} else {
alert(You will need a browser which supports OpenSearch to
install this plugin.);
}
}
(I tested locally on Chrome and it gets to the line with
window.external.AddSearchProvider.)
So what javascript works with Chrome???
Or what is wrong with my xml file on Chrome???
Can someone explicitly spell it out? This isn't Apple; it's not a big
secret is it? ;-)
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: No more commits to third_party/WebKit
This should still work fine. One person can lock the whole directory, then people who need to commit unforkage can lock the specific files they need to unfork using --force. That said, the only forkage that happens these days is when doing a webkit merge. What should the merger do if they find themselves needing to change files in third_party/WebKit (e.g. files in the platform/chromium directories)? Ojan On Fri, May 1, 2009 at 8:46 AM, Dimitri Glazkov dglaz...@chromium.orgwrote: Not yet. There's a small bunch of people still landing unforkages. :DG On Fri, May 1, 2009 at 8:40 AM, Marc-Antoine Ruel mar...@chromium.org wrote: svn lock? On Fri, May 1, 2009 at 11:35 AM, Dimitri Glazkov dglaz...@chromium.org wrote: We are very, very close to total unforking. In order to facilitate the completion of this process, please refrain from landing any changes in trunk/deps/third_party/WebKit. This holds true even if your patch is already approved upstream. So, to put it simply: NO MOAR THIRD_PARTY/WEBKIT COMMEETS. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Notification when a WebFrame is destroyed
Patch is available for review here: http://codereview.chromium.org/99283 Thanks, Marshall On Fri, May 1, 2009 at 12:38 PM, Aaron Boodman a...@chromium.org wrote: Not the right time. WindowObjectCleared() gets called as the frame is coming up and the DOM window is getting setup. We needed something like this for extensions, but we ended up just putting our code in WebFrameImpl instead of adding an interface to the delegate. - a On Fri, May 1, 2009 at 9:28 AM, Darin Fisher da...@chromium.org wrote: Perhaps WebViewDelegate::WindowObjectCleared works? -Darin On Fri, May 1, 2009 at 9:10 AM, Marshall Greenblatt magreenbl...@gmail.com wrote: Hi All, Is there currently a way to be notified when a WebFrame instance is about to be destroyed? The Chromium Embedded Framework needs this notification to properly clean up memory associated with specific WebFrame instances (for example, custom NPObject types bound to the WebFrame via BindToWindowObject()). If this capability doesn't currently exist I propose adding a BeforeDestroyFrame(WebFrame*) method to WebViewDelegate that will be called from WebFrameLoaderClient::frameLoaderDestroyed(). Thanks, Marshall --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
On Fri, May 1, 2009 at 10:19 AM, Aaron Boodman a...@chromium.org wrote: Right now, we are unpacking extensions in the browser process. This basically consists of unzipping the package into a directory structure and parsing a JSON manifest. Both of these things feel like things we should not be doing in the browser. Additionally, extensions can contains PNG images that will be used in the browser process, for example, for themes. Decoding these images also shouldn't be done in the browser process. I'm looking for advice on how best to sandbox all of this. Here are my current thoughts: To me, the conceptually simplest solution would be to do all of the unpacking in whichever renderer happened to be the one that the user clicked Install in. In the case of autoupdate, we'd use the extension's own process, which are also just renderers. The browser would tell the renderer about the zip file that needed to be unpacked, and the renderer would unzip it, parse it, and decode images into bitmaps, which would all be shipped back to the browser. The immediate practical problem with this approach is that the zip library we use works in terms of files, not memory. This could be changed, but I am not sure how good an idea that is since packages could be large. Average Firefox extensions are ~300k, but we are planning for a max of 1M. Maybe the renderers could be allowed to have a temporary directory they are allowed to do work in? The browser could put the zip file there and they could be unpacked in place? We've talked about this for a bunch of different reasons, and always pushed back. But maybe the gears team was going to do that anyway? I'm not sure what we decided at the end. Either way, can you modify your zip library to take a file handle instead of a filename? If so, we have everything you need to pass a file handle across processes, or even better, a memory map file. Nicolas Another orthogonal idea I have heard kicked around is a separate utility process. This seems like it would have the same problems with how to get the data in and out, though, and I don't see why bother having a new process when we already have a renderer we could use. Looking forward to your brilliant ideas, - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
On Fri, May 1, 2009 at 10:19 AM, Aaron Boodman a...@chromium.org wrote: Right now, we are unpacking extensions in the browser process. This basically consists of unzipping the package into a directory structure and parsing a JSON manifest. Both of these things feel like things we should not be doing in the browser. Additionally, extensions can contains PNG images that will be used in the browser process, for example, for themes. Decoding these images also shouldn't be done in the browser process. I'm looking for advice on how best to sandbox all of this. Here are my current thoughts: To me, the conceptually simplest solution would be to do all of the unpacking in whichever renderer happened to be the one that the user clicked Install in. In the case of autoupdate, we'd use the extension's own process, which are also just renderers. The browser would tell the renderer about the zip file that needed to be unpacked, and the renderer would unzip it, parse it, and decode images into bitmaps, which would all be shipped back to the browser. For normal extensions where images are always just rendered in HTML, we don't need to do anything special with the images. They'll always be read and rendered in the renderer. The issue with images is with themes, since they're displayed by the browser process. I'm not sure I followed your flow with this. At install time, you ship decoded images over to the browser process so it can display them. Does it need to re-encode the images itself for storage to disk? Or is it going to need to ask the renderer to decode each time? The immediate practical problem with this approach is that the zip library we use works in terms of files, not memory. This could be changed, but I am not sure how good an idea that is since packages could be large. Average Firefox extensions are ~300k, but we are planning for a max of 1M. I think the max was actually 10M. Perhaps we'd need to implement it as a streaming API. Isn't that kind of logic already in place for audio/video? Maybe the renderers could be allowed to have a temporary directory they are allowed to do work in? The browser could put the zip file there and they could be unpacked in place? Perhaps the renderer could just have read access to the zip file, and then pass the files it's unpacking one-by-one up to the browser. If the zip has any single large files, that gets expensive though. Another orthogonal idea I have heard kicked around is a separate utility process. This seems like it would have the same problems with how to get the data in and out, though, and I don't see why bother having a new process when we already have a renderer we could use. There have been other cases people have brought up where the work being requested wasn't associated with a renderer (PAC parsing for example). With the extension example, I think it could be associated with a renderer, but in some cases, we'd be opening up a new one (say you double click on a crx file). Erik --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Discussion to take over #chrome on irc
On Fri, May 1, 2009 at 2:06 AM, Peter Kasting pkast...@chromium.org wrote: ... IRC is absolutely the wrong method for support. Encouraging that makes everything worse. Why is it a bad method for support? Some people will do things wrong. The number of people who are doing things wrong by joining the wrong IRC channel, a communication method which didn't so much stop being relevant for normal people as much as _never start being relevant_ for normal people, is vanishingly small. ... Correct -- most people don't use IRC -- but you can install a Mibbit web-IRC gateway applet on the Chrome support site, similar to what Mozilla has done on their site. That will allow more non-geeks to use IRC. Also, we can publicly log the channel, to make the archives searchable; also, we can require that people search the forum archives before asking questions in #chrome. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
The issue with images is with themes, since they're displayed by the browser process. The issue with images is also an issue with PageActions, where we want to display icons (handed to us by an extension) inside the Omnibox. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
Thanks for the replies! On Fri, May 1, 2009 at 10:42 AM, Adam Barth aba...@chromium.org wrote: I think we should go with the utility process. We've seen several examples where this would be a useful concept to have. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: There have been other cases people have brought up where the work being requested wasn't associated with a renderer (PAC parsing for example). With the extension example, I think it could be associated with a renderer, but in some cases, we'd be opening up a new one (say you double click on a crx On Fri, May 1, 2009 at 10:48 AM, Ojan Vafai o...@chromium.org wrote: An advantage of the utility process is that it's not tied to the lifetime of the tab, so we don't have to deal with edge cases like when the user closes a tab that's mid-installing an extension. I hadn't thought of the double-click the CRX case. If this is the only example, I'd be willing to lose this feature, to be honest. I think losing an in-process install when the tab that started it goes away would be reasonable behavior. On the other other hand, implementing a utility process doesn't seem like a huge amount of work, and I guess it would be useful for other chromium systems. I guess the bigger issues is getting data in and out of whichever process we do the work in. On Fri, May 1, 2009 at 10:42 AM, Adam Barth aba...@chromium.org wrote: As for the zip libraries, I seem to recall that we can marshal file handles into sandboxed processes, but I'm not an expert on this. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: I think the max was actually 10M. Perhaps we'd need to implement it as a streaming API. Isn't that kind of logic already in place for audio/video? Perhaps the renderer could just have read access to the zip file, and then pass the files it's unpacking one-by-one up to the browser. If the zip has any single large files, that gets expensive though. On Fri, May 1, 2009 at 10:45 AM, Nicolas Sylvain nsylv...@chromium.org wrote: We've talked about this for a bunch of different reasons, and always pushed back. But maybe the gears team was going to do that anyway? I'm not sure what we decided at the end. Either way, can you modify your zip library to take a file handle instead of a filename? If so, we have everything you need to pass a file handle across processes, or even better, a memory map file. We can use DuplicateHandle() to get the input file handle in, but I am not sure what to do about getting the directory sturcture out. I thought perhaps the case with Gears was slightly different, but I'm not sure why. Here, all we would need is a temporary directory (any temporary directory) we could use to do work in. In Gears, we needed access to a particular path. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: For normal extensions where images are always just rendered in HTML, we don't need to do anything special with the images. They'll always be read and rendered in the renderer. The issue with needing to decode images in the package is going to come up for other things. Finnur brought up one example, PageActions. But I forsee us needing to display images that came with the extension in the browser for other reasons. The issue with images is with themes, since they're displayed by the browser process. I'm not sure I followed your flow with this. At install time, you ship decoded images over to the browser process so it can display them. Does it need to re-encode the images itself for storage to disk? Or is it going to need to ask the renderer to decode each time? I was thinking that ideally, the renderer would just unpack the extension into whatever directory structure is useful to us. It could be that part of this would be decode any images into bitmaps that we store along side the extension. Later on, the browser process refers to these. - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Performance impact due to unforking ResourceResponse.h
Team, As part of the global WebKit unforking, I will be rolling out shortly the change to ResourceResponse.h that we put in a while back: http://codereview.chromium.org/29007 We have now completed the investigation and there's no need for this fork anymore. As a result, you will see a memory/speed change in the Intl1 cycler. Do not be alarmed. This is a just a trade-off due to more accurate cache accounting. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
On Fri, May 1, 2009 at 11:17 AM, Aaron Boodman a...@chromium.org wrote: We can use DuplicateHandle() to get the input file handle in, but I am not sure what to do about getting the directory sturcture out. Crazy-talk: Have the renderer unpack the zip into a SQLite database. Architecture-astronaut-talk: Have a virtual filesystem API which you could expose either from the browser to a renderer (chroot-like sandboxing), or from a utility process to a renderer (utility process handles the unzipping). It's only crazy if this problem never comes up again. -scott --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
Utility process is an amenable idea. We do something like that for first-run import as well. Key items, I can think of: 1- Utility process would not display UI (would it?) 2- We can allow a directory to be available for read/write 3- Use IPC for progress / heartbeat In other words pretty much a custom renderer. For the images I am lost. Unless we transcode I don't see the point. Transcoding to a format that we handle well or that is not crazy would mitigate most attacks. Or mandate the image format and do a cursory decoding to validate. On May 1, 11:17 am, Aaron Boodman a...@chromium.org wrote: Thanks for the replies! On Fri, May 1, 2009 at 10:42 AM, Adam Barth aba...@chromium.org wrote: I think we should go with the utility process. We've seen several examples where this would be a useful concept to have. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: There have been other cases people have brought up where the work being requested wasn't associated with a renderer (PAC parsing for example). With the extension example, I think it could be associated with a renderer, but in some cases, we'd be opening up a new one (say you double click on a crx On Fri, May 1, 2009 at 10:48 AM, Ojan Vafai o...@chromium.org wrote: An advantage of the utility process is that it's not tied to the lifetime of the tab, so we don't have to deal with edge cases like when the user closes a tab that's mid-installing an extension. I hadn't thought of the double-click the CRX case. If this is the only example, I'd be willing to lose this feature, to be honest. I think losing an in-process install when the tab that started it goes away would be reasonable behavior. On the other other hand, implementing a utility process doesn't seem like a huge amount of work, and I guess it would be useful for other chromium systems. I guess the bigger issues is getting data in and out of whichever process we do the work in. On Fri, May 1, 2009 at 10:42 AM, Adam Barth aba...@chromium.org wrote: As for the zip libraries, I seem to recall that we can marshal file handles into sandboxed processes, but I'm not an expert on this. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: I think the max was actually 10M. Perhaps we'd need to implement it as a streaming API. Isn't that kind of logic already in place for audio/video? Perhaps the renderer could just have read access to the zip file, and then pass the files it's unpacking one-by-one up to the browser. If the zip has any single large files, that gets expensive though. On Fri, May 1, 2009 at 10:45 AM, Nicolas Sylvain nsylv...@chromium.org wrote: We've talked about this for a bunch of different reasons, and always pushed back. But maybe the gears team was going to do that anyway? I'm not sure what we decided at the end. Either way, can you modify your zip library to take a file handle instead of a filename? If so, we have everything you need to pass a file handle across processes, or even better, a memory map file. We can use DuplicateHandle() to get the input file handle in, but I am not sure what to do about getting the directory sturcture out. I thought perhaps the case with Gears was slightly different, but I'm not sure why. Here, all we would need is a temporary directory (any temporary directory) we could use to do work in. In Gears, we needed access to a particular path. On Fri, May 1, 2009 at 10:48 AM, Erik Kay erik...@chromium.org wrote: For normal extensions where images are always just rendered in HTML, we don't need to do anything special with the images. They'll always be read and rendered in the renderer. The issue with needing to decode images in the package is going to come up for other things. Finnur brought up one example, PageActions. But I forsee us needing to display images that came with the extension in the browser for other reasons. The issue with images is with themes, since they're displayed by the browser process. I'm not sure I followed your flow with this. At install time, you ship decoded images over to the browser process so it can display them. Does it need to re-encode the images itself for storage to disk? Or is it going to need to ask the renderer to decode each time? I was thinking that ideally, the renderer would just unpack the extension into whatever directory structure is useful to us. It could be that part of this would be decode any images into bitmaps that we store along side the extension. Later on, the browser process refers to these. - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Why are pref keys wchar_t's?
Why are our internal pref keys all wchar_t strings? That's pretty wasteful for something the user never sees and doesn't need to be localized. It's really wasteful on Mac and Linux (32bit wchar_t). Is this on anyone's radar to fix? Should I file a bug? -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unpacking Extensions and the Sandbox
On Fri, May 1, 2009 at 11:36 AM, cpu c...@chromium.org wrote: Utility process is an amenable idea. We do something like that for first-run import as well. Key items, I can think of: 1- Utility process would not display UI (would it?) 2- We can allow a directory to be available for read/write 3- Use IPC for progress / heartbeat In other words pretty much a custom renderer. I think it's also important to add the following: 4 - Very little (if any) state in the utility process so that restarting it is trivial. 5 - A design so that sync calls from browser to helper are OK. (If the utility process dies during a call, we can maybe retry and return an error if it crashes again.) As for #2, are you suggesting that the utility process would do all operations in its directory and then the browser process would push things in or pull them out before/after the processing is done? This might be a simple and element way to avoid having some extensive virtual file system layer. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
Is there a place that actually describes when it's appropriate to use which string type, and how to know if we should be fixing code we run across? Is everything just supposed to be string16? -Albert On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org wrote: We have a bunch of places where wchar_ts still exist, and none of them are correct. I think this isn't particular instance isn't likely to b *that* much waste but it definitely would be nice to fix. I fixed command line switch names to be ASCII on the train into work once just 'cause it was bothering me. On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton pinker...@chromium.org wrote: Why are our internal pref keys all wchar_t strings? That's pretty wasteful for something the user never sees and doesn't need to be localized. It's really wasteful on Mac and Linux (32bit wchar_t). Is this on anyone's radar to fix? Should I file a bug? -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
Well, in this case they're not user-visible, so there's no reason for them to not be char*s. Unless I'm missing something obvious. On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑) ajw...@chromium.org wrote: Is there a place that actually describes when it's appropriate to use which string type, and how to know if we should be fixing code we run across? Is everything just supposed to be string16? -Albert On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org wrote: We have a bunch of places where wchar_ts still exist, and none of them are correct. I think this isn't particular instance isn't likely to b *that* much waste but it definitely would be nice to fix. I fixed command line switch names to be ASCII on the train into work once just 'cause it was bothering me. On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton pinker...@chromium.org wrote: Why are our internal pref keys all wchar_t strings? That's pretty wasteful for something the user never sees and doesn't need to be localized. It's really wasteful on Mac and Linux (32bit wchar_t). Is this on anyone's radar to fix? Should I file a bug? -- Mike Pinkerton Mac Weenie pinker...@google.com -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] POSIX: EINTR correctness
On POSIX systems, system calls can be interrupted by signals. In this case,
they'll return EINTR, indicating that the system call needs to be restarted.
(The situation is a little more complicated than this with SA_RESTART, but you
can read man 7 signal if you like.)
The short of it is that you need to catch EINTR and restart the call for these
system calls:
* read, readv, write, writev, ioctl
* open() when dealing with a fifo
* wait*
* Anything socket based (send*, recv*, connect, accept etc)
* flock and lock control with fcntl
* mq_ functions which can block
* futex
* sem_wait (and timed wait)
* pause, sigsuspend, sigtimedwait, sigwaitinfo
* poll, epoll_wait, select and 'p' versions of the same
* msgrcv, msgsnd, semop, semtimedop
* close (although, on Linux, EINTR won't happen here)
* any sleep functions (careful, you need to handle this are restart with
different arguments)
We've been a little sloppy with this until now. Once the tree reopens, I'll be
landing 100225 which adds a macro for dealing with this and corrects every case
of these system calls (that I found).
The macro is HANDLE_EINTR in base/eintr_wrapper.h. It's safe to include on
Windows and is a no-op there.
On POSIX, it uses GCC magic to return the correct type based on the expression
and restarts the system call if it throws EINTR. Here it is:
#define HANDLE_EINTR(x) ({ \
typeof(x) ret; \
do { \
ret = x; \
} while (ret == -1 errno == EINTR); \
ret;\
})
And you can use it like:
HANDLE_EINTR(close(fd));
Or:
ssize_t bytes_read = HANDLE_EINTR(read(fd, buffer, len));
AGL
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
The history is that the Value type, which is the underlying data type used by PrefService used to only have wstring types. This bled into PrefService which caused PrefService to only understand wstring as keys. I'd be happy to see a patch that changed PrefService keys to std::string or char* which we DCHECK as ASCII. I'm worried that we may use user input or URLs as keys in some places and we need to encode more than ASCII in the key, but maybe we should just squash that use case (e.g., I think 'remove from NTP' uses hashes of URLs, which would work fine). tony 2009/5/1 Mike Pinkerton pinker...@chromium.org: Well, in this case they're not user-visible, so there's no reason for them to not be char*s. Unless I'm missing something obvious. On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑) ajw...@chromium.org wrote: Is there a place that actually describes when it's appropriate to use which string type, and how to know if we should be fixing code we run across? Is everything just supposed to be string16? -Albert On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org wrote: We have a bunch of places where wchar_ts still exist, and none of them are correct. I think this isn't particular instance isn't likely to b *that* much waste but it definitely would be nice to fix. I fixed command line switch names to be ASCII on the train into work once just 'cause it was bothering me. On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton pinker...@chromium.org wrote: Why are our internal pref keys all wchar_t strings? That's pretty wasteful for something the user never sees and doesn't need to be localized. It's really wasteful on Mac and Linux (32bit wchar_t). Is this on anyone's radar to fix? Should I file a bug? -- Mike Pinkerton Mac Weenie pinker...@google.com -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
I'm still kind of new here, so forgive me if this is a silly question, but
why do this with a define and not an template function?
On Fri, May 1, 2009 at 12:35 PM, Adam Langley a...@chromium.org wrote:
On POSIX systems, system calls can be interrupted by signals. In this case,
they'll return EINTR, indicating that the system call needs to be
restarted.
(The situation is a little more complicated than this with SA_RESTART, but
you
can read man 7 signal if you like.)
The short of it is that you need to catch EINTR and restart the call for
these
system calls:
* read, readv, write, writev, ioctl
* open() when dealing with a fifo
* wait*
* Anything socket based (send*, recv*, connect, accept etc)
* flock and lock control with fcntl
* mq_ functions which can block
* futex
* sem_wait (and timed wait)
* pause, sigsuspend, sigtimedwait, sigwaitinfo
* poll, epoll_wait, select and 'p' versions of the same
* msgrcv, msgsnd, semop, semtimedop
* close (although, on Linux, EINTR won't happen here)
* any sleep functions (careful, you need to handle this are restart with
different arguments)
We've been a little sloppy with this until now. Once the tree reopens,
I'll be
landing 100225 which adds a macro for dealing with this and corrects every
case
of these system calls (that I found).
The macro is HANDLE_EINTR in base/eintr_wrapper.h. It's safe to include on
Windows and is a no-op there.
On POSIX, it uses GCC magic to return the correct type based on the
expression
and restarts the system call if it throws EINTR. Here it is:
#define HANDLE_EINTR(x) ({ \
typeof(x) ret; \
do { \
ret = x; \
} while (ret == -1 errno == EINTR); \
ret;\
})
And you can use it like:
HANDLE_EINTR(close(fd));
Or:
ssize_t bytes_read = HANDLE_EINTR(read(fd, buffer, len));
AGL
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Discussion to take over #chrome on irc
On Fri, May 1, 2009 at 11:01 AM, Jason A. Spiro jasonspi...@gmail.comwrote: On Fri, May 1, 2009 at 2:06 AM, Peter Kasting pkast...@chromium.org wrote: IRC is absolutely the wrong method for support. Encouraging that makes everything worse. Why is it a bad method for support? Because it's roughly one-to-one. We want something that is roughly one-to-ten-million. This is precisely what all our existing support mechanisms try to achieve. we can require that people search the forum archives before asking questions in #chrome. No, you can _tell_ people to search archives. You can't require that they do, that I know of. If non-Googlers want to help with support, contributing to our existing forums is far better than finding ways to get people to use IRC. The cost/benefit test doesn't come close to passing. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
I once went on a mission to change Value to use UTF-8 strings, and hilariously enough after doing a few of those changes we ended up with string16. Maybe I'll go on another crusade to change Value to use string16... Anyway, the tricky part is that it's the Dictionary Value type forcing wstring. I made the change to allow setting/getting UTF8 strings, however Dictionary still uses wstring keys. As Tony said, this bled into a few other areas (PrefService, JSONReader/Writer). We may get stuck having to support legacy preferences which may have been written to disk with wstring. Andrew 2009/5/1 Tony Chang t...@chromium.org The history is that the Value type, which is the underlying data type used by PrefService used to only have wstring types. This bled into PrefService which caused PrefService to only understand wstring as keys. I'd be happy to see a patch that changed PrefService keys to std::string or char* which we DCHECK as ASCII. I'm worried that we may use user input or URLs as keys in some places and we need to encode more than ASCII in the key, but maybe we should just squash that use case (e.g., I think 'remove from NTP' uses hashes of URLs, which would work fine). tony 2009/5/1 Mike Pinkerton pinker...@chromium.org: Well, in this case they're not user-visible, so there's no reason for them to not be char*s. Unless I'm missing something obvious. On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑) ajw...@chromium.org wrote: Is there a place that actually describes when it's appropriate to use which string type, and how to know if we should be fixing code we run across? Is everything just supposed to be string16? -Albert On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org wrote: We have a bunch of places where wchar_ts still exist, and none of them are correct. I think this isn't particular instance isn't likely to b *that* much waste but it definitely would be nice to fix. I fixed command line switch names to be ASCII on the train into work once just 'cause it was bothering me. On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton pinker...@chromium.org wrote: Why are our internal pref keys all wchar_t strings? That's pretty wasteful for something the user never sees and doesn't need to be localized. It's really wasteful on Mac and Linux (32bit wchar_t). Is this on anyone's radar to fix? Should I file a bug? -- Mike Pinkerton Mac Weenie pinker...@google.com -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Discussion to take over #chrome on irc
#chromium is a channel for Chromium project contributors, not a support channel. We will not be making any changes to direct users there for support. -Ben On Fri, May 1, 2009 at 11:01 AM, Jason A. Spiro jasonspi...@gmail.com wrote: On Fri, May 1, 2009 at 2:06 AM, Peter Kasting pkast...@chromium.org wrote: ... IRC is absolutely the wrong method for support. Encouraging that makes everything worse. Why is it a bad method for support? Some people will do things wrong. The number of people who are doing things wrong by joining the wrong IRC channel, a communication method which didn't so much stop being relevant for normal people as much as _never start being relevant_ for normal people, is vanishingly small. ... Correct -- most people don't use IRC -- but you can install a Mibbit web-IRC gateway applet on the Chrome support site, similar to what Mozilla has done on their site. That will allow more non-geeks to use IRC. Also, we can publicly log the channel, to make the archives searchable; also, we can require that people search the forum archives before asking questions in #chrome. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 12:35 PM, Adam Langley a...@chromium.org wrote:
On POSIX, it uses GCC magic to return the correct type based on the expression
and restarts the system call if it throws EINTR. Here it is:
#define HANDLE_EINTR(x) ({ \
typeof(x) ret; \
do { \
ret = x; \
} while (ret == -1 errno == EINTR); \
ret;\
})
Wow, that's pretty deep magic.
And you can use it like:
HANDLE_EINTR(close(fd));
Isn't it disaster if you say:
HANDLE_EINTR(close(ret));
Adam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: No more commits to third_party/WebKit
Does this also include third_party\WebKit\WebKit\chromium\public\WebKitClient.h? I'm guessing not, since none of those files are in the WebKit repository yet, but just want to double check. On Fri, May 1, 2009 at 8:35 AM, Dimitri Glazkov dglaz...@chromium.orgwrote: We are very, very close to total unforking. In order to facilitate the completion of this process, please refrain from landing any changes in trunk/deps/third_party/WebKit. This holds true even if your patch is already approved upstream. So, to put it simply: NO MOAR THIRD_PARTY/WEBKIT COMMEETS. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:07 PM, Adam Barth aba...@chromium.org wrote: Wow, that's pretty deep magic. And you can use it like: HANDLE_EINTR(close(fd)); Isn't it disaster if you say: HANDLE_EINTR(close(ret)); Yes. Regretfully, C doesn't have hygienic macros. It probably would be a good idea to change ret to a name less likely to conflict... -- Elliot --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 12:53 PM, Jeremy Orlow jor...@google.com wrote:
I'm still kind of new here, so forgive me if this is a silly question, but
why do this with a define and not an template function?
One could imagine a template function:
templatetypename T
T handle_eintr(T a) {
..
}
But when using it as:
handle_eintr(close(fd));
How would one trigger close(fd) to be possibly evalulated multiple
times? I think that's the one thing which precludes a template
function sadly.
AGL
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:13 PM, Elliot Glaysher (Chromium) e...@chromium.org wrote: Yes. Regretfully, C doesn't have hygienic macros. It probably would be a good idea to change ret to a name less likely to conflict... Yep, good point. It's now __eintr_result__. Cheers AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:23 PM, Mike Belshe mbel...@google.com wrote: It's been a while since I dealt with unix signals; but in the work I did, the common trick was to disable signals on all threads except one. Then, you only have to deal with handling signals there. Otherwise, you've pretty much always got trouble because you never know which threads will service a signal. Does this work? My memory is hazy because it has been too long :-) We could try this, but it's a little tricky. glibc likes to use signals internally sometimes. That might have gone away with LinuxThreads, but maybe someone is still using that library? You also have to be sure that no 3rd party code is modifying your signal mask. There's also a place where we use EINTR to interrupt a sleep system call deliberately. Also, I hear noises that we might like to split net/ out so that others could use it. In that case, we would want the code to function in other signal environments. So, it's a good idea, but I think handling EINTR works better for us. Cheers AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:27 PM, Adam Langley a...@chromium.org wrote:
On Fri, May 1, 2009 at 12:53 PM, Jeremy Orlow jor...@google.com wrote:
I'm still kind of new here, so forgive me if this is a silly question,
but
why do this with a define and not an template function?
One could imagine a template function:
templatetypename T
T handle_eintr(T a) {
..
}
But when using it as:
handle_eintr(close(fd));
How would one trigger close(fd) to be possibly evalulated multiple
times? I think that's the one thing which precludes a template
function sadly.
Doh. Completely forgot about that. You'd have to do some silliness like
have a template for each number of parameters to the system call. Obviously
the define's the better solution in this case. :-)
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
There's also a problem if you write something like: HANDLE_EINTR(close(PromptUserForFileDescriptor())); Macros suck. What about something like base::close that's inline and knows how to loop? Adam On Fri, May 1, 2009 at 1:27 PM, Adam Langley a...@chromium.org wrote: On Fri, May 1, 2009 at 1:13 PM, Elliot Glaysher (Chromium) e...@chromium.org wrote: Yes. Regretfully, C doesn't have hygienic macros. It probably would be a good idea to change ret to a name less likely to conflict... Yep, good point. It's now __eintr_result__. Cheers AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] depot_tools is moving!
gcl, gclient and friends are moving from http://src.chromium.org/svn/trunk/depot_tools/ to http://src.chromium.org/svn/trunk/tools/depot_tools/ To help you with the switch, there is now a little script to switch automatically. Just run ** *convert_depot_tools* to convert the depot_tools to the new checkout. Warning: the output of this tool isn't nice. If anything fails, just checkout manually: *svn co **http://src.chromium.org/svn/**trunk/tools/depot_tools*http://src.chromium.org/svn/trunk/tools/depot_tools The end result is the same. The far biggest advantage is that there is only one place for all the scripts (no more platform specific) and you can now send patches directly from your depot_tools; e.g. no need to do a separate checkout and run scons anymore. The old depot_tools is scheduled to be removed on May 12, 2009 and you will be upgraded *automatically* next time you run gclient after that date. Windows-only side-effects: - It won't install svn client nor python if they are found in %PATH%. - If you used svn lately, the old depot_tools version was upgraded to 1.6. If you have svn 1.5 in your %PATH%, you may have trouble working with your checkout. Just removing your old client from the path and run gclient help again. M-A --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:30 PM, Adam Langley a...@chromium.org wrote: On Fri, May 1, 2009 at 1:23 PM, Mike Belshe mbel...@google.com wrote: It's been a while since I dealt with unix signals; but in the work I did, the common trick was to disable signals on all threads except one. Then, you only have to deal with handling signals there. Otherwise, you've pretty much always got trouble because you never know which threads will service a signal. Does this work? My memory is hazy because it has been too long :-) We could try this, but it's a little tricky. glibc likes to use signals internally sometimes. That might have gone away with LinuxThreads, but maybe someone is still using that library? You also have to be sure that no 3rd party code is modifying your signal mask. Agree it can be tricky But if 3rd party code uses signals, you're actually probably screwed already. Signals can arrive on any thread that doesn't mask them. So if 3rd party code relies on signals, other threads can get the signals and need to service them. There's also a place where we use EINTR to interrupt a sleep system call deliberately. Also, I hear noises that we might like to split net/ out so that others could use it. In that case, we would want the code to function in other signal environments. pthread_cond_timedwait() may be a better answer than sleep() if you need the abort capability. Anyway, you certainly don't need a signal for this if you want to avoid signals. So, it's a good idea, but I think handling EINTR works better for us. I'll betcha that signals bite us over and over again in the future until we nix em altogether. :-) Mike Cheers AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
What about NewRunnableFunction() in task.h? Slightly overkill because of heap usage and parameter copy but it is quite strict. It also requires writing the call in a different and less obvious way, which may counter the benefit. M-A On Fri, May 1, 2009 at 4:39 PM, Adam Langley a...@chromium.org wrote: On Fri, May 1, 2009 at 1:32 PM, Adam Barth aba...@chromium.org wrote: There's also a problem if you write something like: HANDLE_EINTR(close(PromptUserForFileDescriptor())); Macros suck. What about something like base::close that's inline and knows how to loop? Yea, the macro will trigger multiple evaluation of the arguments. Having inspected every case in base/, net/ and chrome/, it's not currently a problem. Hopefully the ALL_CAPS name will alert people to the possibility. The alternative is to implement a wrapper for every system call which needs wrapping. I don't much like that, but I'm willing to do it if the balance of option favours it. AGL --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: POSIX: EINTR correctness
On Fri, May 1, 2009 at 1:31 PM, Jeremy Orlow jor...@chromium.org wrote:
On Fri, May 1, 2009 at 1:27 PM, Adam Langley a...@chromium.org wrote:
On Fri, May 1, 2009 at 12:53 PM, Jeremy Orlow jor...@google.com wrote:
I'm still kind of new here, so forgive me if this is a silly question,
but
why do this with a define and not an template function?
One could imagine a template function:
templatetypename T
T handle_eintr(T a) {
..
}
But when using it as:
handle_eintr(close(fd));
How would one trigger close(fd) to be possibly evalulated multiple
times? I think that's the one thing which precludes a template
function sadly.
Creating a set of templates isn't too bad. Looking through the list of
functions agl sent, sendto and recvfrom take hte most arguments, at 6.
IOCtl will be annoying though cause of the vararg.
-Albert
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: CoInitialize gone from the renderer
On Apr 30, 3:26 pm, Evan Martin e...@chromium.org wrote: On Thu, Apr 30, 2009 at 3:13 PM, Peter Kasting pkast...@chromium.org wrote: On Thu, Apr 30, 2009 at 1:50 PM, cpu c...@chromium.org wrote: Inhttp://src.chromium.org/viewvc/chrome?view=revrevision=14983I removed a CoInitialize()/CoUnInitialize() pair in the renderer process of your favorite browser. Does this make starting a renderer process any faster? Looks like it made startup 2ms faster. :P Heh, indeed it did made XP 3% faster and Vista 2.5% faster. I was expecting no speedup because the dlls that get loaded are hot, having been just loaded in the browser process. and to think I once like COM, but I digress... http://build.chromium.org/buildbot/perf/xp-release-dual-core/startup/..., click the point on the far right where the graph jogs down. The closest we have to tracking renderer startup is the new tab startuphttp://build.chromium.org/buildbot/perf/xp-release-dual-core/new-tab-... but that is so slow this change may be lost in the noise. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
+100!! Awesome!!! On Fri, May 1, 2009 at 2:42 PM, Scott Hess sh...@chromium.org wrote: HEROIC! On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Unforking: Canary Bot Lives!
Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.orgwrote: What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Nice! Please do write something for the Chromium blog! We need posts there. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] unforking: 20% perf hit for international page cycler expected soon
One of the few remaining forks is in WebCore/platform/graphics/FontCache.cpp (https://bugs.webkit.org/show_bug.cgi?id=21451). I'll be checking in a change to remove this fork and as such we should expect ~20% perf hit for the international page cycler. The internatonal page cycler test intentionally uses more fonts than users are likely to use, so the perf hit isn't something that users would notice in browsing scenarios. Dave PS Here's the code review url: http://codereview.chromium.org/100276 --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: unforking: 20% perf hit for international page cycler expected soon
The suggestions on that code review are good: we ought to measure how many fonts normal users see, and then pick the cache tuning parameter accordingly. Adam Barth is a good person to ask about how to do this, since he seems to be measuring all sorts of things. On Fri, May 1, 2009 at 3:07 PM, David Levin le...@chromium.org wrote: One of the few remaining forks is in WebCore/platform/graphics/FontCache.cpp (https://bugs.webkit.org/show_bug.cgi?id=21451). I'll be checking in a change to remove this fork and as such we should expect ~20% perf hit for the international page cycler. The internatonal page cycler test intentionally uses more fonts than users are likely to use, so the perf hit isn't something that users would notice in browsing scenarios. Dave PS Here's the code review url: http://codereview.chromium.org/100276 --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
+1 to the blog entry On Fri, May 1, 2009 at 2:44 PM, Evan Martin e...@chromium.org wrote: Truly momentous. You should post the the Chromium blog about it (and why it's meaningful). Great work! On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: No more commits to third_party/WebKit
Right. Changes to WebKit/WebKit/chromium are still allowed, because this is not yet upstream. :DG On Fri, May 1, 2009 at 1:11 PM, John Abd-El-Malek j...@chromium.org wrote: Does this also include third_party\WebKit\WebKit\chromium\public\WebKitClient.h? I'm guessing not, since none of those files are in the WebKit repository yet, but just want to double check. On Fri, May 1, 2009 at 8:35 AM, Dimitri Glazkov dglaz...@chromium.org wrote: We are very, very close to total unforking. In order to facilitate the completion of this process, please refrain from landing any changes in trunk/deps/third_party/WebKit. This holds true even if your patch is already approved upstream. So, to put it simply: NO MOAR THIRD_PARTY/WEBKIT COMMEETS. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
Amazing! Congrats Dimitri :) - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
What day would this momentous occasion have occurred if Dimitri hadn't joined Google on Monday August 4, 2008? * * *Thanks for bailing us out, DG!* * * *:-)* * * *Mike* On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.orgwrote: Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
HEROIC! On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unforking: Canary Bot Lives!
Truly momentous. You should post the the Chromium blog about it (and why it's meaningful). Great work! On Fri, May 1, 2009 at 2:38 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Hello all, This is kind of a momentous occasion. For the first time -- ever, our WebKit Canary bot (the one that pulls directly from WebKit upstream) has built successfully and was able to run tests: http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20(webkit.org)/builds/2937 What does this mean? This means that our unforking efforts have finally paid off -- we can now pull directly from WebKit upstream! Congratulations to all of you who worked and helped during the last 7 months! This was a long run, and despite various setbacks, we have reached this milestone. This also means that the daily merge as we know it is soon to be replaced with a less daunting activity -- WebKit gardening. I am working on the document to define what this will entail. Stay tuned. :DG --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: unforking: 20% perf hit for international page cycler expected soon
2009/5/1 Evan Martin e...@chromium.org The suggestions on that code review are good: we ought to measure how many fonts normal users see, and then pick the cache tuning parameter accordingly. Adam Barth is a good person to ask about how to do this, since he seems to be measuring all sorts of things. That'll be great. As discussed in another thread a while ago, we may also consider adding (replacing existing ones with) new intl page cycler tests for a 'monolingual' (well, not really) user (e.g. 70?% of Japanese pages + 30% English pages.). 7:3 split is arbitrary. Maybe, one cycler with 100% Japanese(or C or K) pages and the second with 70% Hebrew (or Arabic) + 30% English and the third with 40% Hindi + 60% English work. (Russian and Greek wouldn't be different from English/German/French, etc). Jungshik On Fri, May 1, 2009 at 3:07 PM, David Levin le...@chromium.org wrote: One of the few remaining forks is in WebCore/platform/graphics/FontCache.cpp (https://bugs.webkit.org/show_bug.cgi?id=21451). I'll be checking in a change to remove this fork and as such we should expect ~20% perf hit for the international page cycler. The internatonal page cycler test intentionally uses more fonts than users are likely to use, so the perf hit isn't something that users would notice in browsing scenarios. Dave PS Here's the code review url: http://codereview.chromium.org/100276 --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: unforking: 20% perf hit for international page cycler expected soon
On Sat, May 2, 2009 at 6:09 AM, Evan Martin e...@chromium.org wrote: The suggestions on that code review are good: we ought to measure how many fonts normal users see, and then pick the cache tuning parameter accordingly. Adam Barth is a good person to ask about how to do this, since he seems to be measuring all sorts of things. I think the correct answer is # of fonts used per some unit of time. The total number of fonts will monotonically increase over time. Instead, we want to know how many fonts we should keep in the cache, which is temporal. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
Hi, I have done a small patch that converts the pref's to use wstring
instead of wchar_t (everywhere in the code). It is just a few places. The
code compiles. But I would like to get some advice on why some errors occur.
I don't know who would like to CR this...
http://codereview.chromium.org/100291
Basically, if you look at the following:
http://codereview.chromium.org/100291/diff/27/30
http://codereview.chromium.org/100291/diff/27/30For some reason (which I
dont' have knowledge in) that array of keys is returning instead of the
actual key name. Does anyone know why?
A mini example:
const std::wstring kPrefsToObserve[] = {
prefs::kAlternateErrorPagesEnabled,
prefs::kWebKitJavaEnabled,
prefs::kWebKitJavascriptEnabled,
prefs::kWebKitLoadsImagesAutomatically,
prefs::kWebKitPluginsEnabled,
prefs::kWebKitUsesUniversalDetector,
prefs::kWebKitSerifFontFamily,
prefs::kWebKitSansSerifFontFamily,
prefs::kWebKitFixedFontFamily,
prefs::kWebKitDefaultFontSize,
prefs::kWebKitDefaultFixedFontSize,
prefs::kDefaultCharset
};
const int kPrefsToObserveLength = arraysize(kPrefsToObserve);
for (int i = 0; i kPrefsToObserveLength; ++i)
prefs-AddPrefObserver(kPrefsToObserve[i], this);
kPrefsToObserve[i] is returning (empty), any reason?
2009/5/1 Andrew Scherkus scher...@chromium.org
I once went on a mission to change Value to use UTF-8 strings, and
hilariously enough after doing a few of those changes we ended up with
string16. Maybe I'll go on another crusade to change Value to use
string16...
Anyway, the tricky part is that it's the Dictionary Value type forcing
wstring. I made the change to allow setting/getting UTF8 strings, however
Dictionary still uses wstring keys. As Tony said, this bled into a few
other areas (PrefService, JSONReader/Writer). We may get stuck having to
support legacy preferences which may have been written to disk with wstring.
Andrew
2009/5/1 Tony Chang t...@chromium.org
The history is that the Value type, which is the underlying data type
used by PrefService used to only have wstring types. This bled into
PrefService which caused PrefService to only understand wstring as
keys.
I'd be happy to see a patch that changed PrefService keys to
std::string or char* which we DCHECK as ASCII. I'm worried that we
may use user input or URLs as keys in some places and we need to
encode more than ASCII in the key, but maybe we should just squash
that use case (e.g., I think 'remove from NTP' uses hashes of URLs,
which would work fine).
tony
2009/5/1 Mike Pinkerton pinker...@chromium.org:
Well, in this case they're not user-visible, so there's no reason for
them to not be char*s. Unless I'm missing something obvious.
On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑)
ajw...@chromium.org wrote:
Is there a place that actually describes when it's appropriate to use
which
string type, and how to know if we should be fixing code we run
across?
Is everything just supposed to be string16?
-Albert
On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org
wrote:
We have a bunch of places where wchar_ts still exist, and none of them
are correct. I think this isn't particular instance isn't likely to b
*that* much waste but it definitely would be nice to fix.
I fixed command line switch names to be ASCII on the train into work
once just 'cause it was bothering me.
On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton
pinker...@chromium.org
wrote:
Why are our internal pref keys all wchar_t strings? That's pretty
wasteful for something the user never sees and doesn't need to be
localized. It's really wasteful on Mac and Linux (32bit wchar_t).
Is this on anyone's radar to fix? Should I file a bug?
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
A wstring is a C++ wrapper around a wchar_t string.
What Mike was proposing was changing to char*.
2009/5/1 Mohamed Mansour m0.interact...@gmail.com:
Hi, I have done a small patch that converts the pref's to use wstring
instead of wchar_t (everywhere in the code). It is just a few places. The
code compiles. But I would like to get some advice on why some errors occur.
I don't know who would like to CR this...
http://codereview.chromium.org/100291
Basically, if you look at the following:
http://codereview.chromium.org/100291/diff/27/30
For some reason (which I dont' have knowledge in) that array of keys is
returning instead of the actual key name. Does anyone know why?
A mini example:
const std::wstring kPrefsToObserve[] = {
prefs::kAlternateErrorPagesEnabled,
prefs::kWebKitJavaEnabled,
prefs::kWebKitJavascriptEnabled,
prefs::kWebKitLoadsImagesAutomatically,
prefs::kWebKitPluginsEnabled,
prefs::kWebKitUsesUniversalDetector,
prefs::kWebKitSerifFontFamily,
prefs::kWebKitSansSerifFontFamily,
prefs::kWebKitFixedFontFamily,
prefs::kWebKitDefaultFontSize,
prefs::kWebKitDefaultFixedFontSize,
prefs::kDefaultCharset
};
const int kPrefsToObserveLength = arraysize(kPrefsToObserve);
for (int i = 0; i kPrefsToObserveLength; ++i)
prefs-AddPrefObserver(kPrefsToObserve[i], this);
kPrefsToObserve[i] is returning (empty), any reason?
2009/5/1 Andrew Scherkus scher...@chromium.org
I once went on a mission to change Value to use UTF-8 strings, and
hilariously enough after doing a few of those changes we ended up with
string16. Maybe I'll go on another crusade to change Value to use
string16...
Anyway, the tricky part is that it's the Dictionary Value type forcing
wstring. I made the change to allow setting/getting UTF8 strings, however
Dictionary still uses wstring keys. As Tony said, this bled into a few
other areas (PrefService, JSONReader/Writer). We may get stuck having to
support legacy preferences which may have been written to disk with wstring.
Andrew
2009/5/1 Tony Chang t...@chromium.org
The history is that the Value type, which is the underlying data type
used by PrefService used to only have wstring types. This bled into
PrefService which caused PrefService to only understand wstring as
keys.
I'd be happy to see a patch that changed PrefService keys to
std::string or char* which we DCHECK as ASCII. I'm worried that we
may use user input or URLs as keys in some places and we need to
encode more than ASCII in the key, but maybe we should just squash
that use case (e.g., I think 'remove from NTP' uses hashes of URLs,
which would work fine).
tony
2009/5/1 Mike Pinkerton pinker...@chromium.org:
Well, in this case they're not user-visible, so there's no reason for
them to not be char*s. Unless I'm missing something obvious.
On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑)
ajw...@chromium.org wrote:
Is there a place that actually describes when it's appropriate to use
which
string type, and how to know if we should be fixing code we run
across?
Is everything just supposed to be string16?
-Albert
On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org
wrote:
We have a bunch of places where wchar_ts still exist, and none of
them
are correct. I think this isn't particular instance isn't likely to
b
*that* much waste but it definitely would be nice to fix.
I fixed command line switch names to be ASCII on the train into work
once just 'cause it was bothering me.
On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton
pinker...@chromium.org
wrote:
Why are our internal pref keys all wchar_t strings? That's pretty
wasteful for something the user never sees and doesn't need to be
localized. It's really wasteful on Mac and Linux (32bit wchar_t).
Is this on anyone's radar to fix? Should I file a bug?
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
Why wouldn't we just use std::string ? Many places in the code uses
std::string. DictionaryValue needs to be converted as well as many others.
So what do we finally decide, go what Pink stated and use char* or use
std::string.
2009/5/1 Evan Martin e...@chromium.org
A wstring is a C++ wrapper around a wchar_t string.
What Mike was proposing was changing to char*.
2009/5/1 Mohamed Mansour m0.interact...@gmail.com:
Hi, I have done a small patch that converts the pref's to use wstring
instead of wchar_t (everywhere in the code). It is just a few places. The
code compiles. But I would like to get some advice on why some errors
occur.
I don't know who would like to CR this...
http://codereview.chromium.org/100291
Basically, if you look at the following:
http://codereview.chromium.org/100291/diff/27/30
For some reason (which I dont' have knowledge in) that array of keys is
returning instead of the actual key name. Does anyone know why?
A mini example:
const std::wstring kPrefsToObserve[] = {
prefs::kAlternateErrorPagesEnabled,
prefs::kWebKitJavaEnabled,
prefs::kWebKitJavascriptEnabled,
prefs::kWebKitLoadsImagesAutomatically,
prefs::kWebKitPluginsEnabled,
prefs::kWebKitUsesUniversalDetector,
prefs::kWebKitSerifFontFamily,
prefs::kWebKitSansSerifFontFamily,
prefs::kWebKitFixedFontFamily,
prefs::kWebKitDefaultFontSize,
prefs::kWebKitDefaultFixedFontSize,
prefs::kDefaultCharset
};
const int kPrefsToObserveLength = arraysize(kPrefsToObserve);
for (int i = 0; i kPrefsToObserveLength; ++i)
prefs-AddPrefObserver(kPrefsToObserve[i], this);
kPrefsToObserve[i] is returning (empty), any reason?
2009/5/1 Andrew Scherkus scher...@chromium.org
I once went on a mission to change Value to use UTF-8 strings, and
hilariously enough after doing a few of those changes we ended up with
string16. Maybe I'll go on another crusade to change Value to use
string16...
Anyway, the tricky part is that it's the Dictionary Value type forcing
wstring. I made the change to allow setting/getting UTF8 strings,
however
Dictionary still uses wstring keys. As Tony said, this bled into a few
other areas (PrefService, JSONReader/Writer). We may get stuck having
to
support legacy preferences which may have been written to disk with
wstring.
Andrew
2009/5/1 Tony Chang t...@chromium.org
The history is that the Value type, which is the underlying data type
used by PrefService used to only have wstring types. This bled into
PrefService which caused PrefService to only understand wstring as
keys.
I'd be happy to see a patch that changed PrefService keys to
std::string or char* which we DCHECK as ASCII. I'm worried that we
may use user input or URLs as keys in some places and we need to
encode more than ASCII in the key, but maybe we should just squash
that use case (e.g., I think 'remove from NTP' uses hashes of URLs,
which would work fine).
tony
2009/5/1 Mike Pinkerton pinker...@chromium.org:
Well, in this case they're not user-visible, so there's no reason for
them to not be char*s. Unless I'm missing something obvious.
On Fri, May 1, 2009 at 3:02 PM, Albert J. Wong (王重傑)
ajw...@chromium.org wrote:
Is there a place that actually describes when it's appropriate to
use
which
string type, and how to know if we should be fixing code we run
across?
Is everything just supposed to be string16?
-Albert
On Fri, May 1, 2009 at 11:59 AM, Evan Martin e...@chromium.org
wrote:
We have a bunch of places where wchar_ts still exist, and none of
them
are correct. I think this isn't particular instance isn't likely
to
b
*that* much waste but it definitely would be nice to fix.
I fixed command line switch names to be ASCII on the train into
work
once just 'cause it was bothering me.
On Fri, May 1, 2009 at 11:42 AM, Mike Pinkerton
pinker...@chromium.org
wrote:
Why are our internal pref keys all wchar_t strings? That's pretty
wasteful for something the user never sees and doesn't need to be
localized. It's really wasteful on Mac and Linux (32bit wchar_t).
Is this on anyone's radar to fix? Should I file a bug?
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--
Mike Pinkerton
Mac Weenie
pinker...@google.com
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
2009/5/1 Mohamed Mansour m0.interact...@gmail.com: Why wouldn't we just use std::string ? Many places in the code uses std::string. DictionaryValue needs to be converted as well as many others. So what do we finally decide, go what Pink stated and use char* or use std::string. I believe the reason is that you pass predefined constants to the pref functions. The constants are either char or wchar_t, we don't allow global objects like std::string. The caller never has to deal with these parameters, just pass the correct constant, so using string objects doesn't help anything. If we changed to std::string objects, it would require constructing a string object and making a copy through a heap allocation, so it's wasteful for no benefit to use std::strings here. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
