[chromium-dev] Re: Cannot use gcl change, svn fails. Any help is appreciated ...
Sorry for another post, but now I am trying to commit my changes in Linux (It was working fine, I submitted a patch couple of days ago). Now I can't even do gcl changes command. What should I do to fix up my messups :( I have no experience in svn (only in chromium), so any help is appreciated. m...@m0-desktop:~/code/chromium/src$ gcl changes Traceback (most recent call last): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1149, in module sys.exit(main()) File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1057, in main if not os.path.exists(GetInfoDir()): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 113, in GetInfoDir return os.path.join(GetRepositoryRoot(), '.svn', 'gcl_info') File /mnt/backup/code/chromium/depot_tools/gcl.py, line 99, in GetRepositoryRoot raise Exception(gcl run outside of repository) Exception: gcl run outside of repository m...@m0-desktop:~/code/chromium/src$ On May 27, 1:53 am, Mohamed Mansour m0.interact...@gmail.com wrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour --~--~-~--~~~---~--~~ 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: Cannot use gcl change, svn fails. Any help is appreciated ...
You can debug it! From the backtrace you posted it looks like it's looking for a directory under .svn called gcl_info or something, right? I'd look to see what's there, maybe try moving it to the side temporarily to see if that helps, etc. On Tue, May 26, 2009 at 11:01 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for another post, but now I am trying to commit my changes in Linux (It was working fine, I submitted a patch couple of days ago). Now I can't even do gcl changes command. What should I do to fix up my messups :( I have no experience in svn (only in chromium), so any help is appreciated. m...@m0-desktop:~/code/chromium/src$ gcl changes Traceback (most recent call last): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1149, in module sys.exit(main()) File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1057, in main if not os.path.exists(GetInfoDir()): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 113, in GetInfoDir return os.path.join(GetRepositoryRoot(), '.svn', 'gcl_info') File /mnt/backup/code/chromium/depot_tools/gcl.py, line 99, in GetRepositoryRoot raise Exception(gcl run outside of repository) Exception: gcl run outside of repository m...@m0-desktop:~/code/chromium/src$ On May 27, 1:53 am, Mohamed Mansour m0.interact...@gmail.com wrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour --~--~-~--~~~---~--~~ 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: Cannot use gcl change, svn fails. Any help is appreciated ...
Sorry for not being more detailed in my problem, but I already tried moving the gcl_info cause its not part of svn. And the failure still persists. That is when I tried to msg dev mailing list. I don't want to loose my gcl_info patches, cause some patches of mine are already open, and if I download a new branch, I loose my CL's. -- Mohamed Mansour On Wed, May 27, 2009 at 2:04 AM, Evan Martin e...@chromium.org wrote: You can debug it! From the backtrace you posted it looks like it's looking for a directory under .svn called gcl_info or something, right? I'd look to see what's there, maybe try moving it to the side temporarily to see if that helps, etc. On Tue, May 26, 2009 at 11:01 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for another post, but now I am trying to commit my changes in Linux (It was working fine, I submitted a patch couple of days ago). Now I can't even do gcl changes command. What should I do to fix up my messups :( I have no experience in svn (only in chromium), so any help is appreciated. m...@m0-desktop:~/code/chromium/src$ gcl changes Traceback (most recent call last): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1149, in module sys.exit(main()) File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1057, in main if not os.path.exists(GetInfoDir()): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 113, in GetInfoDir return os.path.join(GetRepositoryRoot(), '.svn', 'gcl_info') File /mnt/backup/code/chromium/depot_tools/gcl.py, line 99, in GetRepositoryRoot raise Exception(gcl run outside of repository) Exception: gcl run outside of repository m...@m0-desktop:~/code/chromium/src$ On May 27, 1:53 am, Mohamed Mansour m0.interact...@gmail.com wrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour --~--~-~--~~~---~--~~ 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: Cannot use gcl change, svn fails. Any help is appreciated ...
Hi Are you using vaguely equivalent versions of svn btw. between windows and linux? Sharing a working copy is always bad news for svn and I suspect horrid issues between different versions of svn on windows and linux or perhaps even cr/lf issues contributed to this problem :( Given that you have the modified source files, you could paste those into a fresh checkout and recreate/copy the gcl info stuff by hand if you wanted to push updates to existing patches in rietveld. The gcl_info stuff doesn't look too hard to fake and I have successfully hand edited the files before to force it to update the right CL when I screwed something up before. I'd paste an example gcl_info changes file here but cut-and-paste doesn't seem to be working in chrome in gmail now :( --Craig On Wed, May 27, 2009 at 8:09 AM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for not being more detailed in my problem, but I already tried moving the gcl_info cause its not part of svn. And the failure still persists. That is when I tried to msg dev mailing list. I don't want to loose my gcl_info patches, cause some patches of mine are already open, and if I download a new branch, I loose my CL's. -- Mohamed Mansour On Wed, May 27, 2009 at 2:04 AM, Evan Martin e...@chromium.org wrote: You can debug it! From the backtrace you posted it looks like it's looking for a directory under .svn called gcl_info or something, right? I'd look to see what's there, maybe try moving it to the side temporarily to see if that helps, etc. On Tue, May 26, 2009 at 11:01 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for another post, but now I am trying to commit my changes in Linux (It was working fine, I submitted a patch couple of days ago). Now I can't even do gcl changes command. What should I do to fix up my messups :( I have no experience in svn (only in chromium), so any help is appreciated. m...@m0-desktop:~/code/chromium/src$ gcl changes Traceback (most recent call last): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1149, in module sys.exit(main()) File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1057, in main if not os.path.exists(GetInfoDir()): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 113, in GetInfoDir return os.path.join(GetRepositoryRoot(), '.svn', 'gcl_info') File /mnt/backup/code/chromium/depot_tools/gcl.py, line 99, in GetRepositoryRoot raise Exception(gcl run outside of repository) Exception: gcl run outside of repository m...@m0-desktop:~/code/chromium/src$ On May 27, 1:53 am, Mohamed Mansour m0.interact...@gmail.com wrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour --~--~-~--~~~---~--~~ 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: Cannot use gcl change, svn fails. Any help is appreciated ...
On Wed, May 27, 2009 at 11:23 AM, Mohamed Mansour m0.interact...@gmail.comwrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. Strangely, it is treating files (.cc) files as directories, and then looking for svn entries within. Maybe you need to delete these files (check if they are there anyway) then run svn revert followed by svn update in that folder (M:\code\chromium\src\chrome\browser\views\options\ ) Note that svn revert will revert all your changes made in that directory and its subdirectories. It might have something to do with browsing ext3 on windows. I use NTFS on linux and fortunately have not faced any such issues. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour -- Arindam Sharma Undergraduate Student Department of Computer Science and Engineering Indian Institute of Technology, Kharagpur --~--~-~--~~~---~--~~ 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: UI_test.exe fails to complete (please help)
thanx wan, after setting chromium as the def browser i now only have 1 test that fails [ RUN ] UnloadTest.BrowserCloseTabWhenOtherTabHasListener ..\..\browser\unload_uitest.cc(349): error: Value of: popup_title Actual: L Expected: std::wstring(Lpopup) Which is: Lpopup [ FAILED ] UnloadTest.BrowserCloseTabWhenOtherTabHasListener (1301 ms) and there are 2 tests which are waiting for my user input as otherwise they don't close all on chromium 16972 i will later re-run the tests and provide more info on the 2 that demanded user manual intervention . --~--~-~--~~~---~--~~ 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: Cannot use gcl change, svn fails. Any help is appreciated ...
Hi, The thing that puzzles me is that gclient sync works fine in both platforms, and the initial gcl change CL works fine. It only fails if I lint, change, or upload that CL. I guess I will just keep a single source, and use gclient/gcl only in linux (while I could build in windows and linux),. -- Mohamed Mansour On Wed, May 27, 2009 at 2:59 AM, Craig Schlenter craig.schlen...@gmail.comwrote: Hi Are you using vaguely equivalent versions of svn btw. between windows and linux? Sharing a working copy is always bad news for svn and I suspect horrid issues between different versions of svn on windows and linux or perhaps even cr/lf issues contributed to this problem :( Given that you have the modified source files, you could paste those into a fresh checkout and recreate/copy the gcl info stuff by hand if you wanted to push updates to existing patches in rietveld. The gcl_info stuff doesn't look too hard to fake and I have successfully hand edited the files before to force it to update the right CL when I screwed something up before. I'd paste an example gcl_info changes file here but cut-and-paste doesn't seem to be working in chrome in gmail now :( --Craig On Wed, May 27, 2009 at 8:09 AM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for not being more detailed in my problem, but I already tried moving the gcl_info cause its not part of svn. And the failure still persists. That is when I tried to msg dev mailing list. I don't want to loose my gcl_info patches, cause some patches of mine are already open, and if I download a new branch, I loose my CL's. -- Mohamed Mansour On Wed, May 27, 2009 at 2:04 AM, Evan Martin e...@chromium.org wrote: You can debug it! From the backtrace you posted it looks like it's looking for a directory under .svn called gcl_info or something, right? I'd look to see what's there, maybe try moving it to the side temporarily to see if that helps, etc. On Tue, May 26, 2009 at 11:01 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Sorry for another post, but now I am trying to commit my changes in Linux (It was working fine, I submitted a patch couple of days ago). Now I can't even do gcl changes command. What should I do to fix up my messups :( I have no experience in svn (only in chromium), so any help is appreciated. m...@m0-desktop:~/code/chromium/src$ gcl changes Traceback (most recent call last): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1149, in module sys.exit(main()) File /mnt/backup/code/chromium/depot_tools/gcl.py, line 1057, in main if not os.path.exists(GetInfoDir()): File /mnt/backup/code/chromium/depot_tools/gcl.py, line 113, in GetInfoDir return os.path.join(GetRepositoryRoot(), '.svn', 'gcl_info') File /mnt/backup/code/chromium/depot_tools/gcl.py, line 99, in GetRepositoryRoot raise Exception(gcl run outside of repository) Exception: gcl run outside of repository m...@m0-desktop:~/code/chromium/src$ On May 27, 1:53 am, Mohamed Mansour m0.interact...@gmail.com wrote: Hi, Today I am trying to submit another patch, and it is not allowing me to do that, it somewhat fails. I am trying to do the following: gcl change 11435 It brings up notepad and I add my stuff as I normally do. Then I save and close notepad. I run the gcl change command again (or even lint). I get these svn issues. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.cc\.svn\entries': The directory name is invalid. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\advanced_contents_view.cc\.svn\entries': The directory name is invalid. I run gcl change 11435 and I see those two files missing, so I do another lint, and I see. svn: Can't open file 'M:\code\chromium\src\chrome\browser\views\options\content_page_view.h\.svn\entries': The directory name is invalid. I do another gcl change 11435 to see whats wrong, and my changed files are not in the list anymore, its like it deleted everything. What may have went wrong? Maybe my setup is incorrect, I am submitting bugs on linux and on windows, and I shared my ext3 linux partition in windows. I installed chromium on linux first, and today I decided to fix a bug, after spending 3 hours on it (refactoring etc) I can't submit the patch. I would really like to fix this. Any suggestions? Its 2 AM EST and IRC seems dead tonight after 10 :(. Thanks! -- Mohamed Mansour --~--~-~--~~~---~--~~ 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: Avoiding crash after autoupdate on Linux
[and now to the list, grr] The problem with that, and with the hard link, is that the package manager cleans up the old version when the update happens, so it's no longer available to run. (And if we tried to keep them around, we'd have a garbage collection problem.) It just doesn't fit well, the package managers have no concept of leave these files installed until this process exits or leave these files installed until the n+1th update. On Wed, May 27, 2009 at 6:55 AM, Marc-Antoine Ruel mar...@chromium.org wrote: *A totally uninformed comment* If a hardlink is an issue due to package manager, why not use a shell script? Could bash not be present on a system with X installed? That'd be surprising. $ cat /usr/bin/chrome #!/bin/sh # Call the right version here: /usr/bin/chrome2.0.1.2 $* No need for zygote with that. M-A On Tue, May 26, 2009 at 6:30 PM, Dan Kegel daniel.r.ke...@gmail.com wrote: On Tue, May 26, 2009 at 1:06 PM, Adam Langley a...@chromium.org wrote: On Tue, May 26, 2009 at 12:00 PM, Dan Kegel daniel.r.ke...@gmail.com wrote: http://codereview.chromium.org/115773 is my try at fixing http://crbug.com/11841 (autoupdate broke my browser, familiar to anyone who's used Firefox on Linux). I haven't cleaned up the code, but it's a lot less invasive than I thought it was going to be. ... However, we could easily make a hardlink with a specific version in the name. ...A patch to use the zygote hammer for the auto-updating issue would first have to show that there's no easier alternatives! Adam agrees that there's an issue with the hardlink idea, so perhaps zygotes are the way to go for the moment after all. BTW, on my work machine, for that patch to work at the moment, I have to invoke it with a real absolute path a la ENABLE_ZYGOTE_MANAGER=1 `/bin/pwd`/sconsbuild/Debug/chrome since my chromium directory is a symlink, and pwd reports the name of the symlink rather than the target. I'll see if I can get rid of the need for invoking with an absolute path. --~--~-~--~~~---~--~~ 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: Avoiding crash after autoupdate on Linux
On Wed, May 27, 2009 at 7:11 AM, Dan Kegel daniel.r.ke...@gmail.com wrote: The problem with that, and with the hard link, is that the package manager cleans up the old version when the update happens, so it's no longer available to run. (And if we tried to keep them around, we'd have a garbage collection problem.) It just doesn't fit well, the package managers have no concept of leave these files installed until this process exits or leave these files installed until the n+1th update. Also, there are issues with shared libraries that we need. An upgrade may bring in a new major version of libxyz and update chromium to match at the same time, but our versioned hardlink is still going to need the old one. What we really want are transient references to the binary and it's shared libraries which we can get by just holding file descriptors to them. However, we would then have to write a userspace fexecve function to build a new child from the references. I think that could actually be done, but it's not going to be nice code :) 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: reminder: don't use CHECK()
I attempted to add to
http://sites.google.com/a/chromium.org/dev/developers/coding-style but
!...@#@!...@#! sites in its typically infuriating way made it so I
couldn't add text after the last bit I added -- clicking the
unindent button would move my cursor a paragraph up -- so I will
leave it at what I wrote, offer it up to you to improve, and take some
slow deep breaths.
(I seriously wonder if the sites code has some if (evan in the user
name) { go_all_crazy(); }.)
On Wed, May 27, 2009 at 8:00 AM, Erik Kay erik...@google.com wrote:
Perhaps someone could formalize this discussion in the style portion of the
chromium wiki?
Erik
On Tue, May 26, 2009 at 10:18 PM, Darin Fisher da...@chromium.org wrote:
On Tue, May 26, 2009 at 9:13 PM, Peter Kasting pkast...@chromium.org
wrote:
On Tue, May 26, 2009 at 8:31 PM, Brett Wilson bre...@chromium.org
wrote:
Don't bother doing an assertion when the next line will crash anyway:
DCHECK(foo);
foo-DoSomething();
I mostly do what Brett does, but I do sometimes DCHECK in cases like
this, where the DCHECK is placed at the very beginning of a function, and is
really being used to indicate the function's preconditions. This is
effectively an attempt to document in code that the developer of the
function explicitly never wanted the function called with NULL, and if you
fail the check, you should fix the caller rather than bandaiding with a
conditional (which can be a tempting fix if you lack the DCHECK and just see
someone dereference NULL). I think this adds clarity.
Like Brett, I not only use CHECK for potential security problems but also
to indicate cases that will absolutely fail later on and we should just
catch with an easier-to-find crash now instead of a subtle bug later.
PK
+1
It has always been the preferred convention in chrome code to not overuse
CHECK. We do however make great use of it to track down the source of
crashes. DCHECK is good for documentation and for helping developers
understand how a function should not be used.
Historically we used CHECK to protect against malformed IPC. Better to
crash than to exploit, but prior to launch we converted a lot of that code
that lived in the browser over to kill-the-renderer code
instead. So, if you are tempted to CHECK in the browser for security reasons, step back and consider if it might be better to crash the offending renderer that sent you
the IPC that triggered it all.
While it is good to not overuse CHECK, I think it is also reasonable to
use it when you want to know if a bad condition can happen in the field.
We'll know if the CHECK is failing in the field, and that will then guide
us to remove the CHECK and understand why our assertion was false. Of
course, there are other options like UMA logging and histograms that are
probably much better :-)
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Documentation about Linux crash dumping
I wrote up how crash dumping on Linux currently works (with cross-process dumping etc): http://code.google.com/p/chromium/wiki/LinuxCrashDumping 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: reminder: don't use CHECK()
Thanks for trying
I want to amend what you wrote since it doesn't really capture the common
usage of CHECK to help track down crashers, etc.
Also, the statement that we should always recover from a failed DCHECK seems
very wrong to me. I don't think it is a good idea to try to recover from
all manner of bad input to a function, but I do think it is valuable to
DCHECK bad input to a function. The DCHECK helps developers when they are
writing new code, but does nothing to help us know if the failure is reached
in the field.
Trying to recover in the field to a DCHECK failure is often difficult. You
can easily suppress a crash, but the result is often that a feature just
stops working. I'd much rather we get a crash report about such a condition
than have no information reported at all. I think over checking inputs at
runtime to functions can lead to this problem. Sure, you didn't crash, but
you left the product in a gimp state, and no one but the user is aware of
it.
The fear of crashing is overblown. Information about bad states is far more
valuable.
I'd much rather read blog comments from users saying that Chrome crashes too
much than hear about hangs, dead clicks, or other misbehaving non-crash
conditions.
-Darin
On Wed, May 27, 2009 at 11:21 AM, Evan Martin e...@chromium.org wrote:
I attempted to add to
http://sites.google.com/a/chromium.org/dev/developers/coding-style but
!...@#@!...@#! sites in its typically infuriating way made it so I
couldn't add text after the last bit I added -- clicking the
unindent button would move my cursor a paragraph up -- so I will
leave it at what I wrote, offer it up to you to improve, and take some
slow deep breaths.
(I seriously wonder if the sites code has some if (evan in the user
name) { go_all_crazy(); }.)
On Wed, May 27, 2009 at 8:00 AM, Erik Kay erik...@google.com wrote:
Perhaps someone could formalize this discussion in the style portion of
the
chromium wiki?
Erik
On Tue, May 26, 2009 at 10:18 PM, Darin Fisher da...@chromium.org
wrote:
On Tue, May 26, 2009 at 9:13 PM, Peter Kasting pkast...@chromium.org
wrote:
On Tue, May 26, 2009 at 8:31 PM, Brett Wilson bre...@chromium.org
wrote:
Don't bother doing an assertion when the next line will crash anyway:
DCHECK(foo);
foo-DoSomething();
I mostly do what Brett does, but I do sometimes DCHECK in cases like
this, where the DCHECK is placed at the very beginning of a function,
and is
really being used to indicate the function's preconditions. This is
effectively an attempt to document in code that the developer of the
function explicitly never wanted the function called with NULL, and if
you
fail the check, you should fix the caller rather than bandaiding with a
conditional (which can be a tempting fix if you lack the DCHECK and
just see
someone dereference NULL). I think this adds clarity.
Like Brett, I not only use CHECK for potential security problems but
also
to indicate cases that will absolutely fail later on and we should just
catch with an easier-to-find crash now instead of a subtle bug later.
PK
+1
It has always been the preferred convention in chrome code to not
overuse
CHECK. We do however make great use of it to track down the source of
crashes. DCHECK is good for documentation and for helping developers
understand how a function should not be used.
Historically we used CHECK to protect against malformed IPC. Better to
crash than to exploit, but prior to launch we converted a lot of that
code
that lived in the browser over to kill-the-renderer code
instead. So, if you are tempted to CHECK in the browser for security
reasons, step back and consider if it might be better to crash the offending
renderer that sent you
the IPC that triggered it all.
While it is good to not overuse CHECK, I think it is also reasonable to
use it when you want to know if a bad condition can happen in the field.
We'll know if the CHECK is failing in the field, and that will then
guide
us to remove the CHECK and understand why our assertion was false. Of
course, there are other options like UMA logging and histograms that are
probably much better :-)
-Darin
--~--~-~--~~~---~--~~
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: reminder: don't use CHECK()
On Wed, May 27, 2009 at 11:42 AM, Darin Fisher da...@chromium.org wrote:
Also, the statement that we should always recover from a failed DCHECK
seems very wrong to me.
I agree, we should almost never recover from a DCHECK. 90+% of the time,
DCHECK is better than if (condition) { NOTREACHED(); try_to_continue; }
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: reminder: don't use CHECK()
On Tue, May 26, 2009 at 11:31 PM, Brett Wilson bre...@chromium.org wrote: Don't bother doing an assertion when the next line will crash anyway: DCHECK(foo); foo-DoSomething(); will normally crash pretty obviously dereferencing a NULL pointer (even though it will be inside DoSomething). Well, that all depends on what DoSomething does, what happens to be in low memory, whether or not it's also passed an index that's applied to some component of this, etc. If foo *can* be NULL at that point, for whatever reason, not checking it before calling DoSomething is probably a recipe for a security hole. I'd be much happier if DCHECK exited immediately rather than relying on the code to crash, or we insisted that code using DCHECK *always* attempt to recover. --Amanda --~--~-~--~~~---~--~~ 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: reminder: don't use CHECK()
On Wed, May 27, 2009 at 11:56 AM, Amanda Walker ama...@chromium.org wrote: I'd be much happier if DCHECK exited immediately rather than relying on the code to crash, or we insisted that code using DCHECK *always* attempt to recover. Yeah, it seems that relying on the code to crash by itself after a DCHECK is unreliable. It might crash, or it might just do some truly random things (wiping out preferences or URLs or any other persistent data) that the user might not notice until it was too late, not to mention security holes (well, I guess Amanda already mentioned them :). I think that after a DCHECK you should either recover completely (which is probably not possible unless the DCHECK could be replaced by some automatic recovery code anyhow) or crash so we get some info as to why it happened. -Greg. --~--~-~--~~~---~--~~ 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: Avoiding crash after autoupdate on Linux
On Tue, May 26, 2009 at 1:06 PM, Adam Langley a...@chromium.org wrote: On Tue, May 26, 2009 at 12:00 PM, Dan Kegel daniel.r.ke...@gmail.com wrote: http://codereview.chromium.org/115773 is my try at fixing http://crbug.com/11841 (autoupdate broke my browser, familiar to anyone who's used Firefox on Linux). Zygotes ... might have some performance benefits ... I tried measuring those today. On a athlon 64 laptop running ubuntu 9.04 32 bit, I uninstalled tracker (since it chews cpu time) wrote a script to run startup_tests with and without zygote mode, and ran it both after a normal boot and after a boot with mem=256M appended to the kernel commandline. I waited after login until the load average went down to 0.5 before starting. Scripts and data are at http://kegel.com/chromium/bench.tgz Executive summary: zygote mode seems to be slightly faster. Letting the test run overnight might yield more conclusive results, but I'm not sure it's worth it. (Caveat: I don't really know what the startup_tests results represent, I'm just blindly crunching the numbers that came out of it.) Here are some statistics on the startup time results under the four conditions. (n is for normal, z is for zygote.) :: 1.5g/startup-1.nlog.sum :: median meanmax stddev warm: t= 433 606 3647717 cold: t= 16781684185256 warm: t_ref= 314 517 2735539 new_tab: 174417312234333 :: 1.5g/startup-1.zlog.sum :: median meanmax stddev warm: t= 424 593 3550697 cold: t= 16641671183249 warm: t_ref= 313 435 2559503 new_tab: 161116051943266 :: 256mb/startup-1.nlog.sum :: median meanmax stddev warm: t= 790 927 3548617 cold: t= 15371580177797 warm: t_ref= 348 474 2623508 new_tab: 162216632619612 :: 256mb/startup-1.zlog.sum :: median meanmax stddev warm: t= 783 941 3659641 cold: t= 15291565170478 warm: t_ref= 352 476 2571495 new_tab: 996 12172050468 --~--~-~--~~~---~--~~ 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: reminder: don't use CHECK()
It is a nice idea to try to recover from all DCHECKs, but what happens is that you end up with excessive and redundant checking at runtime. It leads to bloat, complexity, and degrades readability (obscuring the code). Those are not good things for a codebase. To be clear, I think we should always treat user input or input from the web with extreme care. I don't mean to imply playing fast-n-loose with that kind of stuff. I think if we require everyone to handle every failed DCHECK, then what we will really do is compel people to write fewer DCHECKs, which means that we will lose some of the documentation benefits. That seems undesirable to me. I think there really is a balance here between adding DCHECKs for the sake of documentation / guidance to developers and dealing with runtime errors that we really need to worry about. People just have to use best judgment to decide when a failed DCHECK should also be handled in release builds. -Darin On Wed, May 27, 2009 at 12:09 PM, Greg Spencer gspen...@google.com wrote: On Wed, May 27, 2009 at 11:56 AM, Amanda Walker ama...@chromium.orgwrote: I'd be much happier if DCHECK exited immediately rather than relying on the code to crash, or we insisted that code using DCHECK *always* attempt to recover. Yeah, it seems that relying on the code to crash by itself after a DCHECK is unreliable. It might crash, or it might just do some truly random things (wiping out preferences or URLs or any other persistent data) that the user might not notice until it was too late, not to mention security holes (well, I guess Amanda already mentioned them :). I think that after a DCHECK you should either recover completely (which is probably not possible unless the DCHECK could be replaced by some automatic recovery code anyhow) or crash so we get some info as to why it happened. -Greg. --~--~-~--~~~---~--~~ 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: reminder: don't use CHECK()
On Wed, May 27, 2009 at 11:56 AM, Amanda Walker ama...@chromium.org wrote: On Tue, May 26, 2009 at 11:31 PM, Brett Wilson bre...@chromium.org wrote: Don't bother doing an assertion when the next line will crash anyway: DCHECK(foo); foo-DoSomething(); will normally crash pretty obviously dereferencing a NULL pointer (even though it will be inside DoSomething). Well, that all depends on what DoSomething does, what happens to be in low memory, whether or not it's also passed an index that's applied to some component of this, etc. If foo can be NULL at that point, for whatever reason, not checking it before calling DoSomething is probably a recipe for a security hole. Like I said, if there are security considerations for your code, then you need to use CHECK and not DCHECK. 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: reminder: don't use CHECK()
On Wed, May 27, 2009 at 3:20 PM, Darin Fisher da...@chromium.org wrote: I think if we require everyone to handle every failed DCHECK, then what we will really do is compel people to write fewer DCHECKs, which means that we will lose some of the documentation benefits. That seems undesirable to me. I agree. Given a choice, I would say that DCHECK should just crash immediately on failure rather than relying on the code that follows it to crash. That is, treat it like an assertion--if it's false, we don't know what state the process is in, and thus don't know how to recover. I think there really is a balance here between adding DCHECKs for the sake of documentation / guidance to developers and dealing with runtime errors that we really need to worry about. People just have to use best judgment to decide when a failed DCHECK should also be handled in release builds. For informative but non-fatal stuff, using the logging facility seems like a better solution. For example, during the Mac Linux porting efforts we've sprinkled log messages all over, which has been very helpful. --Amanda --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Important: User visible UI changes must be discussed with the UI folk
This has happened a couple of times now. If you're adding any visible user interface, even if it's just a checkbox to the advanced page of Options, you must discuss it with: myself (ben@) Glen Murphy (glen@) Nick Baum (nickbaum@) Brian Rakowski (brian@) first. If you don't, it's highly likely that your change will be reverted :-) -Ben --~--~-~--~~~---~--~~ 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://downloads/ behaviour (some suggestions and a bug, i think)
for the last few days (and for sure on 3.0.183.0 (16994)) you stopped showing the DL speed on that tab (unless the size of the download is not known) i actually like the consistent look of FF with the curr/totoal (speed) feel also, as this issue reports, http://code.google.com/p/chromium/issues/detail?id=12610can=4sort=-idcolspec=ID%20Stars%20Pri%20Area%20Type%20Status%20Summary%20Modified%20Owner%20Mstone you report the time left and speed using an algorithm which takes the start download time into account which has some limitations in my eyes namely (mainly ?) i had mad cases where i got my internet connection disconnected and chrome keeps reporting that the download will finish soon it never will but the display is confusing ok, hope to hear from you guys (also, having read the source code that handles downloads, i didn't really get why you first create a temp file only to transfer it at the end to the real name. i am sure you have reasons but i was just curious) --~--~-~--~~~---~--~~ 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: Important: User visible UI changes must be discussed with the UI folk
Also FYI, If you're just porting existing UI to a different platform, no need to discuss with us. Same thing if it's a platform specific difference that we've discussed previously (e.g. we know there's a menu bar on the Mac). The intent with this is to catch new features that don't exist on any platform yet as they creep in. -Ben On Wed, May 27, 2009 at 12:36 PM, Ben Goodger (Google) b...@chromium.org wrote: This has happened a couple of times now. If you're adding any visible user interface, even if it's just a checkbox to the advanced page of Options, you must discuss it with: myself (ben@) Glen Murphy (glen@) Nick Baum (nickbaum@) Brian Rakowski (brian@) first. If you don't, it's highly likely that your change will be reverted :-) -Ben --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] How can i listen for page load complete event
Hi, In chromium how can I listen for page load complete event (i.e the whole page (including iframe/frame) is done loading and CSS is parsed and built /JS are executed). Thank you. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Chromium 1 process per tab architecture
Hi, My understanding is chromium has 1 process per tab architecture. My question is does chromium has the capability/functionality to: 1. if a process/tab runs away (in an infinite loop and taking all cpu) or hangs (block waiting for something for some reason, chromium will kill that process/tab? 2. limit the among of memory that each tab/process can use? Thank you. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Are we now using Visual Studio 2008?
I've been working on a private branch for a while now and recently did a sync. When I try to open chrome.sln in VS 2005, it complains that it can't parse version numbers. Apparently the project files have been upgraded to 9.0? Did I miss the announcement that we were going to VS 2008? --~--~-~--~~~---~--~~ 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: Are we now using Visual Studio 2008?
On Wed, May 27, 2009 at 1:36 PM, Book'em Dano daniel.c...@gmail.com wrote: Did I miss the announcement that we were going to VS 2008? Not that I know of. Perhaps your local copy is modified somehow? 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: Are we now using Visual Studio 2008?
The version of chrome.sln on the trunk (http://src.chromium.org/viewvc/ chrome/trunk/src/chrome/chrome.sln?view=markup) contains snipMicrosoft Visual Studio Solution File, Format Version 9.00/ snip This would seem to indicate VS 2008, no? On May 27, 1:43 pm, Peter Kasting pkast...@google.com wrote: On Wed, May 27, 2009 at 1:36 PM, Book'em Dano daniel.c...@gmail.com wrote: Did I miss the announcement that we were going to VS 2008? Not that I know of. Perhaps your local copy is modified somehow? 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: Are we now using Visual Studio 2008?
On Wed, May 27, 2009 at 1:46 PM, Book'em Dano daniel.c...@gmail.com wrote: The version of chrome.sln on the trunk (http://src.chromium.org/viewvc/ chrome/trunk/src/chrome/chrome.sln?view=markup) contains snipMicrosoft Visual Studio Solution File, Format Version 9.00/ snip This would seem to indicate VS 2008, no? No, that's VS 2005. You should svn diff chrome.sln. 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: Chromium 1 process per tab architecture
See http://dev.chromium.org/developers/design-documents/multi-process-architecture and http://dev.chromium.org/developers/design-documents/process-models http://dev.chromium.org/developers/design-documents/process-modelsThese are on http://dev.chromium.org/ by clicking on For Developers and then Engineering Design Docs. Let us know if the documentation doesn't answer your questions. We want to keep it up to date and useful. Thanks, Mark On Wed, May 27, 2009 at 13:31, Daniel Dreiberg daniel.dreiber...@gmail.comwrote: Hi, My understanding is chromium has 1 process per tab architecture. My question is does chromium has the capability/functionality to: 1. if a process/tab runs away (in an infinite loop and taking all cpu) or hangs (block waiting for something for some reason, chromium will kill that process/tab? 2. limit the among of memory that each tab/process can use? Thank you. --~--~-~--~~~---~--~~ 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: Are we now using Visual Studio 2008?
So sverrir's suggestion is right idea. Currently gyp is detecting and emitting 2005/2008 based on what you have installed. Once chrome.sln is eaten this should allow people to use either, but currently if you have 2008 installed it changes lower level stuff but the checked in sln is still at 2005. A more thought out default might have been prudent, but it will be moot shortly. -BradN On Wed, May 27, 2009 at 1:48 PM, Peter Kasting pkast...@chromium.orgwrote: On Wed, May 27, 2009 at 1:46 PM, Book'em Dano daniel.c...@gmail.comwrote: The version of chrome.sln on the trunk (http://src.chromium.org/viewvc/ chrome/trunk/src/chrome/chrome.sln?view=markuphttp://src.chromium.org/viewvc/%0Achrome/trunk/src/chrome/chrome.sln?view=markup) contains snipMicrosoft Visual Studio Solution File, Format Version 9.00/ snip This would seem to indicate VS 2008, no? No, that's VS 2005. You should svn diff chrome.sln. 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: chrome://downloads/ behaviour (some suggestions and a bug, i think)
From what I understand from your email, you're asking for consensus about the following aspects of download progress, please correct me if I'm wrong or if there are additional issues: * DL speed isn't reported for items of a known size (regression) - [yoav: is there a bug open for this?] * Change format of download time estimate - http://crbug.com/4718 . There are a few suggestions in that bug and from what I understand you're asking if we want to use Firefox's format for the string? * If the network connection is cut while a download is in progress the time estimate gets messed up - http://crbug.com/12610 . In the bug you suggest: calc the DL speed based on the last kSeconds (say 4?) who passed since the last sample I hope other people will chime in on this thread but if not you might want to focus on one issue (4718 for example) and start a new thread asking specifically about that with a few options and asking people to pick the one we want to implement. Best regards, Jeremy On Wed, May 27, 2009 at 12:52 PM, nakro yoav.zilberb...@gmail.com wrote: for the last few days (and for sure on 3.0.183.0 (16994)) you stopped showing the DL speed on that tab (unless the size of the download is not known) i actually like the consistent look of FF with the curr/totoal (speed) feel also, as this issue reports, http://code.google.com/p/chromium/issues/detail?id=12610can=4sort=-idcolspec=ID%20Stars%20Pri%20Area%20Type%20Status%20Summary%20Modified%20Owner%20Mstone you report the time left and speed using an algorithm which takes the start download time into account which has some limitations in my eyes namely (mainly ?) i had mad cases where i got my internet connection disconnected and chrome keeps reporting that the download will finish soon it never will but the display is confusing ok, hope to hear from you guys (also, having read the source code that handles downloads, i didn't really get why you first create a temp file only to transfer it at the end to the real name. i am sure you have reasons but i was just curious) --~--~-~--~~~---~--~~ 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: Important: User visible UI changes must be discussed with the UI folk
So do we send a CL to all of you guys or just one? Assuming all of you touched it. -- Mohamed Mansour On Wed, May 27, 2009 at 4:01 PM, Ben Goodger (Google) b...@chromium.orgwrote: Also FYI, If you're just porting existing UI to a different platform, no need to discuss with us. Same thing if it's a platform specific difference that we've discussed previously (e.g. we know there's a menu bar on the Mac). The intent with this is to catch new features that don't exist on any platform yet as they creep in. -Ben On Wed, May 27, 2009 at 12:36 PM, Ben Goodger (Google) b...@chromium.org wrote: This has happened a couple of times now. If you're adding any visible user interface, even if it's just a checkbox to the advanced page of Options, you must discuss it with: myself (ben@) Glen Murphy (glen@) Nick Baum (nickbaum@) Brian Rakowski (brian@) first. If you don't, it's highly likely that your change will be reverted :-) -Ben --~--~-~--~~~---~--~~ 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: Important: User visible UI changes must be discussed with the UI folk
It would save effort if you discussed the changes you plan to make before writing code. Otherwise just drop us an email and point to the CL (screenshots help, as not all of the team are engineers). On Wed, May 27, 2009 at 2:19 PM, Mohamed Mansourm0.interact...@gmail.com wrote: So do we send a CL to all of you guys or just one? Assuming all of you touched it. -- Mohamed Mansour On Wed, May 27, 2009 at 4:01 PM, Ben Goodger (Google) b...@chromium.org wrote: Also FYI, If you're just porting existing UI to a different platform, no need to discuss with us. Same thing if it's a platform specific difference that we've discussed previously (e.g. we know there's a menu bar on the Mac). The intent with this is to catch new features that don't exist on any platform yet as they creep in. -Ben On Wed, May 27, 2009 at 12:36 PM, Ben Goodger (Google) b...@chromium.org wrote: This has happened a couple of times now. If you're adding any visible user interface, even if it's just a checkbox to the advanced page of Options, you must discuss it with: myself (ben@) Glen Murphy (glen@) Nick Baum (nickbaum@) Brian Rakowski (brian@) first. If you don't, it's highly likely that your change will be reverted :-) -Ben --~--~-~--~~~---~--~~ 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: Important: User visible UI changes must be discussed with the UI folk
I want to stress this point. It's always best to discuss any change in Chrome code with others before you invest a lot of time on it. You will doubtless get helpful feedback and there's a high likelihood you'll save a lot of time. -Ben On Wed, May 27, 2009 at 2:23 PM, Glen Murphy g...@chromium.org wrote: It would save effort if you discussed the changes you plan to make before writing code. Otherwise just drop us an email and point to the CL (screenshots help, as not all of the team are engineers). On Wed, May 27, 2009 at 2:19 PM, Mohamed Mansourm0.interact...@gmail.com wrote: So do we send a CL to all of you guys or just one? Assuming all of you touched it. -- Mohamed Mansour On Wed, May 27, 2009 at 4:01 PM, Ben Goodger (Google) b...@chromium.org wrote: Also FYI, If you're just porting existing UI to a different platform, no need to discuss with us. Same thing if it's a platform specific difference that we've discussed previously (e.g. we know there's a menu bar on the Mac). The intent with this is to catch new features that don't exist on any platform yet as they creep in. -Ben On Wed, May 27, 2009 at 12:36 PM, Ben Goodger (Google) b...@chromium.org wrote: This has happened a couple of times now. If you're adding any visible user interface, even if it's just a checkbox to the advanced page of Options, you must discuss it with: myself (ben@) Glen Murphy (glen@) Nick Baum (nickbaum@) Brian Rakowski (brian@) first. If you don't, it's highly likely that your change will be reverted :-) -Ben --~--~-~--~~~---~--~~ 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://downloads/ behaviour (some suggestions and a bug, i think)
Jeremy, you got my points right however issue 4718, seems to have been solved in my eyes by looking at static std::wstring FormatTimeImpl(const TimeDelta delta, FormatType format_type) and by running 3.0.183.0 (16972) and actually downloading a file but the way you calc time remaining (and DL Speed) is still a bit wrong in my eyes --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Fixing our keyboard handling act on OSX
Hi All, We currently fudge our keyboard handling on OSX, we interpret command key shortcuts ourselves and thus miss out on quite a few Cocoa text handling niceities. We also don't support IMEs. Relevant bugs: http://crbug.com/10862 - OS X: Can't use command-key shortcuts with foreign keyboard layouts http://crbug.com/12698 - standard macintosh text editing keyboard shortcuts are missing http://crbug.com/11981 - Deadkeys do not work http://crbug.com/11952 - IME support (Chinese input method doesnt work) We also don't handle activation/deactivation of edit menu items correctly (select all is always disabled). I've done a little digging in WebKit's keyboard handling and what follows is a proposal on how we can more closely match the OSX keyboard handling code and [hopefully] do things the right way. I don't purport to understand this all and there may be simpler ways to achieve this so feel free to comment. Standard commands (Copy/Paste/Select-all): WebKit handles these through the regular obj-c selectors in WebHTMLView, see the WEBCORE_COMMAND macro. We could do the same thing and add these selectors to BrowserWindowController which would then send an IPC message over to the renderer process to execute the appropriate command. The tricky bit is updating the menus, the model in Cocoa is for the OS to call you for each menu item before displaying a menu. We can't block the browser process on the renderer process so the browser process would always need to know if there is an active selection. WebCore appears to already have a mechanism to do this via notifyAccessibilityForSelectionChange() - we could use the same or a similar mechanism to send an IPC message back to the browser process each time the selection changes. Emacs keyboard commands and IMEs: The IME part of the title may be nonsense, but looking at the code it appears the code path and issues are the same. I've attached the stack trace for hitting cntrl-t to the end of this email. The tricky bit here is that WebCore is first given a shot at handling the event and then passes it back to WebHTMLView to take another look at the event which is where it's actually parsed. I assume this is to give JS code a chance to listen on these events. Since we can't serialize an NSEvent, we can't replicate this code solely in the renderer. A possible solution to this would be to store a queue of the last N NSEvents per renderer matched with an ID. the event would then be serialized and sent to the renderer which could then send it's own IPC message back to the browser process to get Cocoa to handle the message, we could pick the NSEvent out of the queue by ID and send back the relevant edit command to the renderer. Since the events belong to a specific renderer a malicious renderer process can't access events not targeted at them. Best regards, Jeremy Stack trace of hitting cntrl-t (transpose) in a text view, Cocoa [browser-process] stuff marked in bold. #0 WebCore::executeTranspose (frame=0x70b9800) at /Users/Shared/playmobil/work/git.WebKit/WebCore/editing/EditorCommand.cpp:961 #1 0x03702f0f in WebCore::Editor::Command::execute (this=0xbfffe2f4, paramet...@0xbfffe2ac, triggeringEvent=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/editing/EditorCommand.cpp:1450 #2 0x037052ad in WebCore::Editor::Command::execute (this=0xbfffe2f4, triggeringEvent=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/editing/EditorCommand.cpp:1455 *#3 0x0031d3da in -[WebHTMLView(WebNSTextInputSupport) doCommandBySelector:] (self=0x6cefe90, _cmd=0x943edca4, selector=0x94423fa4) at /Users/Shared/playmobil/work/git.WebKit/WebKit/mac/WebView/WebHTMLView.mm:5337 #4 0x0031c238 in -[WebHTMLView(WebInternal) _interceptEditingKeyEvent:shouldSaveCommand:] (self=0x6cefe90, _cmd=0x3a405c, event=0x24178380, shouldSave=0 '\000') at /Users/Shared/playmobil/work/git.WebKit/WebKit/mac/WebView/WebHTMLView.mm:4966 *#5 0x002eb19b in WebEditorClient::handleKeyboardEvent (this=0x6c31aa0, event=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebKit/mac/WebCoreSupport/WebEditorClient.mm:440 #6 0x036f8875 in WebCore::Editor::handleKeyboardEvent (this=0x70b9cd4, event=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/editing/Editor.cpp:105 #7 0x037156fb in WebCore::EventHandler::defaultKeyboardEventHandler (this=0x70b9d00, event=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/page/EventHandler.cpp:1907 #8 0x03a7876e in WebCore::Node::defaultEventHandler (this=0x1c23f430, event=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/dom/Node.cpp:2812 #9 0x037c7d17 in WebCore::HTMLInputElement::defaultEventHandler (this=0x1c23f430, evt=0x24178380) at /Users/Shared/playmobil/work/git.WebKit/WebCore/html/HTMLInputElement.cpp:1120 #10 0x03a70c27 in WebCore::Node::dispatchGenericEvent (this=0x1c23f430, prpeve...@0xbfffe63c) at /Users/Shared/playmobil/work/git.WebKit/WebCore/dom/Node.cpp:2439 #11 0x03a70f0f in
[chromium-dev] Re: Are we now using Visual Studio 2008?
You said snipCurrently gyp is detecting and emitting 2005/2008 based on what you have installed/snip. When you say gyp, I'm assuming you mean that some program is interpreting the *.gyp files (e.g. chrome.gyp) and then using these files to generate the project files? What program is doing this and how do I invoke it? I was only able to find details of the GYP syntax on the wiki, but nothing on how these files are actually converted into vcproj files. Thanks, Daniel On May 27, 1:54 pm, Bradley Nelson bradnel...@google.com wrote: So sverrir's suggestion is right idea. Currently gyp is detecting and emitting 2005/2008 based on what you have installed. Once chrome.sln is eaten this should allow people to use either, but currently if you have 2008 installed it changes lower level stuff but the checked in sln is still at 2005. A more thought out default might have been prudent, but it will be moot shortly. -BradN On Wed, May 27, 2009 at 1:48 PM, Peter Kasting pkast...@chromium.orgwrote: On Wed, May 27, 2009 at 1:46 PM, Book'em Dano daniel.c...@gmail.comwrote: The version of chrome.sln on the trunk (http://src.chromium.org/viewvc/ chrome/trunk/src/chrome/chrome.sln?view=markuphttp://src.chromium.org/viewvc/%0Achrome/trunk/src/chrome/chrome.sln?...) contains snipMicrosoft Visual Studio Solution File, Format Version 9.00/ snip This would seem to indicate VS 2008, no? No, that's VS 2005. You should svn diff chrome.sln. 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: Are we now using Visual Studio 2008?
It gets run automatically on a gclient sync when gyp files have changed. You can run it explicitly with: gclient runhooks --force -BradN On Wed, May 27, 2009 at 5:13 PM, Mohamed Mansour m0.interact...@gmail.comwrote: They use http://code.google.com/p/gyp/ it generates the projects for you. GYP automatically detects 2005/2008 based on what you have installed. If you want to force it to use a specific environment, you can do what sverrir stated, set your environment variable GYP_MSVS_VERSION=2005, which works great. -- Mohamed Mansour On Wed, May 27, 2009 at 8:08 PM, Book'em Dano daniel.c...@gmail.comwrote: You said snipCurrently gyp is detecting and emitting 2005/2008 based on what you have installed/snip. When you say gyp, I'm assuming you mean that some program is interpreting the *.gyp files (e.g. chrome.gyp) and then using these files to generate the project files? What program is doing this and how do I invoke it? I was only able to find details of the GYP syntax on the wiki, but nothing on how these files are actually converted into vcproj files. Thanks, Daniel On May 27, 1:54 pm, Bradley Nelson bradnel...@google.com wrote: So sverrir's suggestion is right idea. Currently gyp is detecting and emitting 2005/2008 based on what you have installed. Once chrome.sln is eaten this should allow people to use either, but currently if you have 2008 installed it changes lower level stuff but the checked in sln is still at 2005. A more thought out default might have been prudent, but it will be moot shortly. -BradN On Wed, May 27, 2009 at 1:48 PM, Peter Kasting pkast...@chromium.org wrote: On Wed, May 27, 2009 at 1:46 PM, Book'em Dano daniel.c...@gmail.com wrote: The version of chrome.sln on the trunk ( http://src.chromium.org/viewvc/ chrome/trunk/src/chrome/chrome.sln?view=markup http://src.chromium.org/viewvc/%0Achrome/trunk/src/chrome/chrome.sln?.. .) contains snipMicrosoft Visual Studio Solution File, Format Version 9.00/ snip This would seem to indicate VS 2008, no? No, that's VS 2005. You should svn diff chrome.sln. 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] Unittesting python
Do we have any unittest support for python in the chromium code base? I did a find ./ -name '*test*.py' and didn't find much. I'm writing a tool to auto-gen some stub code, and didn't see a clear test framework to hook into. -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: Unittesting python
Usually we use the default unittest framework that comes with python http://src.chromium.org/viewvc/chrome/trunk/tools/buildbot/scripts/master/unittests/ http://src.chromium.org/viewvc/chrome/trunk/src/PRESUBMIT_unittest.py?view=markup gclient also has a bunch of tests: http://code.google.com/p/gclient/source/browse/trunk/gclient/gclient_test.py Nicolas On Wed, May 27, 2009 at 6:32 PM, Albert J. Wong (王重傑) ajw...@chromium.orgwrote: Do we have any unittest support for python in the chromium code base? I did a find ./ -name '*test*.py' and didn't find much. I'm writing a tool to auto-gen some stub code, and didn't see a clear test framework to hook into. -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] Where is the integration point betwee chromium and V8
Hi, i am trying to understand how chromium passes JS script node/JS file to v8 engine for execution. So i setup breakpoints in Xcode with test)shell xcode project opened: Compiler::Compile Compiler::CompileEval Compiler::CompileLazy And then I 'build and go (debug)' to get a TestShell. It did start up the TestShell, and it did break in the initial breakpoint I setup in test_shell_main.cc. But when I load a page with Javascript for sure, e.g. www.cnn.con, it never breaks in the Compiler functions that I mentioned above. Can you please tell me how does chromium passes JS script node/JS file to v8 engine for execution --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
