Re: [chromium-dev] new_tab_ui_warm_test is "flaky"
I have a change out that might make this less flaky. Hopefully. http://codereview.chromium.org/427005 On Fri, Nov 20, 2009 at 6:12 PM, Tim Steele wrote: > This happened several times (at least 6) throughout the day, alternating > with successful passes. > It seems bad (as in an actual error, rather than a FLAKY_ test) if it > actually takes more than 5 seconds for a new tab, warm or otherwise. > > I filed http://crbug.com/28448 as a perf regression -- not exactly sure > what the protocol is as sheriff for this case. > > [ RUN ] NewTabUIStartupTest.PerfWarm > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\startup\feature_startup_test.cc(91): > error: Value of: window->WaitForTabCountToBecome(3, 5000) > Actual: false > Expected: true > [ FAILED ] NewTabUIStartupTest.PerfWarm (28016 ms) > > > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Linux: gold linker users should upgrade to 2.20 soon.
Problem solved. Coincident with using the new linker I started building the Google Chrome build. This uses breakpad, which turns on stabs, which makes everything slower and bigger by a factor of 5. Only do the Chrome build if you need to is the lesson. Dave On Sat, Nov 21, 2009 at 8:39 PM, David Moore wrote: > I checked and I compiled with -O2. I see a similar slowdown in the ar > calls. I also notice that the size of the chrome executable is now 2GB. I > think before the size was closer to 350MB. It was 750MB if you included > webkit symbols. Perhaps something changed about the way we're managing > symbols. > > Dave > > > On Sat, Nov 21, 2009 at 6:50 PM, Antoine Labour wrote: > >> >> >> On Sat, Nov 21, 2009 at 6:24 PM, David Moore wrote: >> >>> Since I did this upgrade my builds have gotten very very slow. A single >>> file change, recompile and relink used to be about 35 seconds. Now it's 2 >>> and a half minutes. As far as I can tell I'm using the right gold. >>> >>> davemo...@dmoorel:~/chrome/src$ ld --version >>> GNU gold (GNU Binutils 2.20) 1.9 >>> >>> Are other people seeing this on linux? >>> >>> Dave >>> >> >> Make sure you compile gold with -O2. That made a huge difference for me >> when I was playing with it, and that wasn't the default for some reason. >> >> Antoine >> >> >>> On Wed, Nov 18, 2009 at 2:47 AM, Lei Zhang wrote: >>> Hi folks, We'll soon be relanding a patch that pass --gc-sections to the linker on Linux. For this to work, you need either GNU ld or a recent version of GNU gold. You can test and see if your copy of gold is new enough by running: /path/to/gold --gc-sections If you see: "fatal error: no input files", _no action_ is required. If you see: "--gc-sections: unknown option", then you _need to upgrade_. If you installed gold through [1], please run it again. I've updated the script at r32315 to install gold 2.20. I've also updated all the Linux build / try bots to gold 2.20. [1] http://src.chromium.org/svn/trunk/src/build/install-build-deps.sh -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev >>> >>> -- >>> Chromium Developers mailing list: chromium-dev@googlegroups.com >>> View archives, change email options, or unsubscribe: >>> http://groups.google.com/group/chromium-dev >>> >> >> > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Linux: gold linker users should upgrade to 2.20 soon.
I checked and I compiled with -O2. I see a similar slowdown in the ar calls. I also notice that the size of the chrome executable is now 2GB. I think before the size was closer to 350MB. It was 750MB if you included webkit symbols. Perhaps something changed about the way we're managing symbols. Dave On Sat, Nov 21, 2009 at 6:50 PM, Antoine Labour wrote: > > > On Sat, Nov 21, 2009 at 6:24 PM, David Moore wrote: > >> Since I did this upgrade my builds have gotten very very slow. A single >> file change, recompile and relink used to be about 35 seconds. Now it's 2 >> and a half minutes. As far as I can tell I'm using the right gold. >> >> davemo...@dmoorel:~/chrome/src$ ld --version >> GNU gold (GNU Binutils 2.20) 1.9 >> >> Are other people seeing this on linux? >> >> Dave >> > > Make sure you compile gold with -O2. That made a huge difference for me > when I was playing with it, and that wasn't the default for some reason. > > Antoine > > >> On Wed, Nov 18, 2009 at 2:47 AM, Lei Zhang wrote: >> >>> Hi folks, >>> >>> We'll soon be relanding a patch that pass --gc-sections to the linker >>> on Linux. For this to work, you need either GNU ld or a recent version >>> of GNU gold. You can test and see if your copy of gold is new enough >>> by running: >>> >>> /path/to/gold --gc-sections >>> >>> If you see: "fatal error: no input files", _no action_ is required. >>> If you see: "--gc-sections: unknown option", then you _need to upgrade_. >>> >>> If you installed gold through [1], please run it again. I've updated >>> the script at r32315 to install gold 2.20. I've also updated all the >>> Linux build / try bots to gold 2.20. >>> >>> [1] http://src.chromium.org/svn/trunk/src/build/install-build-deps.sh >>> >>> -- >>> Chromium Developers mailing list: chromium-dev@googlegroups.com >>> View archives, change email options, or unsubscribe: >>>http://groups.google.com/group/chromium-dev >>> >> >> -- >> Chromium Developers mailing list: chromium-dev@googlegroups.com >> View archives, change email options, or unsubscribe: >> http://groups.google.com/group/chromium-dev >> > > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Linux: gold linker users should upgrade to 2.20 soon.
On Sat, Nov 21, 2009 at 6:24 PM, David Moore wrote: > Since I did this upgrade my builds have gotten very very slow. A single > file change, recompile and relink used to be about 35 seconds. Now it's 2 > and a half minutes. As far as I can tell I'm using the right gold. > > davemo...@dmoorel:~/chrome/src$ ld --version > GNU gold (GNU Binutils 2.20) 1.9 > > Are other people seeing this on linux? > > Dave > Make sure you compile gold with -O2. That made a huge difference for me when I was playing with it, and that wasn't the default for some reason. Antoine > On Wed, Nov 18, 2009 at 2:47 AM, Lei Zhang wrote: > >> Hi folks, >> >> We'll soon be relanding a patch that pass --gc-sections to the linker >> on Linux. For this to work, you need either GNU ld or a recent version >> of GNU gold. You can test and see if your copy of gold is new enough >> by running: >> >> /path/to/gold --gc-sections >> >> If you see: "fatal error: no input files", _no action_ is required. >> If you see: "--gc-sections: unknown option", then you _need to upgrade_. >> >> If you installed gold through [1], please run it again. I've updated >> the script at r32315 to install gold 2.20. I've also updated all the >> Linux build / try bots to gold 2.20. >> >> [1] http://src.chromium.org/svn/trunk/src/build/install-build-deps.sh >> >> -- >> Chromium Developers mailing list: chromium-dev@googlegroups.com >> View archives, change email options, or unsubscribe: >>http://groups.google.com/group/chromium-dev >> > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 5:43 PM, Fabien Tassin wrote: > On Sat, 2009-11-21 at 12:06 -0800, Evan Martin wrote: > > I'd say we break the automated Ubuntu builds every couple of weeks > > (and get an additional report from users at about that same rate). > > I don't mind when my automated daily builds break once in a while, but > when the same error stays there for several days unnoticed by anyone > else, it's a problem. Even if fixes are often trivial, downstream > patches for dailies proved difficult to maintain as patches choke far > too often, creating unnecessary packaging work. So i prefer to report > that to the irc channel, most of the time, someone else got it first, > but not always. > > I often get user complains like "hey, chromium is broken once again!" > "what's wrong?" "i didn't get an update in a while", and a while is > often just 2 or 3 days :P > > To avoid catching just one error per build = per day, i build with > --keep-going, and if often catch 1 or 2 errors, like today (3): > http://paste.ubuntu.com/324804/ (with 1 that has been there for 3 days, > unfixed). > > If you could just make a global werror variable in gyp that could be > passed via GYP_DEFINES, that would sure help. I'd like then keep -Werror > for my dev distro (now lucid, future 10.04 LTS) and disable it for the > other (released) distros, or let you decide based on your whitelisted > compilers. > GYP_DEFINES='werror=' That should work. Antoine -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Linux: gold linker users should upgrade to 2.20 soon.
Since I did this upgrade my builds have gotten very very slow. A single file change, recompile and relink used to be about 35 seconds. Now it's 2 and a half minutes. As far as I can tell I'm using the right gold. davemo...@dmoorel:~/chrome/src$ ld --version GNU gold (GNU Binutils 2.20) 1.9 Are other people seeing this on linux? Dave On Wed, Nov 18, 2009 at 2:47 AM, Lei Zhang wrote: > Hi folks, > > We'll soon be relanding a patch that pass --gc-sections to the linker > on Linux. For this to work, you need either GNU ld or a recent version > of GNU gold. You can test and see if your copy of gold is new enough > by running: > > /path/to/gold --gc-sections > > If you see: "fatal error: no input files", _no action_ is required. > If you see: "--gc-sections: unknown option", then you _need to upgrade_. > > If you installed gold through [1], please run it again. I've updated > the script at r32315 to install gold 2.20. I've also updated all the > Linux build / try bots to gold 2.20. > > [1] http://src.chromium.org/svn/trunk/src/build/install-build-deps.sh > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: >http://groups.google.com/group/chromium-dev > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, 2009-11-21 at 12:06 -0800, Evan Martin wrote: > I'd say we break the automated Ubuntu builds every couple of weeks > (and get an additional report from users at about that same rate). I don't mind when my automated daily builds break once in a while, but when the same error stays there for several days unnoticed by anyone else, it's a problem. Even if fixes are often trivial, downstream patches for dailies proved difficult to maintain as patches choke far too often, creating unnecessary packaging work. So i prefer to report that to the irc channel, most of the time, someone else got it first, but not always. I often get user complains like "hey, chromium is broken once again!" "what's wrong?" "i didn't get an update in a while", and a while is often just 2 or 3 days :P To avoid catching just one error per build = per day, i build with --keep-going, and if often catch 1 or 2 errors, like today (3): http://paste.ubuntu.com/324804/ (with 1 that has been there for 3 days, unfixed). If you could just make a global werror variable in gyp that could be passed via GYP_DEFINES, that would sure help. I'd like then keep -Werror for my dev distro (now lucid, future 10.04 LTS) and disable it for the other (released) distros, or let you decide based on your whitelisted compilers. /Fabien -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Anyone understand how V8 GC works?
I guess I don't really understand why gcPrologue has that
ASSERT(wrapper.IsWeak()) then, unless there's something in the DOMMap code
that is removing objects from the map when they transition to PENDING.
Anyhow, it sounds like it's safe to do something like the following in the
epilogue:
if (type == V8Index::MESSAGE_PORT && !(wrapper.IsWeak() ||
wrapper.IsNearDeath()) {
// Make the port weak again (undo the ClearWeak() in GcPrologue)
wrapper.MakeWeak();
}
On Sat, Nov 21, 2009 at 12:08 PM, Jens Alfke wrote:
>
> On Nov 21, 2009, at 10:40 AM, Drew Wilson wrote:
>
> > Can anyone explain (or point me at some docs) for how the
> gcPrologue/gcEpilogue stuff should work, wrt the state of the underlying
> handles (what assumptions does that code make)? I guess I don't really
> understand when objects are taken out of the DOMMap.
>
> If you're talking about what happens in V8GCController: the prolog/epilog
> stuff that it does is, in my understanding, mostly related to temporarily
> grouping together the handles for objects in the same DOM tree, so that none
> of them will be collected before the whole tree goes away. I don't think the
> code there moves stuff in or out of the DOMMap.
>
> The DOM map is twiddled at other times. Objects are added to the DOM map
> when they need to be returned in JS form to V8, and removed when V8 calls
> back to tell Chrome that a weak handle's been GC'd.
>
> Disclaimer: This code is fairly nasty and even though I've messed with it
> for a month or two I don't feel that I fully understand it, especially the
> map. (I think the map code needs a redesign at some point: for example, it
> really frightens me that the line of code that ref()s a DOM object being
> added to the map is in an entirely different source file from the line that
> eventually deref()s it when it's removed. This makes me nervous about
> potential ref leak bugs.)
>
> —Jens
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
Re: [chromium-dev] gcl upload error
[+chromium-dev] One issue is that even a smoke test wouldn't have caught this problem since as I said, it was windows specific. Maybe it's time to create a try server for depot_tools. :/ M-A On Sat, Nov 21, 2009 at 5:23 PM, Marc-Antoine Ruel wrote: > I totally agree, I fully understand the pain I cause. :) > > In this particular case for example, the tool was broken on windows > only. Since I tested on linux, I didn't see the failure. I also ran > pychecker on the scripts last week and found at 2 lingering errors for > corner cases. > > I have a idea in the interim, I'll package a very small script that > sends me an IM whenever someone gets an exception in these scripts. > This way, the number of affected people should be much lower since > I'll be able to trap these faster, at least for "exception failures". > I'll kick my ass to add a full smoke test this year. > > When I replied to Munjal, I replied only to him by error. I only > realized when Munjal replied to the thread on chromium-dev that I > didn't cc chromium-dev. > > M-A > > On Sat, Nov 21, 2009 at 3:37 PM, John Abd-El-Malek > wrote: >> Marc-Antoine: let me preface this by saying I really appreciate all of your >> work on trybots + gcl, it makes our life better. >> I just hit this myself as well. I'm wondering if I can ask you to have a a >> small test case that you do which involves uploading, changing, and >> committing a change. The reason is I get so many failures, and with the >> size of the development team now, when the tool is broken it impacts a lot >> of engineers. >> Thanks, >> John >> >> On Sat, Nov 21, 2009 at 11:56 AM, Munjal Doshi wrote: >>> >>> Just in case anyone else hit this, it was fixed in 32748. >>> -Munjal >>> >>> On Sat, Nov 21, 2009 at 10:04 AM, Evan Martin wrote: A guess, but it looks like it's attempting to find your email address out of your SVN authorization data, and that the computer you're on has no SVN auth data. Try something like "svn ls --username=mycommitern...@chromium.org svn://..." and log in if it prompts you. On Sat, Nov 21, 2009 at 12:46 AM, Munjal Doshi wrote: > I get the following error when running gcl upload when it tries to send > the > job to try server after successfully uploading files: > Traceback (most recent call last): > File "C:\Tools\depot_tools\gcl.py", line 1273, in ? > sys.exit(main()) > File "C:\Tools\depot_tools\gcl.py", line 1244, in main > UploadCL(change_info, argv[3:]) > File "C:\Tools\depot_tools\gcl.py", line 865, in UploadCL > TryChange(change_info, trychange_args, swallow_exception=True) > File "C:\Tools\depot_tools\gcl.py", line 901, in TryChange > prog='gcl try') > File "C:\Tools\depot_tools\trychange.py", line 526, in TryChange > options.scm.ProcessOptions() > File "C:\Tools\depot_tools\trychange.py", line 177, in ProcessOptions > self.options.email = scm.SVN.GetEmail(gcl.GetRepositoryRoot()) > File "C:\Tools\depot_tools\scm.py", line 450, in GetEmail > for credfile in os.listdir(auth_dir): > WindowsError: [Errno 3] The system cannot find the path specified: > 'C:\\Users\\munjal\\AppData\\Roaming\\auth\\svn.simpl > e/*.*' > Does anyone know what hte problem is? > -Munjal > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev >>> >>> -- >>> Chromium Developers mailing list: chromium-dev@googlegroups.com >>> View archives, change email options, or unsubscribe: >>> http://groups.google.com/group/chromium-dev >> > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] gcl upload error
Thanks, when I replied to Munjal, I only replied to him by error. So the fix is to run "gclient". M-A On Sat, Nov 21, 2009 at 2:56 PM, Munjal Doshi wrote: > Just in case anyone else hit this, it was fixed in 32748. > -Munjal > > On Sat, Nov 21, 2009 at 10:04 AM, Evan Martin wrote: >> >> A guess, but it looks like it's attempting to find your email address >> out of your SVN authorization data, and that the computer you're on >> has no SVN auth data. Try something like "svn ls >> --username=mycommitern...@chromium.org svn://..." and log in if it >> prompts you. >> >> On Sat, Nov 21, 2009 at 12:46 AM, Munjal Doshi >> wrote: >> > I get the following error when running gcl upload when it tries to send >> > the >> > job to try server after successfully uploading files: >> > Traceback (most recent call last): >> > File "C:\Tools\depot_tools\gcl.py", line 1273, in ? >> > sys.exit(main()) >> > File "C:\Tools\depot_tools\gcl.py", line 1244, in main >> > UploadCL(change_info, argv[3:]) >> > File "C:\Tools\depot_tools\gcl.py", line 865, in UploadCL >> > TryChange(change_info, trychange_args, swallow_exception=True) >> > File "C:\Tools\depot_tools\gcl.py", line 901, in TryChange >> > prog='gcl try') >> > File "C:\Tools\depot_tools\trychange.py", line 526, in TryChange >> > options.scm.ProcessOptions() >> > File "C:\Tools\depot_tools\trychange.py", line 177, in ProcessOptions >> > self.options.email = scm.SVN.GetEmail(gcl.GetRepositoryRoot()) >> > File "C:\Tools\depot_tools\scm.py", line 450, in GetEmail >> > for credfile in os.listdir(auth_dir): >> > WindowsError: [Errno 3] The system cannot find the path specified: >> > 'C:\\Users\\munjal\\AppData\\Roaming\\auth\\svn.simpl >> > e/*.*' >> > Does anyone know what hte problem is? >> > -Munjal >> > >> > -- >> > Chromium Developers mailing list: chromium-dev@googlegroups.com >> > View archives, change email options, or unsubscribe: >> > http://groups.google.com/group/chromium-dev > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 12:33 PM, Peter Kasting wrote: > On Sat, Nov 21, 2009 at 12:06 PM, Evan Martin wrote: > >> This works for warnings we know about now, but not warnings that will >> occur in the future, which is the larger problem. >> I'd say we break the automated Ubuntu builds every couple of weeks >> (and get an additional report from users at about that same rate). >> > > What I'm concerned about is that occasionally these warnings may be real > and useful, rather than always being stupid. It seems like each warning > should be something we at least take a look at to make sure it's not a > concern, which is why I'm uneasy just disabling warnings. (This goes for > Jens' compile error case too.) > I agree 100% that those warnings are useful and a life saver sometimes. However, if the buildbots aren't building with that particular version of the compiler, we break it all the time, and never notice it - in particular the person who committed the change isn't responsible for fixing it. I know that first-hand for working on the ARM version (we don't have an ARM buildbot yet, working on it). Especially if committers don't build with that particular version, then any attempt to fix it (if anyone cares enough about it) can only get a rubber-stamp lgtm - and that's potentially dangerous. I once 'fixed' some uninitialized variable error in some code that I didn't know, by initializing it to some value that seemed to make sense, but without a clue whether that was the right thing to do - hopefully the reviewer was sufficiently familiar with the code to actually check that it was the right thing to do. So in practice, enabling -Werror for compilers we're not looking at has about no positive effect for us, and a negative effect for users/potential contributors, so I'll agree with Evan that we should only enable -Werror for compilers we have a buildbot for (but for those, we should absolutely enable it). Antoine > PK > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Re: class has virtual method but non-virtual destructor
On Sat, Nov 21, 2009 at 7:21 AM, rahul wrote:
> Sorry about the late reply(I am in a different timezone and when my
> comment didn't appear after 12 hours of submission, I thought it had
> been pruned).
>
> @Eric Roman
> >What class do you get this error on?
> I actually got fed up of the compilation errors and grabbed a binary
> build for Ubuntu. Other people here do seem to know about this
> possible issue. However, I would be happy to reproduce the issue if
> you would wish so.
>
>
> @Evan Martin
>
> >It turns out
> >for our codebase that is very common (due to lots of "observer"-like
> >patterns), so we decided to not rely on this compiler warning. It's
> >only caused horrible bugs once or twice! :)
> My generated Makefile had -Werror which made the build fail. IMHO
> "horrible bugs once ore twice" are worth the effort to fix them for
> the long term. However, I see that the dev team is divided on the
> issue. From an end-user perspective, it is annoying to have the build
> fail for something which the dev team knows about; The end-user
> doesn't and he is left with the task of investigation:-)
>
> >The correct fix would be to disable this particular warning in
> >build/common.gypi.
> Oh, I didn't know about this. I took the brute force approach of
> finding all files with -Werror in them and deleting them.
> $> awk -F: '{print $1}' <(grep -R '\-Werror' *) | xargs perl -i.bak -
> pe 's/-Werror//g' (Not pretty but works)
>
You can disable -Werror in a simpler way:
export GYP_DEFINES='werror='
gclient runhooks --force
Antoine
> I would prefer this to be part of documentation somewhere.
>
> >However, I'm puzzled why nobody else has encountered the problem
> >you're mentioning.
> >Can you provide some details about your compiler, system configuration,
> etc.?
> I ran into this issue with both my work and home system. I downloaded
> the source tarball and did a "gclient sync --force" after setting
> "GYP_GENERATORS=make".
> Work - RHEL 5, gcc 4.4, python 2.6.4
> Home - Ubuntu 9.10, gcc 4.4, python 2.6.4
>
> @Jacob Mandelson
> > (So, add on -Wno-non-virtual-dtor to get a build.)
> Didn't know about this and didn't bother to read through gcc doc.
> Thanks for the tip.
>
>
> @Marc Mentoval
> >I happen to find this warning very useful, just as I find our policy
> >to make warnings hard errors in our own code helpful.
> Yes, of course. That's a good practice. But what about the end-users
> who have to deal with warnings as hard errors.
>
> >Strictly speaking, the problem isn't precisely that a leak will occur
> >if Sub is larger than Base. The real problem is that Sub::~Sub won't
> >run unless Base declares a virtual destructor.
> My bad. Didn't word it properly.
>
>
> @Peter Kasting
>
> > However, the edit doesn't create or own pointers to its
> >controller, and never deletes its controller, so this abstract class
> doesn't
> >have a virtual destructor. The pattern here is "implements interface X"
> as
> >opposed to the "is a specialized type of an X" pattern of parent and child
> >classes.
> No two opinions about elegance of your design patterns. But it looks
> like sort of black magic to count on "doesn't create or own pointers
> to its
> controller, and never deletes its controller". Of course, it isn't a
> problem in a homogeneous environment where people know about the
> established conventions.
>
> @Mark Mentoval
> >The style guide recommends always providing an empty virtual
> >destructor in interface classes.
> >http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showon...
> >"To make sure all implementations of the interface can be destroyed
> >correctly, they must also declare a virtual destructor (in an
> >exception to the first rule, this should not be pure)."
>
>
> Pardon my naive self, but I still don't get it. Enforcing it won't be
> breaking anything other than:
> 1) Purity of design patter. C++ doesn't have interfaces as first class
> citizens, so using an abstract class with a virtual d'tor as an
> interface doesn't amount to violation(your design guide says so as
> well).
> 2) Some extra overhead which isn't really large - one extra entry in
> vtbl, one extra indirection, possible loss of compiler inlining.
>
> >but again, there are valid uses where this is not strictly required.
> But those usage aren't being hurt by enforcing this except for what
> you guys already agree on.
>
>
>
>
>
>
>
>
>
> --
> Chromium Developers mailing list: chromium-dev@googlegroups.com
> View archives, change email options, or unsubscribe:
>http://groups.google.com/group/chromium-dev
>
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 12:06 PM, Evan Martin wrote: > This works for warnings we know about now, but not warnings that will > occur in the future, which is the larger problem. > I'd say we break the automated Ubuntu builds every couple of weeks > (and get an additional report from users at about that same rate). > What I'm concerned about is that occasionally these warnings may be real and useful, rather than always being stupid. It seems like each warning should be something we at least take a look at to make sure it's not a concern, which is why I'm uneasy just disabling warnings. (This goes for Jens' compile error case too.) PK -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Anyone understand how V8 GC works?
On Sat, Nov 21, 2009 at 12:08 PM, Jens Alfke wrote: > Disclaimer: This code is fairly nasty and even though I've messed with it for > a month or two I don't feel that I fully understand it, especially the map. > (I think the map code needs a redesign at some point: for example, it really > frightens me that the line of code that ref()s a DOM object being added to > the map is in an entirely different source file from the line that eventually > deref()s it when it's removed. This makes me nervous about potential ref leak > bugs.) Some of that is my fault. The map is pretty complicated even though it's trying to do something conceptually simple. If you have ideas for how to make it simpler, I'm happy to discuss them with you. 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: class has virtual method but non-virtual destructor
On Nov 22, 12:51 am, Peter Kasting wrote: > You did note that we're in the process of enforcing precisely what you want > enforced, right? > > PK Yes. In the last post, I didn't see the codereview and was thinking that there still isn't consensus. Just saw the review now. -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Nov 21, 2009, at 10:19 AM, Evan Martin wrote: > We should whitelist known compiler versions that build successfully > with -Werror, and then turn it off for the rest. I did something in this spirit, though in kludgy form, a few weeks ago. After I fixed all the current GCC warnings in WebCore, I turned on -Wall and -Werror in the .gyp file, but only for the Mac platform. It would be great if we had a coherent, organized way to do this kind of thing. (And hopefully we'd keep -Werror enabled for all the platforms we officially build on.) >> There is another fix, which is to disable to warning within the file or >> globally for GCC versions less than X. > > This works for warnings we know about now, but not warnings that will > occur in the future, which is the larger problem. It actually goes beyond warnings, unfortunately. Last week I checked in a patch that turned out to cause a compile error under GCC 4.4 (due to some obscure detail about template instantiation), which I didn't know about because the Linux try bot builds with 4.2. I guess I'm saying that in general we can always have problems with outlying compilers that our official build process doesn't use, even if we don't enable -Werror. I've said 'official' twice above, but I actually don't know what our policy is about various compilers and versions. Is there an explicit list somewhere of what compiler/platform combinations we support, i.e. commit to keep Chromium working with? —Jens -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Anyone understand how V8 GC works?
On Nov 21, 2009, at 10:40 AM, Drew Wilson wrote: > Can anyone explain (or point me at some docs) for how the > gcPrologue/gcEpilogue stuff should work, wrt the state of the underlying > handles (what assumptions does that code make)? I guess I don't really > understand when objects are taken out of the DOMMap. If you're talking about what happens in V8GCController: the prolog/epilog stuff that it does is, in my understanding, mostly related to temporarily grouping together the handles for objects in the same DOM tree, so that none of them will be collected before the whole tree goes away. I don't think the code there moves stuff in or out of the DOMMap. The DOM map is twiddled at other times. Objects are added to the DOM map when they need to be returned in JS form to V8, and removed when V8 calls back to tell Chrome that a weak handle's been GC'd. Disclaimer: This code is fairly nasty and even though I've messed with it for a month or two I don't feel that I fully understand it, especially the map. (I think the map code needs a redesign at some point: for example, it really frightens me that the line of code that ref()s a DOM object being added to the map is in an entirely different source file from the line that eventually deref()s it when it's removed. This makes me nervous about potential ref leak bugs.) —Jens -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 11:59 AM, Peter Kasting wrote: > There is another fix, which is to disable to warning within the file or > globally for GCC versions less than X. GCC exposes a number of different > macros and switches that let you determine the version precisely either from > script or in the preprocessor, and we already have a file that holds > compiler-specific magic. > The reason this would be nice compared to the whitelist proposal is that it > wouldn't produce false negatives. The assumption is that there are only a > small number of these kinds of cases, so enumerating them is not going to do > awful things to our code. This works for warnings we know about now, but not warnings that will occur in the future, which is the larger problem. I'd say we break the automated Ubuntu builds every couple of weeks (and get an additional report from users at about that same rate). -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 10:19 AM, Evan Martin wrote:
> I have been particularly frustrated with gcc warning bugs that have
> been fixed in newer versions of gcc. In older gccs, the following
> code produces a "variable may be used uninitialized" warning depending
> on your optimization settings.
> int x;
> { x = 3; }
> printf("%d\n", x);
> The fix is to put in a redundant initialization of x, but people
> (reasonably) will remove them when working on the code.
There is another fix, which is to disable to warning within the file or
globally for GCC versions less than X. GCC exposes a number of different
macros and switches that let you determine the version precisely either from
script or in the preprocessor, and we already have a file that holds
compiler-specific magic.
The reason this would be nice compared to the whitelist proposal is that it
wouldn't produce false negatives. The assumption is that there are only a
small number of these kinds of cases, so enumerating them is not going to do
awful things to our code.
Random side note: In the final build fix you linked, you used a C-style
"(void)" cast. Can we use a C++-style cast? The style guide frowns on
C-style casts. (This assumes we don't disable the warning via your or my
above methods.)
PK
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Anyone understand how V8 GC works?
On Sat, Nov 21, 2009 at 9:40 PM, Drew Wilson wrote:
> Can anyone explain (or point me at some docs) for how the
> gcPrologue/gcEpilogue stuff should work, wrt the state of the underlying
> handles (what assumptions does that code make)? I guess I don't really
> understand when objects are taken out of the DOMMap. Let's say I have an
> unentangled MessagePort (so, its state is WEAK, and we don't change this in
> gcPrologue) - if garbage collection decides that it can be freed, is it
> actually freed and removed from the DOMMap at that time (so it won't be
> iterated over in gcEpilogue)?
The order of operations here is:
1. GC prologue.
2. Actual GC. Here we determine if an object is alive only because
weak handles reference it. If this is the case, such handles become
"pending".
3. Post GC processing. Here we invoke callbacks on pending weak
handles allowing the binding layer to dispose associated native
objects and the handles. (Note: JS objects are still alive here. They
will be collected on the next GC cycle since then there will be no
handles referencing them.)
4. GC epilogue.
So stuff in weak callbacks happens before the epilogue. But it is also
possible that a callback triggers another (nested) GC before post
processing (and epilogue) for the current one is done.
I haven't really looked into MessagePort problem, but my gut feeling
is that in general we should avoid manipulating handle states in
prologue/epilogue and prefer creating extra links in JS instead.
-- Vitaly
> I'm trying to make sure that if a MessagePort is determined to be
> unreferenced, I don't somehow resurrect it by incorrectly changing its state
> back to WEAK.
> -atw
>
> On Fri, Nov 20, 2009 at 7:41 PM, Adam Barth wrote:
>>
>> I "wrote" most of this code in the sense that I took the old garbage
>> collector design and refactored it to make isolated worlds possible.
>> I don't understand the MessagePorts lifetime issues in detail,
>> however.
>>
>> Adam
>>
>>
>> On Fri, Nov 20, 2009 at 7:02 PM, Drew Wilson
>> wrote:
>> > So I looked into the problem further - the issue was that calling
>> > close()
>> > would still leave MessagePorts in a seemingly entangled state, which
>> > meant
>> > that they would never be freed (turns out this caused some related
>> > issues on
>> > the WebKit side related to delaying worker exit).
>> > I updated the code so that MessagePorts now are disentangled when they
>> > are
>> > close()-d, and now I'm getting a failed assertion in the V8GCPrologue
>> > code
>> > in the WebKit v8 bindings.
>> > Here's the sequence of events:
>> > 1) I hit reload on a page - this forces a GC to clean up the old context
>> > and
>> > related objects.
>> > 2) GCPrologueVisitor is called by our GCPrologueCallback for a set of
>> > objects. It has code that looks like this:
>> > void visitDOMWrapper(void* object, v8::Persistent
>> > wrapper)
>> > {
>> > ASSERT(wrapper.IsWeak());
>> > V8ClassIndex::V8WrapperType type =
>> > V8DOMWrapper::domWrapperType(wrapper);
>> > ...
>> > if (type == V8ClassIndex::MESSAGEPORT) {
>> > MessagePort* port1 = static_cast(object);
>> > if (port1->isEntangled())
>> > wrapper.ClearWeak();
>> > ...
>> > }
>> > The end result is that if MessagePort::isEntangled() returns true, the
>> > MessagePort has its Weak flag cleared, and is not garbage collected.
>> > 3) GC happens
>> > 4) After garbage collection, GCEpilogueVisitor is called via our
>> > GCEpilogueCallback. It walks through all the various nodes, and does
>> > this:
>> > void visitDOMWrapper(void* object, v8::Persistent
>> > wrapper)
>> > {
>> > V8ClassIndex::V8WrapperType type =
>> > V8DOMWrapper::domWrapperType(wrapper);
>> >
>> > if (type == V8ClassIndex::MESSAGEPORT) {
>> > MessagePort* port1 = static_cast(object);
>> > if (port1->isEntangled()) {
>> > // We marked this port as reachable in
>> > GCPrologueVisitor.
>> > Undo this now since the
>> > // port could be not reachable in the future if it gets
>> > disentangled (and also
>> > // GCPrologueVisitor expects to see all handles marked
>> > as
>> > weak).
>> > wrapper.MakeWeak(port1,
>> > &DOMDataStore::weakActiveDOMObjectCallback);
>> > }
>> > ...
>> > }
>> > Looks hunky-dory, except... with my latest change, isEntangled() can now
>> > (correctly) change state when close() is called. What happens in step 3
>> > is
>> > garbage collection causes the ScriptExecutionContext (Document object)
>> > for
>> > the old page to get freed. As part of this, MessagePort::close() is
>> > invoked
>> > on all of the message ports associated with that context. So they are
>> > entangled in step 2, so ClearWeak is called on them, but they *aren't*
>> > entangled in step 4, so MarkWeak is no longer called. The next time we
>> > do a
>> > GC, we die due to th
Re: [chromium-dev] gcl upload error
Just in case anyone else hit this, it was fixed in 32748. -Munjal On Sat, Nov 21, 2009 at 10:04 AM, Evan Martin wrote: > A guess, but it looks like it's attempting to find your email address > out of your SVN authorization data, and that the computer you're on > has no SVN auth data. Try something like "svn ls > --username=mycommitern...@chromium.org svn://..." and log in if it > prompts you. > > On Sat, Nov 21, 2009 at 12:46 AM, Munjal Doshi > wrote: > > I get the following error when running gcl upload when it tries to send > the > > job to try server after successfully uploading files: > > Traceback (most recent call last): > > File "C:\Tools\depot_tools\gcl.py", line 1273, in ? > > sys.exit(main()) > > File "C:\Tools\depot_tools\gcl.py", line 1244, in main > > UploadCL(change_info, argv[3:]) > > File "C:\Tools\depot_tools\gcl.py", line 865, in UploadCL > > TryChange(change_info, trychange_args, swallow_exception=True) > > File "C:\Tools\depot_tools\gcl.py", line 901, in TryChange > > prog='gcl try') > > File "C:\Tools\depot_tools\trychange.py", line 526, in TryChange > > options.scm.ProcessOptions() > > File "C:\Tools\depot_tools\trychange.py", line 177, in ProcessOptions > > self.options.email = scm.SVN.GetEmail(gcl.GetRepositoryRoot()) > > File "C:\Tools\depot_tools\scm.py", line 450, in GetEmail > > for credfile in os.listdir(auth_dir): > > WindowsError: [Errno 3] The system cannot find the path specified: > > 'C:\\Users\\munjal\\AppData\\Roaming\\auth\\svn.simpl > > e/*.*' > > Does anyone know what hte problem is? > > -Munjal > > > > -- > > Chromium Developers mailing list: chromium-dev@googlegroups.com > > View archives, change email options, or unsubscribe: > > http://groups.google.com/group/chromium-dev > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Re: class has virtual method but non-virtual destructor
You did note that we're in the process of enforcing precisely what you want enforced, right? PK -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
Re: [chromium-dev] Anyone understand how V8 GC works?
Can anyone explain (or point me at some docs) for how the
gcPrologue/gcEpilogue stuff should work, wrt the state of the underlying
handles (what assumptions does that code make)? I guess I don't really
understand when objects are taken out of the DOMMap. Let's say I have an
unentangled MessagePort (so, its state is WEAK, and we don't change this in
gcPrologue) - if garbage collection decides that it can be freed, is it
actually freed and removed from the DOMMap at that time (so it won't be
iterated over in gcEpilogue)?
I'm trying to make sure that if a MessagePort is determined to be
unreferenced, I don't somehow resurrect it by incorrectly changing its state
back to WEAK.
-atw
On Fri, Nov 20, 2009 at 7:41 PM, Adam Barth wrote:
> I "wrote" most of this code in the sense that I took the old garbage
> collector design and refactored it to make isolated worlds possible.
> I don't understand the MessagePorts lifetime issues in detail,
> however.
>
> Adam
>
>
> On Fri, Nov 20, 2009 at 7:02 PM, Drew Wilson
> wrote:
> > So I looked into the problem further - the issue was that calling close()
> > would still leave MessagePorts in a seemingly entangled state, which
> meant
> > that they would never be freed (turns out this caused some related issues
> on
> > the WebKit side related to delaying worker exit).
> > I updated the code so that MessagePorts now are disentangled when they
> are
> > close()-d, and now I'm getting a failed assertion in the V8GCPrologue
> code
> > in the WebKit v8 bindings.
> > Here's the sequence of events:
> > 1) I hit reload on a page - this forces a GC to clean up the old context
> and
> > related objects.
> > 2) GCPrologueVisitor is called by our GCPrologueCallback for a set of
> > objects. It has code that looks like this:
> > void visitDOMWrapper(void* object, v8::Persistent
> wrapper)
> > {
> > ASSERT(wrapper.IsWeak());
> > V8ClassIndex::V8WrapperType type =
> > V8DOMWrapper::domWrapperType(wrapper);
> > ...
> > if (type == V8ClassIndex::MESSAGEPORT) {
> > MessagePort* port1 = static_cast(object);
> > if (port1->isEntangled())
> > wrapper.ClearWeak();
> >...
> > }
> > The end result is that if MessagePort::isEntangled() returns true, the
> > MessagePort has its Weak flag cleared, and is not garbage collected.
> > 3) GC happens
> > 4) After garbage collection, GCEpilogueVisitor is called via our
> > GCEpilogueCallback. It walks through all the various nodes, and does
> this:
> > void visitDOMWrapper(void* object, v8::Persistent
> wrapper)
> > {
> > V8ClassIndex::V8WrapperType type =
> > V8DOMWrapper::domWrapperType(wrapper);
> >
> > if (type == V8ClassIndex::MESSAGEPORT) {
> > MessagePort* port1 = static_cast(object);
> > if (port1->isEntangled()) {
> > // We marked this port as reachable in GCPrologueVisitor.
> > Undo this now since the
> > // port could be not reachable in the future if it gets
> > disentangled (and also
> > // GCPrologueVisitor expects to see all handles marked as
> > weak).
> > wrapper.MakeWeak(port1,
> > &DOMDataStore::weakActiveDOMObjectCallback);
> > }
> > ...
> >}
> > Looks hunky-dory, except... with my latest change, isEntangled() can now
> > (correctly) change state when close() is called. What happens in step 3
> is
> > garbage collection causes the ScriptExecutionContext (Document object)
> for
> > the old page to get freed. As part of this, MessagePort::close() is
> invoked
> > on all of the message ports associated with that context. So they are
> > entangled in step 2, so ClearWeak is called on them, but they *aren't*
> > entangled in step 4, so MarkWeak is no longer called. The next time we do
> a
> > GC, we die due to the ASSERT(wrapper->IsWeak()) in GCPrologueVisitor.
> > So I'm not sure exactly what the right fix is here, and I'm hoping that
> > perhaps someone who wrote this code initially could chime in - I'm
> thinking
> > that we should just call MarkWeak on all MessagePorts in GCEpilogue
> (since
> > GCEpilogue wouldn't be called on them if they weren't supposed to be
> weak),
> > but I'm not sure if that will have any adverse side-effects if there are
> > references. For example, if I do this:
> > var p = new MessageChannel().port1;
> > p.close();
> > I'd still like that MessagePort to hang around as long as there's a
> > reachable reference (i.e. "p").
> > Any thoughts from someone more familiar with this code?
> > -atw
> >
> > On Thu, Nov 19, 2009 at 11:07 PM, Mads Ager wrote:
> >>
> >> Let us know what you find Drew. I'll be happy to help figure this out
> as
> >> well.
> >>
> >> -- Mads
> >>
> >> On Thu, Nov 19, 2009 at 11:46 PM, Drew Wilson
> >> wrote:
> >> > There's no bug on this yet - I ran across this when I ran out of
> memory
> >> > in
> >> > my SharedWorkers soak test I was running (
[chromium-dev] whitlisting compilers for -Werror
On Sat, Nov 21, 2009 at 7:21 AM, rahul wrote:
>>I happen to find this warning very useful, just as I find our policy
>>to make warnings hard errors in our own code helpful.
> Yes, of course. That's a good practice. But what about the end-users
> who have to deal with warnings as hard errors.
I have been meaning to bring this up again. I think I proposed this
before but was shot down.
We should whitelist known compiler versions that build successfully
with -Werror, and then turn it off for the rest. Unless we have a
buildbot tracking a given compiler version, it is likely that -Werror
makes us unable to build with that version of the compiler.
Concrete proposal:
- whitelist the compiler version used on our buildbots to always use -Werror
- turn off -Werror by default on other compilers
- make it easy to turn on -Werror back on if someone wants to fight
some warnings
I have been particularly frustrated with gcc warning bugs that have
been fixed in newer versions of gcc. In older gccs, the following
code produces a "variable may be used uninitialized" warning depending
on your optimization settings.
int x;
{ x = 3; }
printf("%d\n", x);
The fix is to put in a redundant initialization of x, but people
(reasonably) will remove them when working on the code.
Here's a patch I've been working on to add these redundant initializers:
http://build.chromium.org/buildbot/try-server/builders/linux/builds/9235/steps/gclient/logs/patch
Smarter compilers already recognize the existing code is correct, but gcc warns.
Some example changes I have made in this area recently:
"This is needed to work around a gcc bug when building with -Os."
(on the older gcc 4.3 only)
http://src.chromium.org/viewvc/chrome?view=rev&revision=32186
"fix a warning in the process_util unittest" (that only came up on
fta's build, not on our builds on ghardy or karmic)
http://src.chromium.org/viewvc/chrome?view=rev&revision=32648
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
Re: [chromium-dev] gcl upload error
A guess, but it looks like it's attempting to find your email address out of your SVN authorization data, and that the computer you're on has no SVN auth data. Try something like "svn ls --username=mycommitern...@chromium.org svn://..." and log in if it prompts you. On Sat, Nov 21, 2009 at 12:46 AM, Munjal Doshi wrote: > I get the following error when running gcl upload when it tries to send the > job to try server after successfully uploading files: > Traceback (most recent call last): > File "C:\Tools\depot_tools\gcl.py", line 1273, in ? > sys.exit(main()) > File "C:\Tools\depot_tools\gcl.py", line 1244, in main > UploadCL(change_info, argv[3:]) > File "C:\Tools\depot_tools\gcl.py", line 865, in UploadCL > TryChange(change_info, trychange_args, swallow_exception=True) > File "C:\Tools\depot_tools\gcl.py", line 901, in TryChange > prog='gcl try') > File "C:\Tools\depot_tools\trychange.py", line 526, in TryChange > options.scm.ProcessOptions() > File "C:\Tools\depot_tools\trychange.py", line 177, in ProcessOptions > self.options.email = scm.SVN.GetEmail(gcl.GetRepositoryRoot()) > File "C:\Tools\depot_tools\scm.py", line 450, in GetEmail > for credfile in os.listdir(auth_dir): > WindowsError: [Errno 3] The system cannot find the path specified: > 'C:\\Users\\munjal\\AppData\\Roaming\\auth\\svn.simpl > e/*.*' > Does anyone know what hte problem is? > -Munjal > > -- > Chromium Developers mailing list: chromium-dev@googlegroups.com > View archives, change email options, or unsubscribe: > http://groups.google.com/group/chromium-dev -- 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: class has virtual method but non-virtual destructor
Sorry about the late reply(I am in a different timezone and when my
comment didn't appear after 12 hours of submission, I thought it had
been pruned).
@Eric Roman
>What class do you get this error on?
I actually got fed up of the compilation errors and grabbed a binary
build for Ubuntu. Other people here do seem to know about this
possible issue. However, I would be happy to reproduce the issue if
you would wish so.
@Evan Martin
>It turns out
>for our codebase that is very common (due to lots of "observer"-like
>patterns), so we decided to not rely on this compiler warning. It's
>only caused horrible bugs once or twice! :)
My generated Makefile had -Werror which made the build fail. IMHO
"horrible bugs once ore twice" are worth the effort to fix them for
the long term. However, I see that the dev team is divided on the
issue. From an end-user perspective, it is annoying to have the build
fail for something which the dev team knows about; The end-user
doesn't and he is left with the task of investigation:-)
>The correct fix would be to disable this particular warning in
>build/common.gypi.
Oh, I didn't know about this. I took the brute force approach of
finding all files with -Werror in them and deleting them.
$> awk -F: '{print $1}' <(grep -R '\-Werror' *) | xargs perl -i.bak -
pe 's/-Werror//g' (Not pretty but works)
I would prefer this to be part of documentation somewhere.
>However, I'm puzzled why nobody else has encountered the problem
>you're mentioning.
>Can you provide some details about your compiler, system configuration, etc.?
I ran into this issue with both my work and home system. I downloaded
the source tarball and did a "gclient sync --force" after setting
"GYP_GENERATORS=make".
Work - RHEL 5, gcc 4.4, python 2.6.4
Home - Ubuntu 9.10, gcc 4.4, python 2.6.4
@Jacob Mandelson
> (So, add on -Wno-non-virtual-dtor to get a build.)
Didn't know about this and didn't bother to read through gcc doc.
Thanks for the tip.
@Marc Mentoval
>I happen to find this warning very useful, just as I find our policy
>to make warnings hard errors in our own code helpful.
Yes, of course. That's a good practice. But what about the end-users
who have to deal with warnings as hard errors.
>Strictly speaking, the problem isn't precisely that a leak will occur
>if Sub is larger than Base. The real problem is that Sub::~Sub won't
>run unless Base declares a virtual destructor.
My bad. Didn't word it properly.
@Peter Kasting
> However, the edit doesn't create or own pointers to its
>controller, and never deletes its controller, so this abstract class doesn't
>have a virtual destructor. The pattern here is "implements interface X" as
>opposed to the "is a specialized type of an X" pattern of parent and child
>classes.
No two opinions about elegance of your design patterns. But it looks
like sort of black magic to count on "doesn't create or own pointers
to its
controller, and never deletes its controller". Of course, it isn't a
problem in a homogeneous environment where people know about the
established conventions.
@Mark Mentoval
>The style guide recommends always providing an empty virtual
>destructor in interface classes.
>http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showon...
>"To make sure all implementations of the interface can be destroyed
>correctly, they must also declare a virtual destructor (in an
>exception to the first rule, this should not be pure)."
Pardon my naive self, but I still don't get it. Enforcing it won't be
breaking anything other than:
1) Purity of design patter. C++ doesn't have interfaces as first class
citizens, so using an abstract class with a virtual d'tor as an
interface doesn't amount to violation(your design guide says so as
well).
2) Some extra overhead which isn't really large - one extra entry in
vtbl, one extra indirection, possible loss of compiler inlining.
>but again, there are valid uses where this is not strictly required.
But those usage aren't being hurt by enforcing this except for what
you guys already agree on.
--
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
Re: [chromium-dev] How to compile Google Chrome with Visual C++ 2008 Express Edition
Ok. First, I'm not sure if you should dynamically link with ATL because it works without on my PC. Second, can you look in VC++ 2005 Express under Tools > Options > Projects and Solutions > Show directories for: Library files and ensure you have added the ATL directory from the Windows Driver Kit there? Something like C:\WinDDK\6001.18002\lib\atl\i386 (may be different on you PC if you downloaded WDK 7). I looked back at my HOWTO and saw I did not include that step. Let me know if it works for you and I'll update it. Thanks! -- Dominic. On Tue, Nov 17, 2009 at 10:51 AM, PhistucK wrote: > OK, so I am trying with Visual C++ 2005 Express. After working around most > of the errors, I got one (hopefully...) left - > fatal error LNK1104: cannot open file ‘atl.lib’ chrome_dll > The last error that I resolved before that, was almost the same, but “atlz” > instead of “atl” in the file name. > So I went to the chrome_dll properties and chose to dynamically link ATL and > it worked. But then I got the above error and I am at a loss. > Any ideas? > ☆PhistucK > > > On Tue, Nov 17, 2009 at 04:03, Marc-Antoine Ruel wrote: >> >> Dominic, you should turn /WX off globally in src/build/common.gypi. >> That would simplify the patch. >> >> I guess the incremental link failure is due to the exe size on a 32 >> bit OS, I'll mark it as a large binary so you can remove that step >> too. >> >> M-A >> >> On Mon, Nov 16, 2009 at 8:58 PM, Bradley Nelson >> wrote: >> > That's good to hear! >> > I see you are mainly just turning off warnings as errors in a few spots. >> > Is this something that we can either gate in the build based on some >> > flag, >> > or are the warnings something that we could fix properly in the source? >> > I'd >> > love to peel off more steps from the process. >> > -BradN >> > On Mon, Nov 16, 2009 at 5:44 PM, Dominic Jodoin >> > >> > wrote: >> >> >> >> Hey Brad! >> >> >> >> I just wanted to let you know that this is working great for me on >> >> VC++ 2008 Express Edition. Thanks a lot! >> >> >> >> I have updated my page to reflect your change. I also added a patch >> >> that modifies some GYP files in such a way that you won’t have to >> >> manually “play” with the project settings in order to compile without >> >> errors (with this compiler that is). >> >> >> >> >> >> >> >> http://cotsog.wordpress.com/2009/11/08/how-to-compile-google-chrome-with-visual-c-2008-express-edition/ >> >> >> >> -- Dominic. >> >> >> >> On Thu, Nov 12, 2009 at 7:51 PM, Bradley Nelson >> >> wrote: >> >> > Ok that fix is in. >> >> > You'd need to set GYP_MSVS_VERSION=2008e >> >> > Let me know how that goes for you. >> >> > -BradN >> >> > >> >> > On Wed, Nov 11, 2009 at 12:50 PM, Marc-Antoine Ruel >> >> > >> >> > wrote: >> >> >> >> >> >> Updated >> >> >> >> >> >> >> >> >> http://sites.google.com/a/chromium.org/dev/developers/how-tos/build-instructions-windows >> >> >> to reference your blog entry. >> >> >> >> >> >> I don't want to copy these instructions since it's too lengthy, >> >> >> inefficient and unsupported. >> >> >> >> >> >> I didn't realize one could download WDK 7 without needing a MSDN >> >> >> account. That's cool. >> >> >> >> >> >> M-A >> >> >> >> >> >> On Tue, Nov 10, 2009 at 10:39 PM, Dominic Jodoin >> >> >> wrote: >> >> >> > On Tue, Nov 10, 2009 at 10:25 PM, Peter Kasting >> >> >> > >> >> >> > wrote: >> >> >> >> >> >> >> >> What do you mean? Or to be more precise, what would considering >> >> >> >> your >> >> >> >> steps >> >> >> >> "a valid setup to contribute" concretely result in? >> >> >> >> PK >> >> >> > >> >> >> > I'm wondering if using a hacked ATL version 7.1 could lead to bugs >> >> >> > given the product is built, I suppose, with ATL coming with Visual >> >> >> > Studio 2005 or 2008 which is a different version. >> >> >> > >> >> >> > But what I meant was that if the steps were to be approved, I >> >> >> > thought >> >> >> > they could be included on http://dev.chromium.org. >> >> >> > >> >> >> > -- Dominic. >> >> >> > >> >> >> > -- >> >> >> > Chromium Developers mailing list: chromium-dev@googlegroups.com >> >> >> > View archives, change email options, or unsubscribe: >> >> >> > http://groups.google.com/group/chromium-dev >> >> >> > >> >> >> >> >> >> -- >> >> >> Chromium Developers mailing list: chromium-dev@googlegroups.com >> >> >> View archives, change email options, or unsubscribe: >> >> >> http://groups.google.com/group/chromium-dev >> >> > >> >> > >> > >> > >> >> -- >> Chromium Developers mailing list: chromium-dev@googlegroups.com >> View archives, change email options, or unsubscribe: >> http://groups.google.com/group/chromium-dev > -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
[chromium-dev] gcl upload error
I get the following error when running gcl upload when it tries to send the job to try server after successfully uploading files: Traceback (most recent call last): File "C:\Tools\depot_tools\gcl.py", line 1273, in ? sys.exit(main()) File "C:\Tools\depot_tools\gcl.py", line 1244, in main UploadCL(change_info, argv[3:]) File "C:\Tools\depot_tools\gcl.py", line 865, in UploadCL TryChange(change_info, trychange_args, swallow_exception=True) File "C:\Tools\depot_tools\gcl.py", line 901, in TryChange prog='gcl try') File "C:\Tools\depot_tools\trychange.py", line 526, in TryChange options.scm.ProcessOptions() File "C:\Tools\depot_tools\trychange.py", line 177, in ProcessOptions self.options.email = scm.SVN.GetEmail(gcl.GetRepositoryRoot()) File "C:\Tools\depot_tools\scm.py", line 450, in GetEmail for credfile in os.listdir(auth_dir): WindowsError: [Errno 3] The system cannot find the path specified: 'C:\\Users\\munjal\\AppData\\Roaming\\auth\\svn.simpl e/*.*' Does anyone know what hte problem is? -Munjal -- Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev
