Re: OpenGL and rendering to texture
3D rendering has come up. It is very challenging because or our architecture and the sandbox. The renderer can't have a HWND on the screen, for example, and OpenGL can't share rendering contexts between processes (on or off screen). (I heard the next version of DirectX might, but I'm not sure.) This means that we'd have to copy out of the screenbuffer to cross the process boundary, which would negate any benefit. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Embedded Browser Control Synopsis
On Sep 20, 2:22 pm, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi M-A, On Sat, Sep 20, 2008 at 1:50 PM, Marc-Antoine Ruel [EMAIL PROTECTED]wrote: Ok fine, I can't give you any guidance if your changes would be accepted, sorry. That seems interesting though and I'd be interested in adding printing support to test_shell. If would be great if our printing system could be separated from the main browser process. It would be much easier to write single-process test shell tests for this functionality. All that would be necessary for integration into the browser would be a proxying layer (I think this is how plugins work now). Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Build problems on build 149.30
On Oct 31, 6:53 am, Lucila Sanjurjo [EMAIL PROTECTED] wrote:
Thanks, I think maybe this version fails to compile.
Last week I have synced to the last version of the code. This version seems
to have an issue when pressing TAB key, it throws an access violation.
void WebContentsViewWin::OnPaint(HDC junk_dc) {
if (web_contents_-render_view_host()
!web_contents_-render_view_host()-IsRenderViewLive()) {
---
if (!web_contents_-sad_tab_.get())
web_contents_-sad_tab_.reset(new SadTabView);
CRect cr;
GetClientRect(cr);
web_contents_-sad_tab_-SetBounds(gfx::Rect(cr));
ChromeCanvasPaint canvas(GetHWND(), true);
web_contents_-sad_tab_-ProcessPaint(canvas);
return;
}
Looking at web_contents_ I can see that render_view_factory_ is NULL.
This is a known issue?
Sync again. I broke this last week, it was broken for about half a
day, so you just got an unlucky build.
Brett
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
Chromium-dev group.
To post to this group, send email to chromium-dev@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/chromium-dev?hl=en
-~--~~~~--~~--~--~---
[chromium-dev] Moving PlatformCanvas to webkit/
I've made changes to remove almost all of the base dependencies in the graphics layer. With a few exceptions, the only parts left are the uses of platform_canvas*, platform_device*, native_theme, and image_operations. There are tricky because they are extensively used in both webkit/port and chrome. The solution I'm leaning toward now is to make a new directory like webkit/port/platform/graphics/skia/public (or something, any better naming suggestions?) and move these files to there. This directory would have the requirement that it depend on nothing outside of Skia, and we allow files inside the browser to include from here (normally you're not allowed to include things in the browser from webkit/ other than glue). We include the above-mentioned very seldom in Chrome since we use PlatformCanvas' derived class ChromeCanvas in almost all places, so this would not have a big impact on browser code. ChromeCanvas would stay the same, but it's base class would get moved. Native_theme is a little more tricky since there is some desire to eventually unify theme drawing with the rest of WebKit. If we did that, it would probably be undesirable from WebKit's perspective to have this file in our directory with our funny rules. We can choose to duplicate the code in this case (I think only a fraction of it is actually needed by the app layer, since it uses actual native controls for most thing), so this wouldn't be as bad as it may sound. There are other options: (1) Move the files in question to Skia. I don't like this too much because image_operations seems out-of-place, and native_theme seems really out of place. If we pull Skia from an external repository someday, we wouldn't be able to have that stuff live in the Skia repository. (2) Duplicate the files in both places. (3) Move the files to port but include headers in base/gfx that include those files, so chrome code doesn't have to directly include files from webkit, which looks a little odd. Comments or suggestions? Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Quick and dirty chrome embedding test
On Tue, Nov 18, 2008 at 6:50 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi All, Is it safe to create (via Browser::Create*()) and/or access Browser object instances from multiple threads? In other words, I need to choose one of the following options: No, the entire UI of Chrome is single-threaded and is not threadsafe. If you need things to access it from multiple threads, you will need to synchronize everything. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Quick and dirty chrome embedding test
On Tue, Nov 18, 2008 at 7:14 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi Brett, On Tue, Nov 18, 2008 at 9:56 AM, Brett Wilson [EMAIL PROTECTED] wrote: On Tue, Nov 18, 2008 at 6:50 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi All, Is it safe to create (via Browser::Create*()) and/or access Browser object instances from multiple threads? In other words, I need to choose one of the following options: No, the entire UI of Chrome is single-threaded and is not threadsafe. If you need things to access it from multiple threads, you will need to synchronize everything. Is there a thread-safe method equivalent to the PostMessage() WinAPI call for adding events to the browser MessageLoop? PostMessage should already be thread safe. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Moving PlatformCanvas to webkit/
I just broke the build again with more NativeImagePtr problems. Right now, this is making me feel much less like tolerating significant differences between our Mac Windows ports. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Moving PlatformCanvas to webkit/
On Tue, Nov 18, 2008 at 10:44 AM, Amanda Walker [EMAIL PROTECTED] wrote: NativeImagePtr is a WebKit type, and is designed to be the platform's native representation. We're all still fighting hidden assumptions about what PLATFORM(XXX) flags mean. Skia and CG are graphics APIs, but webkit (and chromium currently) presume that they are 1:1 with platforms, which they aren't (so, as a result, NativeImagePtr gets typedefed to CGImageRef instead of SkBitmap* by an accident of #ifdef ordering). In the short term, I have a patch in flight that should fix the NativeImagePtr compatibility, but fonts will no doubt present the same problem, and will be much harder to solve (a no significant differences between platforms approach will require a major overhaul on all 3 platforms (well, maybe not Linux, since they don't have much rendering code yet) to use something like Pango + FreeType + Cairo for rendering instead of GDI, CG, or Gtk). No matter what we do each platform should have its own font and form control rendering. I don't think unifying these has ever been on the table. When I talk about unifying I mean the overall architecture. Each platform will have some of its own code to render fonts and form controls into the underlying buffer. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Moving PlatformCanvas to webkit/
On Tue, Nov 18, 2008 at 11:02 AM, Amanda Walker [EMAIL PROTECTED] wrote: On Tue, Nov 18, 2008 at 1:54 PM, Brett Wilson [EMAIL PROTECTED] wrote: No matter what we do each platform should have its own font and form control rendering. I don't think unifying these has ever been on the table. When I talk about unifying I mean the overall architecture. Each platform will have some of its own code to render fonts and form controls into the underlying buffer. Hmm, OK. Why are images different, then? I'm missing something--the architecture's already unified--NativeImagePtr is just a WebKit platform-specific type that's typedefed inconveniently at the moment. This is not platform-specific on any other architecture, and it doesn't need to be. The difference is that font and form control rendering have to be native for us to look correct. This is not the case for images or any other part of GraphicsContext. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Quick and dirty chrome embedding test
On Tue, Nov 18, 2008 at 11:19 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi Brett, On Tue, Nov 18, 2008 at 11:12 AM, Brett Wilson [EMAIL PROTECTED] wrote: On Tue, Nov 18, 2008 at 7:14 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi Brett, On Tue, Nov 18, 2008 at 9:56 AM, Brett Wilson [EMAIL PROTECTED] wrote: On Tue, Nov 18, 2008 at 6:50 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi All, Is it safe to create (via Browser::Create*()) and/or access Browser object instances from multiple threads? In other words, I need to choose one of the following options: No, the entire UI of Chrome is single-threaded and is not threadsafe. If you need things to access it from multiple threads, you will need to synchronize everything. Is there a thread-safe method equivalent to the PostMessage() WinAPI call for adding events to the browser MessageLoop? PostMessage should already be thread safe. Thanks for your input, I think I understand now. Both the Windows message loop and the chrome task queue co-exist in the MessageLoop class via Delegate and Dispatcher implementations. Chrome uses the Windows message loop as a means for managing the task queue (via WM_TIMER calls), so there should be no need for me to post Windows messages directly to the UI thread. Instead, to call a Browser object method (like GoBack()) from a separate thread I should execute MessageLoopForUI::current()-PostTask() (which is itself thread-safe) to have the UI thread execute a method that I provide, which would in turn make the Browser method call. Kind of. MessageLoopForUI doesn't give ui the UI message loop. It gives you the current message loop assuming that it's a UI type of loop. You'll have to track independently which message loop the UI message loop is. We usually pass this information into the objects that live on the secondary threads when they start. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Quick and dirty chrome embedding test
On Tue, Nov 18, 2008 at 11:59 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Hi Brett, On Tue, Nov 18, 2008 at 2:27 PM, Brett Wilson [EMAIL PROTECTED] wrote: On Tue, Nov 18, 2008 at 11:19 AM, Marshall Greenblatt [EMAIL PROTECTED] wrote: Thanks for your input, I think I understand now. Both the Windows message loop and the chrome task queue co-exist in the MessageLoop class via Delegate and Dispatcher implementations. Chrome uses the Windows message loop as a means for managing the task queue (via WM_TIMER calls), so there should be no need for me to post Windows messages directly to the UI thread. Instead, to call a Browser object method (like GoBack()) from a separate thread I should execute MessageLoopForUI::current()-PostTask() (which is itself thread-safe) to have the UI thread execute a method that I provide, which would in turn make the Browser method call. Kind of. MessageLoopForUI doesn't give ui the UI message loop. It gives you the current message loop assuming that it's a UI type of loop. You'll have to track independently which message loop the UI message loop is. We usually pass this information into the objects that live on the secondary threads when they start. Am I correct in thinking that there's only one UI message loop per process (created in BrowserMain())? If so, I'd be curious to know why a pointer to the UI message loop isn't available from the global BrowserProcess instance. The BrowserProcess object is not threadsafe and can only be used on the UI thread. The UI thread could be stored, but it's worked fine to pass it when we need it. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Public interest email
On Tue, Dec 2, 2008 at 6:07 PM, Ben Goodger (Google) [EMAIL PROTECTED] wrote: On a slightly related note, the Powerpuff Girls squeakbat has a leak and has deflated. Does anyone @Google have a bicycle repair kit? If so let me know and we will figure out how to repair it. I swear I don't know how it got that way. I am in no way harboring resentment over the excessive number of times it has had to be used on me. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Extending Value with std::string support
On Mon, Dec 8, 2008 at 7:50 PM, Peter Kasting [EMAIL PROTECTED] wrote: On Mon, Dec 8, 2008 at 6:41 PM, Andrew Scherkus [EMAIL PROTECTED] wrote: Darin touched upon this, who said to document that std::string should refer to UTF-8 strings. How about: - CreateStringValue creates a StringValue object that returns TYPE_UTF8_STRING and has a DCHECK(IsStringUTF8(foo)) in the constructor - CreateWideStringValue creates a WideStringValue object that returns TYPE_UTF16_STRING To be honest, I probably lean more toward a single overloaded CreateStringValue(). I think having different function names decreases clarity and increases verbosity. But it's not a big deal. However, if you go with two names, make the names match the TYPE_ returns: CreateUTF8StringValue() and CreateUTF16StringValue(), or something. Be careful because wstring != UTF16String. In other places of the code, we use GetWString, which if you're returning a wstring, I think is the best naming convention (since wstring changes type depending on the platform). Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Extending Value with std::string support
On Mon, Dec 8, 2008 at 8:32 PM, Andrew Scherkus [EMAIL PROTECTED] wrote: Somewhat in line with the Google style guide, the overloaded CreateStringValue/GetString do accomplish the same thing (variant string type), just with different encodings. I did some partial implementations of #3 and as Peter highlighted, writing GetWideString everywhere started looking really silly. In terms of enums and implementation, would TYPE_STRING and TYPE_WSTRING suffice with documentation I'm happy with that. and DCHECKs for UTF-8 std::strings? What is the source of the data? We normally don't bother verifying the encoding because that be very clumsy. But depending on the source, this may be helpful. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Bookmark Added! GUI redesign ideas
Hi Simon, Thanks for the thoughtful mocks. I like the overall feel of the more horizontal versions better for some reason. However, I also like having the Remove link in the upper right. I think of it somewhat like a close box for the bookmark, and I expect its placement to be in the same place. I also kind of expect the close box to be in the lower right, since that's usually where the OK button is in a dialog box. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Skia merge landing today
I'm planning on landing the Skia merge today. I have the trybots happy, but there will likely be fallout because we haven't gotten a new Skia drop for about 6 months. The main problem is that the antialiasing algorithm changed slightly (for the better, it seems) so image diffs need to be updated. I have about 1200 rebaselined layout test images for Windows, but we'll need the same thing for Linux. I will coordinate with a Linux person who will be ready to rebaseline tests as soon as I check in. There may be other problems as well. One of the reasons for the merge is that it appears to fix some of the UMR failures we see in purify as well as at least one crash. So hopefully, it will fix more problems than it creates. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to chromium-dev+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Inline spell checking.
On Mon, Dec 15, 2008 at 12:14 PM, Aaron Boodman a...@chromium.org wrote: It would be better if we could consult Google for spelling recommendations. It seems like I frequently misspell a word, get the red squiggly, get no recommendations from Chromium, then go to Google and it gets it exactly right. I know, this would be great for an extension. There is a bug in the current spellchecker where our suggestions don't work as well as they should. This was fixed a week or two ago on trunk. Brett --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups Chromium-dev group. To post to this group, send email to chromium-dev@googlegroups.com To unsubscribe from this group, send email to chromium-dev+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/chromium-dev?hl=en -~--~~~~--~~--~--~---
[chromium-dev] Re: Default line ending CR LF, CR, or LF???
On Mon, Dec 29, 2008 at 8:01 PM, Daniel dpc...@hotmail.com wrote: What should be used for the default line endings? DOS (CR LF), MAC (CR) or UNIX (LF) style? I notice that most files seem to use UNIX (LF), but I want to make sure that this is the expected line endings format. You're right, we should generally have Unix line endings. 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] V8Bindings_prebuild slowness
I have been doing some build profiling over the weekend. When I run IncrediBuild, computing dependencies takes about 30 seconds. Then one CPU starts performing custom build step which is V8Bindings_prebuild. In parallel, most dependencies like ICU compile in the next minute. Then it hangs for 3 minutes waiting for V8Bindings_prebuild. Then it compiles all of WebCore and some remaining parts of Chrome in the next 4 minutes, and links in 2 minutes. Why is 1/3 of my time wasted waiting for a makefile to run? During this time I see a continuous stream of sh.exe, perl.exe, and cc1plus.exe getting spawned and killed. Looking at the makefile, this happens 300 times, and each time perl has to search a bunch of different directories of WebCore sources to find the file in question. If I already have this step completed, IncrediBuild can compile all of chrome_exe from scratch in 5:50 on my computer, which is really fast! With a clean debug directory, it takes 8:57. It seems like this is a really big waste of everybody's time. I don't entirely know what's going on, and I barely know Perl, but it seems like there are two options: 1) Improve the step to list the exact locations of each file, and to only run perl once, iterating over the list in perl rather than in the makefile. This loses the incremental aspect if you do change anything, but most people aren't changing the IDL files. I bet this would make it orders of magnitude faster. 2) Check in prebuilt V8 bindings. This means that the WebKit merger would need to run an extra step, but everything would magically work for everbody else. I really like this solution because I think the V8 bindings are the most common thing that causes weird build errors when you sync after a new WebKit has been pulled, and I think it's also the main reason that you have to physically delete the output directory instead of doing a regular clean. It could be that having the correct versions checked in saves everybody a lot of rebuilding in addition to saving several minutes on each build. I really like option #2. Does anybody have any thoughts on how well this would work? 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: V8Bindings_prebuild slowness
On Sun, Jan 4, 2009 at 11:28 AM, Mike Belshe mbel...@google.com wrote: It's been this way for a while; the good news is that it should only happen on initial build; subsequent builds don't regenerate the bindings. The problem is that I (and it seems like many other people) have learned that you have to do a full build basically every time you sync, because the dependencies aren't computed properly. Ironically the biggest problem of incorrect dependencies is the generated files! That's why I was suggesting the solution that will help both of these problems. If 3 minutes is really too much for a full build, we could do your #2; hopefully the dependency checker would be smart enough not to rebuild them if they were checked in. We'd have to find where to check them in as well, because currently they are built into the output directory (for both Release and Debug; perhaps we could just generate one set, as I don't think they differ between release/debug builds) I think we would need a new DEPS entry for them. Ojan's WebKit sync script could probably do the management of this automatically. 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: webkit/port is moving into third_party/WebKit/WebCore
On Mon, Jan 5, 2009 at 8:30 AM, Mike Pinkerton pinker...@chromium.org wrote: Awesome! Now that so much of our code is in the webkit tree, is there a (public) wiki page describing the steps necessary to make changes to anything within third_party/WebKit/WebCore? i.e, does everything have to go upstream, are there any circumstances when forking is allowed on a vendor branch, when should DEPS be rolled in this new world, etc? Seems like things have changed so quickly over the last few months (in a good way), it might be helpful to lay out the current state of things for everyone to re-baseline their knowledge. Currently our port is not upstream, it is just checked into our local copy of WebKit where they will soon be checked in upstream. So you can continue to change our port when you want, although you will have to roll webkit deps by updating src/DEPS to pull your change into Chrome. Once we check in upstream, all changes should be checked in upstream, and they will be pulled in the next day's WebKit merge. 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: Proposal: enforcing unit testing in gcl
I think this sounds like an excellent start. We can tweak it if we
notice things not working properly.
Brett
On Mon, Jan 5, 2009 at 3:25 PM, Pam Greene p...@chromium.org wrote:
We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
Any new source file (.cc, .cpp, .m, or .mm), or
Any new method added to a source or header (.h file
A new method is identified by a flush-left non-comment line that has (
somewhere before the next flush-left line and either ends with { or has { at
the start of the next flush-left line.
What counts as a test?
Any change to any code file whose name ends in test.* or tests.*
This is very rough, but at least it shows that the contributor thought about
testing when making the patch
What do we do if we don't find a test?
On 'gcl change', report a warning to the user
On 'gcl upload', add a warning to the change description so the reviewer
sees it too
Add an option to override this
Future possibilities
Is it worth restricting the check to only public or protected methods?
Since any real change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
Rather than adding a warning to the change description, it'd be nice to have
a separate warning in the review app, so it showed up no matter what and the
path author didn't have to override anything. But since we want the warning
in client-side 'gcl change' anyway, for now we'll keep it simple and trust
people.
Comments and volunteers welcome.
- Pam
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Has your computer melted?
I just checked in a change to use /MP for all compiles, which is a secret undocumented flag that does parallel compiles within each project. Please let me know of your computer melts or becomes unusable during a compile. It should more efficiently use all of your CPUs when doing regular Visual Studio builds (it will have no effect on IncrediBuilds). You can remove it from essential.vsprops if it's causing problems. We tested on Carlos' 2-processor system and it pegged the CPU more, although it's not clear if it's a lot faster than before. On my 4-processor system, a build of just chrome_exe and all of it's dependencies went from 25 to 16 minutes after using this flag. So if you hate IncrediBuild, life might actually be tolerable without it for fast systems. 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: Has your computer melted?
On Jan 5, 4:32 pm, Brett Wilson bre...@chromium.org wrote: I just checked in a change to use /MP for all compiles, which is a secret undocumented flag that does parallel compiles within each project. Please let me know of your computer melts or becomes unusable during a compile. It should more efficiently use all of your CPUs when doing regular Visual Studio builds (it will have no effect on IncrediBuilds). You can remove it from essential.vsprops if it's causing problems. We tested on Carlos' 2-processor system and it pegged the CPU more, although it's not clear if it's a lot faster than before. On my 4-processor system, a build of just chrome_exe and all of it's dependencies went from 25 to 16 minutes after using this flag. So if you hate IncrediBuild, life might actually be tolerable without it for fast systems. This was backed out earlier today due to weird errors, especially with regard to linking and symbols. Some people have also reported that Visual Studio can think everything is out of date and rebuilds when it's not necessary. If you would like to keep trying this change out, do this locally: http://chrome-svn/viewvc/chrome/trunk/src/build/internal/essential.vsprops?r1=7533r2=7573 and restart Visual Studio. 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: Windows SDK Configuration Tool failure workaround
Thanks Yury, I added this to the build instructions page. Brett On Thu, Jan 8, 2009 at 8:33 AM, Yury Semikhatsky yury.semikhat...@gmail.com wrote: If Windows SDK Configuration Tool fails with message Windows SDK Version Selection Tool has encountered a problem and needs to close. We are sorry for the inconvenience. try running it from command line with undocumented flag -legacy: windowssdkver -version:v6.1 -legacy I wasted quite a lot of time trying to figure out why it would fail when I started it from the menu and ran OK but didn't change anything when I launched it from command line. I found answer at: http://social.msdn.microsoft.com/forums/en-US/windowssdk/thread/2514049f-a9f7-4e16-85f6-c7970658271e/ Thanks, Yury On Jan 8, 4:38 am, idanan chr...@cybernium.net wrote: To be clear, developers must install the Platform SDK 6.1 which is part of Windows SDK for Windows Server 2008 and .NET Framework 3.5 and is available at the following link (repeated from above):http://www.microsoft.com/downloads/details.aspx?FamilyID=e6e1c3df-a74... This installer contains lots of stuff including VC++ 9.0 (which we don't use), so do not install it unless you know how to keep VC++ 8.0 (which we use) and make sure that it stays as the compiler for Chromium builds. When the installer completes you must use the Windows SDK Configuration Tool in the Start menu, under Microsoft Platform SDK, to enable this as the current SDK. Otherwise Chromium builds won't pick it up. After it is installed and configured you must clobber your build which means to recursively delete (rd /s /q) the src\Chrome\Debug and src\Chrome\Release directories under your SVN client. A gclient sync is also required. You can do the clobbering and SDK install in any order but the build must be done as the LAST step. - Itai --~--~-~--~~~---~--~~ 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: how to hide navigate bar
On Mon, Jan 12, 2009 at 9:41 PM, if-ifone hello...@gmail.com wrote:
it is known that popup browser with maxsize and no navigate bar can use
script like below
window.open('test.html','blank','fullscreen=1,toolbar=no,menubar=no,scrollbars=no,resizable=yes,location=no,status=no');
how ever chrome always show white location bar like attach shows.
how to hide it.
For security reasons, it is not possible to hide the URL bar (much
like the latest versions of Firefox and IE, if I recall).
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: fixing CommandLine
On Wed, Jan 14, 2009 at 6:22 PM, Evan Martin e...@chromium.org wrote: Our CommandLine class is very confusing -- it is not a class for working with command lines, but in fact a stealth singleton that wraps the command line used to start the process. Further, since it came from Windows, it does all this string-munging and quoting that is not necessary on OS X or Linux. We need a sane way to construct cross-platform command lines and invoke subprocesses. I propose the following: 1) For the singleton use case, we change code to use a real singleton (e.g. CommandLine::Get() or even our SingletonCommandLine). I prefer the ::Get version myself. I was also thinking of CommandLine::ForCurrentProcess() since it might reduce some of the ambiguity that seems to crop up here. 2) We extend the class to also be useful for generating command lines. Here's a taste of API (that would be folded into CommandLine): http://codereview.chromium.org/18073/diff/1/3 The function names intentionally match the old static function names above so it's easier to convert old code. Some callers are already incorrectly (by the current API) using CommandLine like this. If this is ok, I volunteer to fix all callers. (If you haven't dealt with it before, this area of the code is embarassingly prone to endless arguments, so I apologize for bringing this up again.) I'm on the fence about this builder class since it somehow feels like overkill for this application. If we do go with the class, do we really need a vector of strings? It seems like every use (though I haven't looked at all uses) is basically. have a string append some switches use the string so that keeping the vector of strings is just a lot of mallocs and string copies with no purpose. Now that I write this, I think having the class is fine since it provides some useful scoping for these things, but I think it should just keep a string and append to it like we do currently. 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] 2-day merges and the cleanup schedule
I'm currently doing a 2-day merge rotation.As part of this, various layout tests are regressing or getting added that I'm inserting into the tests_fixable list. But, like every other layout test fixer, after my merges are done, I'll finally go back to my old work and not think about it any more. This is how we get a monotonically increasing list of broken tests at the end of the tests fixable. I'm pretty certain this happens faster than the tests are getting fixed because nobody wants to fix them. I'm sort of tempted to just fix the ones my merge broke now, but that will put me behind for my next merge, so I don't do that. I propose a different rotation schedule for WebKit merges. You would do one day of merges, and the next day you would have to clean up all the regressed and new layout tests your merge introduced. The layout tests usually aren't that hard, so it normally wouldn't take the whole day. This way we can be in a good steady state of WebKit merges. It should also have a big advantage for fixing the broken tests since the things that changed, the build numbers involved, etc. are still fresh in the merger's head, and it won't be like approaching a random layout test from the list with no context. The disadvantage of doing this is that we have to find people to do the merges faster (we should probably formalize the schedule), and you'll lose some advantage the second day of having all the instructions and gotchas of the merge fresh in your mind. I was thinking of a 3 day schedule (2 days of merging, 1 day of fixing) but that seems too heavyweight for the people volunteering to do this. Anybody have any comments? 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: 2-day merges and the cleanup schedule
On Thu, Jan 15, 2009 at 1:20 PM, Pam Greene p...@chromium.org wrote: When fixing layout tests only means re-baselining, that's easy. But sometimes they break (or new ones fail) for deeper reasons, and the person doing the merge may not be the right one to make the fix (or may not be able to fix them in one day). So perhaps clean up in this context means re-baseline if that's all it needs, and file individual bugs on specific people for bigger brokenness. I think our tendency to file bugs and forget about it is part of the problem. I am at least as guilty as anybody else. I think the merger should have the responsibility to get their regressions fixed. They will have to talk with some other people to get input. If they aren't the right person to fix a problem, it should be their responsibility as part of the cleanup to make sure that the right person has been assigned to it and is actually working on it. When people are assigned merge bugs, they should be treated as important regressions and prioritized over other work. We currently had a whole lot of layout tests bugs filed that are getting no love. The only way to not keep getting behind is to be much more proactive. Also, to clarify, are you proposing that we only merge every other day, or that we have two people assigned each day: one to merge and one to clean up the previous day's layout-test breakage? If the latter, we could also split the job in the other direction, and have one person merging two days in a row and one fixing up the test list both days. I could imagine people's tastes running more to one job or the other, and we don't really care who does what as long as it gets done. I'm proposing overlapping so we merge every day. I think there is an advantage in having the same person who did the merge do the fixing. This hopefully also makes the merge less tedious since you have different tasks your two days. 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: Making sense of startup
On Thu, Jan 15, 2009 at 1:19 PM, Amanda Walker ama...@chromium.org wrote: That's true. In the example I gave (Mac driver loading), each module has a property list that lists its dependencies (and version requirements, etc.). That's not quite as simple to do inside a single application, of course, but having code do the ordering still seems like a win to me. Consider startup as a sequence of, say: Initializer foo(foo); foo.DependsOn(bar); foo.DependsOn(zot); Initializer alice(alice); alice.DependsOn(bob); alice.DependsOn(eve); [...] Initializer::LaunchAll(); (or, more generally, Initializer:::Launch(chromium); where chromium is a top level module, so that the general framework could be used for things besides app launch) I have a hard time thinking about this with your abstract example. I also have a hard time believing we have so many interdependencies that it requires writing a makefile to start the program, and that such a syntax is easier to read than just listing out in comments why things are done in this order. 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: Layout test and font stroking issue - help wanted
On Fri, Jan 16, 2009 at 9:32 AM, Sverrir Á. Berg sver...@google.com wrote: I'm running these tests: http://trac.webkit.org/export/39759/trunk/LayoutTests/fast/text/stroking-decorations.html http://trac.webkit.org/export/39759/trunk/LayoutTests/fast/text/stroking.html It seems that our font rendering path uses UniScribe/ScriptTextOut because of the accent (this means that the line containing the accented character is rendered incorrectly). For other lines on the page we use paintSkiaText and that works fine (not using GDI because of the stroking). These tests run fine on webkit nightly because they end up using CGContextShowGlyphsWithAdvances which as far as I know not open source. Potential solutions: * Ignore. * Dynamically map e#x0300; into #x00e8; so we can use the skia rendering (along with other similar characters that are actually in use). * Implement complex rendering features on top of Uniscribe. Number two seems simple enough, number three scares me. Number 3 is the right answer. You have to implement ScriptTextOut: http://msdn.microsoft.com/en-us/library/ms776504(VS.85).aspx for Skia in the same way that skiaDrawText implements Windows' DrawText funciton. I don't think this function is that scary, but I'm not totally sure about the details. You're given all of the glyphs and positions, so hopefully it's not that hard. There might be some font fallback and edge cases that won't work or will require extra code, but the initial implementation should be reasonably straightforward. 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] Responsibility reviewing patches with multiple reviewers
There are a lot of patches lately with a lot of reviewers on them, especially related to porting since a lot of people might need to be in the loop for some changes. The problem is that there's no clear responsibility given in these reviews. If I'm the sole reviewer on a change, I know I have to do a good job. When there are three other people, I sometimes assume that somebody else must have looked carefully at some part of the review. I'm sure sometimes all the reviewers think this and the change isn't reviewed properly. In other cases, some reviewers say LGTM for a patch, while others are still expecting changes. The author can get confused as to the status of the review, and I also know some patches have been checked in lately where at least one reviewer expected further changes before checkin. At the same time, we want to encourage many people to participate in the review process and keep tabs on what's going on. I propose several changes to best practices: 1. When a patch author requests more than one reviewer, they should make clear in the review request email what they expect the responsibility of each reviewer to be. Example: agl: bitmap changes evanm: process hacks everybody else: FYI In this case, I might be on the review list because I've asked to be in the loop for multiprocess changes, but I wouldn't be the primary reviewer and the author and other reviewers wouldn't be expecting me to review all the diffs in detail. 2. If you get a review that includes many other people, and the author didn't do (1), please ask them what part you're responsible for if you don't want to review the whole thing in detail. 3. The author should wait for approval from everybody on the reviewer list before checking in. 4. People who are on a review without clear review responsibility should be super responsive and not hold up the review. The patch author should feel free to ping them mercilessly if they are. 5. If you're on FYI person on a review and you didn't actually review in detail (or at all), but don't have a problem with the patch, note this. You could say something like rubber stamp or ACK instead of LGTM. This way the real reviewers know not to trust that you did their work for them, but the author of the patch knows they don't have to wait for further feedback from you. Hopefully we can still keep everybody in the loop but have clear ownership and detailed reviews. It might even speed up some patches since I can quickly ACK patches I don't care about, and the patch author knows they don't have to wait for feedback from me. Or do you think this has too much overhead? Comments? 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: build problem (chromium.r9040.tar.gz)
On Tue, Feb 3, 2009 at 9:05 PM, t...@chromium.org wrote: What evan means is that after downloading the tar ball, you need to run gclient sync to get all the platform specific dependencies. We recently started generating the source tar ball on a regular basis and it doesn't include all the Windows and Mac dependencies from the src/third_party directory. Running gclient sync will download these platform specific dependencies for you. In that case, the build instructions are out of date. I updated the getting-the-code page to reflect that this is now required. 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: hash_maps, hash functions and gcc
2009/2/4 Paweł Hajdan Jr. phajdan...@chromium.org: I recently tried to port chrome/browser/safe_browsing/protocol_manager.cc to Linux, but there's one problem. ProtocolManager uses hash_map of URLFetcher*-s, and GCC needs a hash function for it. I see many ways we can deal with that, and I discussed some of them on irc and in review. The result so far is that it would be better to discuss this further on chromium-dev. So, what we could do: 1) Just implement needed hash function for gcc in URLFetcher's header file. Works, but then it has to be done in every place where it's needed, and we have at least one case of that, for example profile.h. 2) Provide default identity hash function for object pointers (it's probably what MSVC does on Windows). You can see a patch here: http://codereview.chromium.org/20041 . There are concerns about making this a default behavior, so... 3) Maybe create a macro like GCC_IDENTITY_HASH(ClassName) so we don't have to repeat the implementation of identity hash function. Then it's simple to apply it explicitly only where it's needed. But well, it's a macro. Generally each of these choices have some drawbacks, but probably they're not the only ways to deal with this problem. So, if you have any ideas, comments, etc. - please share. I am not familiar with this API, but in general, I think we should just use std::map instead. Maybe a safe browsing expert can comment about this. If we have less than a few hundred or so it doesn't matter. 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: [linux] stracing chrome binaries
On Wed, Feb 4, 2009 at 2:58 PM, Adam Langley a...@chromium.org wrote:
If you've ever tried running strace on our binaries, you've probably
noticed the Ubuntu version messes up a lot because it cannot decode
the 32-bit structures correctly. For example:
[pid 21205] ... recvmsg resumed {msg_name(-11664)=NULL,
msg_iov(18446693667973365760)=0x8254b28,
msg_controllen=6158080064115183616, msg_control=0x8253b2808253b3c,
msg_flags=MSG_DONTROUTE|MSG_PROXY}, MSG_DONTWAIT) = 0
In order to fix this, you can build a 32-bit version of strace for
tracing chrome binaries.
1) Grab the latest source from
http://sourceforge.net/project/platformdownload.php?group_id=2861
2) Unpack it somewhere
3) % export CC=gcc -m32
4) ./configure --host=i686-unknown-linux-gnu
5) make -j4
6) copy the resulting ./strace binary as ~/bin/strace-chrome etc
Can you add this to the build page or some Linux environment setup page?
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: How many GDI handles are reasonable?
On Thu, Feb 5, 2009 at 9:29 PM, Ian Fette i...@chromium.org wrote: Once again, my laptop is refusing to stand by and Windows is throwing up warning dialogs about Insufficient system resources exist to complete the API. Currently Chrome is sucking down a healthy 1043 gdi handles according to gdiview, 647 of which appear to be font handles... What is a reasonable number to expect Chrome to be using? Is 647 font handles indicative of a leak? For reference, I have 23 chrome processes running, and a healthy (or unhealthy?) number of tabs. Is 647 font handles across all chrome processes? If that's across all 23 processes, it's not a problem. 1000 GDI objects is a lot and more than normal, but it depends on what sites you've been visiting because of the font cache. For reference, I've got 200 GDI objects per chrome process, and I have two visual studio instances open and each is using 750 GDI objects. 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: Is there a scroll bar?
On Tue, Feb 17, 2009 at 3:43 PM, Marc-Andre Decoste m...@google.com wrote: Salut, I'm looking for a way to know, at the BrowserView (or even at the RenderWidgetHostViewWin) level, if there is one, and only one scroll bar visible. I thought I could ask Windows with ::GetScrollBarInfo, but it doesn't work since the scroll bars are handled by WebKit and not by Windows. Scroll bars a non-native and rendered by WebKit. The browser doesn't know anything about it. I think you will have to do this at the WebKit level. 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: checked in file third_party/libxml/win32/include/xmlversion.h [Attn: Daniel Veillard]
On Thu, Feb 26, 2009 at 7:59 PM, eager_learner vijay.sankar.ra...@gmail.com wrote: Hello I am trying to integrate test a third party library available in source into the Visual Studio 2008 Pro environment. The problem seems to be that the third_party/libxml/win32/include/xmlversion.h file defines LIBXML_ICU_ENABLED. This causes encoding.h to include unicode/ucnv.h which seems unavailable on Windows. So either I #ifdef around the third party library source but that would be a bad hack. We use ICU. It's in third_party/icu38. You can use the build/using_icu.vsprops to set VS to search in these directories for includes. 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: Unit tests and anonymous namespaces.
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: ‘MyTest_ATest_Test’ has a field
‘MyTest_ATest_Test::anonymous’ whose type uses the anonymous
namespace
warning: ‘MyTest_ATest_Test’ has a base ‘unnamed::MyTest’ whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
In this case the solution is easy. Erase the class MyTest definition
and use TEST instead of TEST_F.
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: Unit tests and anonymous namespaces.
On Tue, Mar 3, 2009 at 4:49 PM, William Chan (陈智昌) willc...@chromium.org wrote: My old team never really used FRIEND_TEST. We found it ugly that our production code depended on test code. We typically used friended Peer classes defined in the unittest file, but not in the anonymous namespace. These are simple shims that provide access to the private section. It also saves on having to FRIEND_TEST each individual test as you add them. It looks like almost every time FRIEND_TEST is used, it's used for multiple tests, not just a single one. I'm not sure how much of a problem chrome has with build dependencies leading to rebuilds, but it was very annoying in google projects to add a FRIEND_TEST to a widely used .h file, thus forcing everyone to rebuild, even though you're only adding a new test. In some cases we make a class that derives from testing::Test a friend, and write pass-throughs there. I agree it's a good idea if we have many tests that need this for the same tested class. We don't have a whole lot of FRIEND_TESTs in the first place, so it probably isn't a big deal for us. 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: Thoughts on copy/paste
On Tue, Mar 10, 2009 at 7:44 AM, Avi Drissman a...@google.com wrote: I actually don't know. It seems like just simple state gathering but it might be more. The big concern is that it fits our model but no one else's. Everyone else is OK with the UI asking the renderer questions directly, and as that's more efficient than doing status updates that are going to be ignored 99.44% of the time, I'm not sure they have incentive. I'm not sure what you mean by this. What is everyone OK with? 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: Thoughts on copy/paste
On Wed, Mar 11, 2009 at 10:01 AM, Avi Drissman a...@google.com wrote: Most platforms (meaning not Chrome) have the ability, when the user pulls down a menu, to adjust the states of the menu items within. For them, when the user selects the Edit menu, they're OK calling into WebCore and asking the Editor canCopy(), canPaste(), etc. Do you mean when you click on the menu, blocking the browser until the renderer responds with whether the menus can be enabled? I would be opposed to that. We're pretty much the only ones decoupling the UI from WebCore to the extent that we disallow blocking. With that design, we're forced to have WebCore (or at least glue/) bubble up state changes. Compared to just asking WebCore at the time of need, having state change notifications is more inefficient. I think it's worth paying the price. But it might be a harder sell to upstream. State changes won't fix the problem either. There is no way in the browser to know about the current state of the renderer without one or the other one blocking. State change notifications don't address this problem. 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: Thoughts on copy/paste
On Wed, Mar 11, 2009 at 10:11 AM, Amanda Walker ama...@chromium.org wrote: On Wed, Mar 11, 2009 at 2:07 PM, Brett Wilson bre...@chromium.org wrote: Do you mean when you click on the menu, blocking the browser until the renderer responds with whether the menus can be enabled? I would be opposed to that. So are we, but that's what WebKit currently assumes. That's why we've been trying to brainstorm a way around it. This seems like it is not worth worrying about until the mac browser is as good as the Windows one. We have this bug on Windows, and it's not held anything up so far. 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: Doing some research with views and GTK.
On Sun, Mar 15, 2009 at 7:37 AM, Dean McNamee de...@chromium.org wrote: I had a discussion about Views with Scott. I think I am on the side of the fence that porting views it not a good idea. One of the things that came up is remote X, would it be possible to ever have good remote X performance with the Views panting model? I wouldn't want to paint ourselves into a corner (dum dum dum). It was my understanding we were already using Skia for a lot of this drawing, so the existing approach wasn't going to make remote X too happy either. 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: Doing some research with views and GTK.
On Sun, Mar 15, 2009 at 10:09 AM, Adam Langley a...@chromium.org wrote: On Sun, Mar 15, 2009 at 8:55 AM, Dean McNamee de...@chromium.org wrote: UI elements like buttons are different, my understanding is that (at least in theory) it should be possible for all of those images of the different states to live on the X server, so they don't need to be sent over the network for every expose. I'm not sure how we would do this with a Skia based drawing pipeline. Although a displaylist like interface would be very nice, it's basically not going to happen. However, I wouldn't worry about drawing the UI elements as a bitmap too much. Firstly, the number of pixels isn't that large. The TabStrip, Omnibox, buttons etc probably take up 5% of the screen, This is a good point. It probably doesn't matter what we do with the tabstrip and toolbar when the page is rendered to a giant DIB. While I would like to do a displaylist-like painting for the page, I don't see this happening in a way that can be reasonably remotable on X. 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: rev 11763 removes frame pointer from ResourceRequest
On Mon, Mar 16, 2009 at 4:25 PM, Marshall Greenblatt magreenbl...@gmail.com wrote: My needs in this case are related to the Chromium Embedded Framework (CEF). The code currently works as follows: 1. Retrieve our custom WebViewDelegate instance pointer via the WebFrame pointer passed to ResourceLoaderBridge::Create(). 2. Pass a CefBrowser pointer retrieved from our custom WebViewDelegate instance into the new ResourceLoaderBridgeImpl. 3. During the execution of RequestProxy::AsyncStart() access callbacks provided by CefHandler (retrieved from CefBrowser) to optionally provide alternate data streams, transparently redirect requests, etc. You can view the source code here: http://code.google.com/p/chromiumembedded/source/browse/trunk/libcef/browser_resource_loader_bridge.cc Does chromium currently maintain the relationship between the 'routing_id' parameter and the WebFrame pointer somewhere within the code base, or will we need to add this mapping capability ourselves? Or is there perhaps a completely different approach that CEF should be using to provide this capability? Routing IDs and frames are different things. If I recall correctly, there can be many frames on a page that would share the same routing ID, or in the case that John mentioned, there can be routing IDs but no frame. 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: Omnibox q around Mac+Chromium best-practices.
I would do the thin Objective C mode, partially since all Chrome developers know C++. I could debug that code or make changes to it if I was doing something that affected it, but I would have a much harder time with Objective-C. Brett On Thu, Mar 19, 2009 at 4:00 PM, Scott Hess sh...@chromium.org wrote: I'm refactoring my Omnibox code towards something I'm willing to put up for review, and am realizing that I need to find a way to rule on whether I should have thick Objective-C helpers or thin ones. Say for instance that I have an NSTableView, I'll need a data source for that, which needs to be an Objective-C object. At the thin extreme, I can put the minimum amount of code in that object to fulfill the data source protocol, plus anything I need for handling delegation or target/action type things, which leaves setup and wiring in the C++ code. At the thick extreme I would push most of the Objective-C code into the Objective-C object, and have the C++ code call into that. Or there's something in the middle. WDYT? Right now it's somewhere in the middle. I don't create Objective-C methods solely to be called from C++, nor C++ methods solely to be called from Objective-C, except for cases where either would need to poke through the encapsulation boundary. Thanks, scott --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Including Droid font in Chrome?
On Wed, Mar 25, 2009 at 10:16 AM, Alex Russell slightly...@google.com wrote: MSFT did a new set of fonts to take specific advantage of ClearType: http://www.microsoft.com/typography/ClearTypeFonts.mspx The Core Fonts are still available and installed by default on all windows versions that I can find information on, but they were designed for a simpler time when sub-pixel hinting was still rare. I'm not sure that implies that we should do anything in particular WRT Droid, though. I have them on my Windows box. They are nice fonts, but I think they are optimized for the antialiasing algorithm on Andorid: I don't find that they look quite as good on Windows using ClearType. For a while, I was interested in including their fallback font which we could use to guarantee that glyphs are available for most characters. But (I assume) for space reasons on the phone, this font is not hinted, so looks pretty ugly. 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: ScrollBar Implementation
As Peter said, the ScrollBar object is not responsible for drawing scroll bars on pages, which is what the bug is about. Brett On Wed, Mar 25, 2009 at 1:15 PM, Mohamed Mansour m0.interact...@gmail.com wrote: There is no context menu for the ScrollBar. -m0 On Wed, Mar 25, 2009 at 3:55 PM, Ben Goodger (Google) b...@chromium.org wrote: No. What's not working? -Ben On Wed, Mar 25, 2009 at 12:54 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Do you recommend me to refactor the Context Menu code from BitmapScrollBar into ScrollBar so that both will have context menu implementations? Currently, the context menu is the page context menu that has reload, print, view page info, etc. Would that be the best approach fixing this feature bug. http://code.google.com/p/chromium/issues/detail?id=7231 -m0 On Wed, Mar 25, 2009 at 3:49 PM, Ben Goodger (Google) b...@chromium.org wrote: There are different kinds of scrollbar supported by views... that native scrollbar and the bitmap scrollbar. bitmap scrollbar isn't used right now but likely will be again. ScrollBar is the base class of both NativeScrollbar and Bitmapscrollbar. Scrollview has a ScrollBar* so it doesn't have to care about the particular implementation. -Ben On Tue, Mar 24, 2009 at 6:36 PM, Mohamed Mansour m0.interact...@gmail.com wrote: Hi chromium, I cc'd Ben cause he seems to be working on UI on chrome alot :x Sorry Ben! I am working on a feature http://code.google.com/p/chromium/issues/detail?id=7231 which requires context menu support for Chromium scroll bars. While searching the code I have come up with these classes: - NativeScrollBar - ScrollBar - ScrollView - BitmapScrollBar The context menu functionality should have the following (system wide in windows, every window that has scrollbar, has that context menu) Horizontal: 1) Scroll Here 2) Left Edge 3) Right Edge 4) Page Left 5) Page Right 6) Scroll Left 7) Scroll Right Vertical: 1) Scroll Here 2) Top 3) Bottom 4) Page Up 5) Page Down 6) Scroll Up 7) Scroll Down So my question is as follows, Chromium uses ScrollView as its main scroll bar. It doesn't use NativeScrollBar, nor BitmapScrollBar. So why do those exist? Why do we have a ScrollBar class? Why can't we just use ScrollView and instead of passing in a ScrollBar, are we assuming Chromium will have *many* different types of scrollbars? Why are we not using native windows scrollbar support, since chromium is creating different views in every operating system, why not just use the standard windows components? There are many functionality within the scrollbar, how would you guys advice me to continue? Should I create a separate class called ScrollBarContextMenu and implement the horizontal and vertical context menu for scroll bars? There must be an easier way, to just use windows/gtk/mac controls for scrollbar support, instead of recreating it from scratch. Ideas? --~--~-~--~~~---~--~~ 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: Should all automatable browser APIs have per-object unique IDs?
On Thu, Mar 26, 2009 at 9:41 AM, Aaron Boodman a...@chromium.org wrote: I'm starting to dig into implementing the tab and window API as a first example of all the other browser-automation type APIs we eventually want to implement for extensions. One of my early assumptions was that the APIs we exposed would be service-style, where we pass dumb json data structures back and forth between the extension process and the browser process. As such, it seems intuitive that each object must have a unique identifier. For example, each tab should have a tab ID and each window should have a window ID. (For more detail, see here: http://dev.chromium.org/developers/design-documents/extensions/api-pattern) The automation system already maintains such a list of IDs for tabs, browsers, and windows for a similar reason. But they are maintained in the automation class hierarchy and not particularly useful to me. I was wondering what people thought of the idea of changing these objects to have built-in IDs for automation purposes. So, for example, each Browser would have an ID, and there'd be a map of ID to instance maintained in BrowserList. Clients (the automation system and the extension system) then wouldn't have to maintain their own mappings, but could call BrowserList::GetBrowserById(), or something. Same with Tabs, Downloads, Bookmarks*, and History*. What would IDs refer to in history? 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: Should all automatable browser APIs have per-object unique IDs?
On Thu, Mar 26, 2009 at 10:09 AM, Aaron Boodman a...@chromium.org wrote: On Thu, Mar 26, 2009 at 9:52 AM, Brett Wilson bre...@chromium.org wrote: What would IDs refer to in history? I've only barely looked into the history data model, but I was hoping it would be a persistent ID for each visit, which is unique across the profile. It appears that there is such an ID in the visit table. Could I use that? Those are exposed, but I wonder which cases an extension needs to refer to them. They are internal IDs generated by sqlite. If you clear your history, you'll get re-used ones. This could also theoretically possible happen if you delete a single item, but I don't know if this happens in practice. So they are often persistent, but the way you ask makes me nervous. 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: Should all automatable browser APIs have per-object unique IDs?
On Thu, Mar 26, 2009 at 11:37 AM, Aaron Boodman a...@chromium.org wrote: On Thu, Mar 26, 2009 at 11:22 AM, Brett Wilson bre...@chromium.org wrote: Those are exposed, but I wonder which cases an extension needs to refer to them. They are internal IDs generated by sqlite. If you clear your history, you'll get re-used ones. This could also theoretically possible happen if you delete a single item, but I don't know if this happens in practice. So they are often persistent, but the way you ask makes me nervous. I'm thinking of a use case where somebody wanted to synchronize browser history between machines. It would be more convenient to do this if there was a unique client side ID that didn't change for each visit. I'm OK exposing this as long as we set expectations correctly. 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: Sandbox Interception Design
On Fri, Mar 27, 2009 at 1:50 PM, Book'em Dano daniel.c...@gmail.com wrote: At the top of sandbox\src\interception.h, it refers the reader to: http://wiki/Main/ChromeSandboxInterceptionDesign. Does anyone know where this document can be found? The sandbox info on http://dev.chromium.org/developers/design-documents/sandbox is too high level...I want to find out more details about the interception framework and design. The internal doc is the same as the external one you found, so there probably isn't one more detailed (but I don't know for sure). 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: How to fix check deps failure?
On Mon, Mar 30, 2009 at 10:12 AM, Jian Li jia...@chromium.org wrote: I got another failure for checkdeps: ERROR in C:/b/slave/chromium-rel-builder/build/src\chrome\test\worker\test_webworker.cc Illegal include: webkit/tools/test_shell/test_webworker_helper.h Because of -webkit from chrome's include_rules. Should I relocate test_worker project from chrome\test\worker to webkit\tools\test_worker? Thanks. We already have webkit/tools/test (it just has the reference build in it). How about putting it there? 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: [Extensions] New API specs: bookmarks, history, downloads
On Wed, Apr 8, 2009 at 7:48 PM, Nick Baum nickb...@chromium.org wrote: Hi all, I fleshed out a few more APIs. I've put them in separate documents since the API pattern doc was getting a bit long. Below are some notes, feedback appreciated. In particular, I'd love feedback from Scott on history and from Paul on downloads. -Nick Bookmarks Do we want to distinguish removeBookmark from removeFolder, or is that unnecessary? Should changes to the contents of a folder trigger eventbookmarkupdated for that folder? How about the folders above it? In BookmarkItem, should fields that don't apply be null or simply not present? In BookmarksQuery, do the root and bookmarksBar booleans make sense? How does returning the children recursively work with updates? Can you update all these items at the same time? Downloads Should getDownloads take a DownloadsQuery object? The current downloads page includes search. What kind of events does clearAllDownloads trigger? do we need a separate event for this? Do we even need this in the first place? How should we deal with progress updates? It seems like overkill to trigger an event for each change in percentage, but on the other hand extensions should be able to track this. History I'm assuming HistoryItems are immutable, so there is no update. The internal history structure is split between visits and urls. Visits don't contain the actual url, so we have to fetch the url object either way. I therefore merged the visit and url objects into one. Is this reasonable? There are a number of stats (timeSpent, fromId, totalVisitCount, totalTypedCount). Do we want to expose those for v1? I would recommend doing as little as possible for history. I worked on a fancy querying system for Firefox history which has not really been used for great effect. We should add features as necessary for things we want to enable. The visits and URLs are split out for space reasons. The URL record has a couple of strings and a bunch of numbers, and the results will contain many copies of the URL (for example, imagine the URL, favicon URL, and title (many common titles are actually quite long) for your homepage being duplicated hundreds of times for a longish history query. If the results are limited to 100, space doesn't matter, but splitting them makes some things easier. Say I want to display only URLs, it keeps me from having to go through every one and uniquing them. So I guess either way is OK with me. I like the idea of limiting the result set to 100. It's too easy to shoot yourself in the foot. I also think you should not be able to do a query over too much time. For full text search results, we've spent a lot of time doing these in chunks. Doing a search for something over all time can easily churn through hundreds of megabytes of data. Our history code does it incrementally by month, so I think this would be a good thing to encourage in the API design that you can't do a query over more than one month. int timeSpent: we don't know the time you spent on the page, so remove this. optional int[] ids: I don't understand what this is. int totalPages: I assume you mean the total number of result sets of 100? This number is not possible for us to compute for queries with full text searches, and is merely very impractical for other ones. optional int page: We can not efficiently do full text queries by count give me the 30-40th results since it must find every match for the text before it can do any such computation. Given my suggestion to keep it simple, I think the query API should more closely reflect the current C++ API, which was carefully designed to be efficient. Instead of asking for individual pages, you do a query over a time range for up to 100 results. If you want more results, you start at the last date and continue to go back in time, getting more results. This requires some more work by extension authors, but not a lot more. If we see there are many applications with certain needs, we should design better APIs for those specific needs. 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: [linux] GTK and multi-threading
On Fri, Apr 10, 2009 at 12:42 PM, Evan Stade est...@chromium.org wrote:
a) wrap the clipboard calls in gtk_threads_{enter,leave}(). This makes
the calls thread safe, but it does so by locking a process-wide mutex,
which will block our UI thread's main loop.
I personally think this is OK. These calls should be on the UI
thread and we'd put them there if we could. So having them on another
thread blocking the UI thread seems equivalent. We just need to be
careful to limit the locking to just that.
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: question on profiles ChromeThread
On Mon, Apr 13, 2009 at 11:01 AM, Tim Steele t...@chromium.org wrote: I don't know of a reason we need more than 1 history thread, but I can't say for sure. HistoryService is responsible today for starting/stopping/destroying the history thread, so if we have multiple services then we need to be smarter about that... Anyone opposed to moving history_thread to BrowserProcess and mimic what is done for the other threads? HistoryService::Cleanup does some fancy footwork and ultimately joins on it's thread, but afaict it doesn't need to [join]. I can think of other options but they feel much less cool, like saying that the default profile's history service manges the history thread, or having a refcountable HistoryThread type, ... We need the UI thread to Join with the history thread to ensure that all the data is written. If it doesn't join, the history thread will be force-terminated when the UI thread exits. However, the BrowserProcess destructor should do this join for you when it deletes the thread. I don't think there's any reason other than completeness that the history thread needs a name, so just removing that is the easiest thing. As you can see, history shutdown is tricky. But I don't have an objection if you want to move it to BrowserProcess, either, as long as you fix any problems that arise :) 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] WebContents is dead, long live TabContents!
I just moved the remaining methods from WebContents to TabContents. I kept the methods and data members the same, just merged the classes, so it should be easy to find what you're looking for (or to resolve conflicts if you have local changes to web_contents.cc). Hopefully this will make the complexity in this area much more manageable. We still have a shell class called WebContents (derived from TabContents) and references to it, which are temporary so I don't have to immediately change all references to it. It will hopefully soon go away. FAQ: Why did we do this? Chromium 1 used a variety of specialized tab contents types which we wrote native interfaces for. This included the history and downloads page. TabContents was the base class providing shared functionality and an external API, and there were a variety of subclasses for the different tab types. WebContents was the TabContents specialization for web pages. There were several problems with this. The original idea is that our native UI would be more awesome than a web page could ever be. But it turned out to be teh suck because a variety of things didn't work (find in page, printing, selection) and because we didn't end up doing anything fancy in there anyway. With native UI, we would have to write it again for every platform, which nobody was excited about doing. There were also very difficult bugs associated with switching tab contents types. For example, in Chromium 1, if you go to the history page, navigate to a regular web page, go back, then navigate to a different regular web page, you would see a flash of the first web page during the second transition. This was the tab contents types being switched between a HistoryContents and a previously-used WebContents. So now we only display web pages inside tabs. This makes the switching problem go away. Glen rewrote history and downloads we be special web pages with additional permissions, and I did a whole heckuva lof of refactoring to delete all the old TabContents switching. We still swap RenderViewHosts inside the TabContents when we need to switch processes. 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: DO NOT ADD DEPENDENCIES FROM CHROME/VIEWS on CHROME/COMMON
On Fri, Apr 24, 2009 at 10:10 PM, Aaron Boodman a...@chromium.org wrote: Is it possible to enforce these rules with code rather than capital letters? I don't think I've made this particular error, but it's something I could see myself doing. From their names, chrome/common seems like a reasonable thing to include from chrome/views. Just a thought. Don't we have something already to enforce dependency rules like this? The problem is that there are already some dependencies, so we can't enforce them. The dependencies are checked on a per-directory basis. I would like to prevent people from including browser from renderer_host and tab_contents but can't for the same reason. We could have some complicated exception list in the checkdeps tool, but that's much harder than shouting on the mailing list. 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: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Mon, Apr 27, 2009 at 8:05 AM, Mike Pinkerton pinker...@chromium.org wrote: On Sun, Apr 26, 2009 at 5:50 PM, Amanda Walker ama...@chromium.org wrote: Application startup is one of the areas where we count every millisecond, and try to touch the disk as little as possible. I don't think it's safe to assume that the cost of creating and writing to a file is negligible in this context without actually measuring it. Just curious, how many files are read/written loading a profile: history, bookmarks, cache, last session, preferences, cookies, etc? I imagine it's non-trivial and happens at every startup. Cache is loaded synchronously on the I/O thread, along with the cookies. The history is loaded asynchronously on the history thread. The bookmarks are loaded asynchronously on the file thread and passed to the UI thread. The only things loaded synchronously are the ones we have to have to continue, which are the preferences and, when open the last session is set in the preferences, the session file. I agree that the proposal just for the sake of consolidating code may not be warranted, but to jump all over it because a file has to be written seems like premature optimization in light of everything else that happens on the disk at app startup. We start up significantly faster than any other browsers because we worry about this stuff. When the disk is thrashing (like right after you start your computer) there will be an even larger difference. 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: unforking: 20% perf hit for international page cycler expected soon
On Sat, May 2, 2009 at 6:09 AM, Evan Martin e...@chromium.org wrote: The suggestions on that code review are good: we ought to measure how many fonts normal users see, and then pick the cache tuning parameter accordingly. Adam Barth is a good person to ask about how to do this, since he seems to be measuring all sorts of things. I think the correct answer is # of fonts used per some unit of time. The total number of fonts will monotonically increase over time. Instead, we want to know how many fonts we should keep in the cache, which is temporal. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Why are pref keys wchar_t's?
2009/5/1 Mohamed Mansour m0.interact...@gmail.com: Why wouldn't we just use std::string ? Many places in the code uses std::string. DictionaryValue needs to be converted as well as many others. So what do we finally decide, go what Pink stated and use char* or use std::string. I believe the reason is that you pass predefined constants to the pref functions. The constants are either char or wchar_t, we don't allow global objects like std::string. The caller never has to deal with these parameters, just pass the correct constant, so using string objects doesn't help anything. If we changed to std::string objects, it would require constructing a string object and making a copy through a heap allocation, so it's wasteful for no benefit to use std::strings here. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Passing PlatformGraphicsContext from WebKit to Chrome code
On Wed, May 6, 2009 at 8:40 AM, Andrew Scherkus scher...@chromium.org wrote: WebKit's MediaPlayerPrivate interface is a bit backwards where they pass in a GraphicsContext: http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/WebKit/WebCore/platform/graphics/MediaPlayer.h?view=markup We currently forward the MediaPlayerPrivate::paint(GraphicsContext* p, const IntRect r) call into Chrome, converting the GraphicsContext to a PlatformContextSkia in the process. We realized today this doesn't work on Mac since it's actually a CGContext. Without resorting to #ifdefs everywhere, what's the best way to pass a PlatformGraphicsContext back to Chrome? Can you just pass a pointer to the pixels and the size? I think the pixel format should be well-defined on each platform, and it should be easy to make a SkBitmap in the Chrome side that uses those pixels directly. 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: Passing PlatformGraphicsContext from WebKit to Chrome code
2009/5/6 Andrew Scherkus scher...@chromium.org: On Tue, May 5, 2009 at 9:49 PM, Albert J. Wong (王重傑) ajw...@chromium.org wrote: On Tue, May 5, 2009 at 9:38 PM, Amanda Walker ama...@chromium.org wrote: Ah, I see. Hmm, going in that direction (from a GraphicsContext back up to the PlatformCanvas that wraps it) is an interesting question (the rest of our rendering code goes in the other direction). And as Brett can attest, this isn't the first time that PLATFORM(CG) has caused a headache--we just (so far) haven't been willing to duplicate/fork all of the Mac rendering code that currently lives in PLATFORM(CG), since there's a lot of it (especially text handling) that would end up the same. Brett's idea (change your paint routine to take a raw pixmap (an RGBA32Buffer), which we can easily draw into a GraphicsContext on any platform, is basically what the ImageDecoder subclasses do (including rudimentary multi-frame support for things like animated GIFs). If that will work, it's probably the easiest to integrate into all 3 platforms, since the dirty work has already been done. If that's too many frame copies for video, we can write a function that can create, say, an SkBitmap* pointing at the destination bits given a GraphicsContext and a rectangle. Would either of those approaches work? They both sound workable, and might be the best option at this point. We'll ping WebKit to find out the reasoning behind passing in a GraphicsContext. Probably for performance reasons and reducing extra blits/copies, but still worth investigating. For some background information... each platform implementation of MediaPlayerPrivate either stores video frames on a platform-dependent surface (cairo_surface_t, SkBitmap) or uses a platform-dependent context (HDC, NSGraphicsContext) to interact with an external library (QTKit on mac, QuickTime on Windows). I'm a bit confused at how the SkBitmap* creation would work (mostly how to deal with scaled elements, but I'm no Skia expert). The RGBA32Buffer sounds like a pretty reasonable first attempt. Upstreaming our media glue code might clean up some things as well. Any transforms will be lost when you do this, of course. But the only cross-platform way of representing that with the surface is through a GraphicsContext object. 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: Skia is moving
On Tue, May 12, 2009 at 2:48 PM, Nicolas Sylvain nsylv...@chromium.org wrote: On Tue, May 12, 2009 at 2:22 PM, Stephen White senorbla...@chromium.org wrote: I'm in the process of updating chromium to use tip-of-tree skia, and in the same CL, moving skia to a third_party directory, retrieved via DEPS. What this means for you: Hi, Why do you want to move it to third party? The other projects developed at google that we fetch through DEPS like googleurl, breakpad, gtests and courgette are all living in src, not third_party.. Nicolas If our repo is not the canonical representation, they should all be in thierd party. So I think that all your examples are in the wrong place. 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: Skia is moving
On Wed, May 13, 2009 at 12:39 PM, Darin Fisher da...@chromium.org wrote: That is what I meant by pain It only applies to third party code that conforms to the google style guide, which says that all include paths must be relative to the root. Come to think of it, I think this could cause problems for such projects, since I'm sure the V8 developers don't think of V8 as a third party piece of code, and in their world, V8 is not checked out into a directory named third_party. We'd probably end up having to add src/third_party to the include path or something like that. We already do this for all third_party code, but from chrome we include it with the third_party prefix. I think this is a good approach. In serverland, they actually change the includes in the third party files which I don't think we should do. 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: Changes to FilePath?
On Wed, May 13, 2009 at 3:51 PM, Amanda Walker ama...@chromium.org wrote: Perhaps what we need is a companion to FilePath. For example: FilePath: much as it is now, lightweight, alternative to string manipulation. FileReference: heavierweight, can talk to the file system and have carnal knowledge of platform specifics for things like resolving / canonicalizing pathnames, determining whether or not they refer to the same files, generating C strings that can be passed to 3rd party libraries, etc. I think this is very dangerous. I think Greg should not be talking to the filesystem when inserting filenames into a set. We don't allow filesystem access from the UI thread of Chrome, and I think other parts of our system should also not do filesystem access on their critical threads, especially if they want to be more part of Chrome in the future. 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: Changes to FilePath?
On Wed, May 13, 2009 at 4:34 PM, Amanda Walker ama...@chromium.org wrote: On Wed, May 13, 2009 at 7:07 PM, Brett Wilson bre...@chromium.org wrote: On Wed, May 13, 2009 at 3:51 PM, Amanda Walker ama...@chromium.org wrote: Perhaps what we need is a companion to FilePath. For example: FilePath: much as it is now, lightweight, alternative to string manipulation. FileReference: heavierweight, can talk to the file system and have carnal knowledge of platform specifics for things like resolving / canonicalizing pathnames, determining whether or not they refer to the same files, generating C strings that can be passed to 3rd party libraries, etc. I think this is very dangerous. I think Greg should not be talking to the filesystem when inserting filenames into a set. We don't allow filesystem access from the UI thread of Chrome, and I think other parts of our system should also not do filesystem access on their critical threads, especially if they want to be more part of Chrome in the future. But in context, he's passing these things to 3rd party libraries that will be doing plenty of file system access (importing and exporting data, for example). That's why I was suggesting something separate from FilePath for such use. Then he doesn't need canonicalization at all. He needs to know how the third party library is going to use the string for filesystem access and then do the corresponding transformations. That does not involve filesystem access. 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: Changes to FilePath?
On Wed, May 13, 2009 at 6:12 PM, Greg Spencer gspen...@google.com wrote: On Wed, May 13, 2009 at 4:07 PM, Brett Wilson bre...@chromium.org wrote: On Wed, May 13, 2009 at 3:51 PM, Amanda Walker ama...@chromium.org wrote: Perhaps what we need is a companion to FilePath. For example: FilePath: much as it is now, lightweight, alternative to string manipulation. FileReference: heavierweight, can talk to the file system and have carnal knowledge of platform specifics for things like resolving / canonicalizing pathnames, determining whether or not they refer to the same files, generating C strings that can be passed to 3rd party libraries, etc. I think this is very dangerous. I think Greg should not be talking to the filesystem when inserting filenames into a set. We don't allow filesystem access from the UI thread of Chrome, and I think other parts of our system should also not do filesystem access on their critical threads, especially if they want to be more part of Chrome in the future. Well, so the use I have for this in O3D at the moment is in our importer, which currently is a separate command-line tool that reads Collada files and writes out our wire format for geometry. So it isn't meant to be occuring in a UI thread, but I could see times when it might be useful to know for sure if two files reference the same file in the UI thread (dragging and dropping a file onto a drop zone, for instance). I do need to know if I have the same file more than once in a set because the COLLADA file might reference the same texture multiple times, or (more dangerous) it might reference a file that is one file on Windows, but (incorrectly) maps to two different files in the (Unix-path-format) .tgz files. To detect that, I need canonicalization. You can't actually canonicalize a filename on Windows, so I think it's dangerous to write a component that claims to do it. I think you just need to come up with some simple rules that makes it work most of the time. Personally I would do ASCII lowercasing and stop worrying about it. If you use ICU to lower-case correctly, Windows won't necessarily agree and you won't be able to use that file. 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: Changes to FilePath?
On Wed, May 13, 2009 at 7:24 PM, Brett Wilson bre...@chromium.org wrote: On Wed, May 13, 2009 at 6:12 PM, Greg Spencer gspen...@google.com wrote: On Wed, May 13, 2009 at 4:07 PM, Brett Wilson bre...@chromium.org wrote: On Wed, May 13, 2009 at 3:51 PM, Amanda Walker ama...@chromium.org wrote: Perhaps what we need is a companion to FilePath. For example: FilePath: much as it is now, lightweight, alternative to string manipulation. FileReference: heavierweight, can talk to the file system and have carnal knowledge of platform specifics for things like resolving / canonicalizing pathnames, determining whether or not they refer to the same files, generating C strings that can be passed to 3rd party libraries, etc. I think this is very dangerous. I think Greg should not be talking to the filesystem when inserting filenames into a set. We don't allow filesystem access from the UI thread of Chrome, and I think other parts of our system should also not do filesystem access on their critical threads, especially if they want to be more part of Chrome in the future. Well, so the use I have for this in O3D at the moment is in our importer, which currently is a separate command-line tool that reads Collada files and writes out our wire format for geometry. So it isn't meant to be occuring in a UI thread, but I could see times when it might be useful to know for sure if two files reference the same file in the UI thread (dragging and dropping a file onto a drop zone, for instance). I do need to know if I have the same file more than once in a set because the COLLADA file might reference the same texture multiple times, or (more dangerous) it might reference a file that is one file on Windows, but (incorrectly) maps to two different files in the (Unix-path-format) .tgz files. To detect that, I need canonicalization. You can't actually canonicalize a filename on Windows, so I think it's dangerous to write a component that claims to do it. I guess you could call GetShortPathName every time you see a name. But I think that's a crazy solution. I still think you should do my suggestion below. I think you just need to come up with some simple rules that makes it work most of the time. Personally I would do ASCII lowercasing and stop worrying about it. If you use ICU to lower-case correctly, Windows won't necessarily agree and you won't be able to use that file. --~--~-~--~~~---~--~~ 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] Clobber if you get V8 linker errors in the next sync
When I landed the WebKIt integration this morning a bunch of the bots needed clobbering due to linker errors. The errors were missing V8 handles while linking generate_profile. I haven't seen any of the commonly-compiled projects fail because of this, so it's possible you don't have to clobber since you probably don't care about this project. 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: Question about chromium architecture
On Tue, Jun 2, 2009 at 4:57 PM, Meryl Silverburgh silverburgh.me...@gmail.com wrote: I am reading this document http://dev.chromium.org/developers/how-tos/getting-around-the-chrome-source-code; about chromium source code: It said: renderer: Code for the subprocess in each tab. This embeds WebKit and talks to browser for I/O. Does that mean chromium do not use the HTTP stack/library that Webkit is using? WebKit uses different HTTP stacks depending on the port. We use a different one than any other ports. And does that mean all the I/O of each tab (webkit renderer) will be sent to 'browser' for I/O, and there is 1 I/O thread in the browser to handle the I/O requests from ALL tab? Correct. 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: [extensions] Gleam API proposal
On Wed, Jun 3, 2009 at 1:00 PM, Aaron Boodman a...@chromium.org wrote: [I actually created this awhile ago, but never sent it 'round] It is really common for extensions to want to offer additional contextual options for content elements. For example Download this movie, edit this image, call this phone number, etc. You can use content scripts for this, but since it is such a common task, I think it deserves its own proper API. This would allow this class of extensions to interact well with pages, with each other, and have a nice consistent UI. Anyway, here's an API proposal for such a thing: http://dev.chromium.org/developers/design-documents/extensions/gleam-api Comments wanted! I think this general concept would be great. Say I have an extension that allows me to open images in Photoshop. Does that mean all images would have these things highlighting them? That seems like it would be annoying. When I read this, I first thought it was just a way to insert stuff into a context menu when the user right-clicks on that. Perhaps having both modes would be useful. The image editing or video download extension would register itself as interested in these things only from the context menu, and they'd get a chance to insert commands into that menu to do whatever they want. This could also be useful for less-intrusive extensions. I want the ability to right-click on one to open it in my favorite mapping provider, but I *don't* want all addresses glowing blue. 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: Question about BrowserRenderProcessHost::OnPageContents() method
On Sat, Jun 6, 2009 at 11:13 PM, meryl silverburgh.me...@gmail.com wrote: Hi, I add debug print status in BrowserRenderProcessHost::OnPageContents () to print out the value of 'contents' void BrowserRenderProcessHost::OnPageContents(const GURL url, int32 page_id, const std::wstring contents); // original code // I add this: cout BrowserRenderProcessHost::OnPageContents endl; cout endl; cout contents; cout endl; } My question is why I only see the 'Text' of the page? I tried loading www.google.ca, but I only see these values: I'm assuming your question is why it doesn't also contain the HTML... This function is called by the renderer with the text of the page so we can index it for history search. As a result, it contains only the text, since we don't want to index the HTML. If you have code that needs to get the full HTML of the page, you need to be in the renderer process and walk the DOM yourself. 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] Mac painting problems
I was informed that I may have broken mac tab contents painting in 18363. I have to leave now, so if this is causing you problems you can back me out. If somebody wants to fix it instead, I'll make you chocolate chip cookies :) 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: How Render process send Image IO request to Browser process
There are a surprising number of comic characters on here. I bet you know Meryl Silverburgh! Generally this type of thing is on the dev site like here: http://dev.chromium.org/developers/design-documents/multi-process-resource-loading 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] Conventions and patterns for multi-platform development
Our team has had somewhat of an ad-hoc approach to organizing code that's different across platforms. In many cases our approach has been quite good. In others, less so, and there have also been questions about what the preferred method for writing a certain component in a cross-platform way. Last night Ben and I wrote a document that tries to clarify this. It's a combination of what we're doing now that works well, and what we probably should be doing: http://dev.chromium.org/developers/design-documents/conventions-and-patterns-for-multi-platform-development This is linked to from the Engineering design documents page. If you're starting a new component or reorganizing an existing one, try to follow these guidelines. It can't possibly cover every case, so you'll have to use your best judgement. Feedback from people who have done a lot of this stuff would be great. Ideally it would be easy to follow and cover most cases for everybody. Thanks! 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: Icelandic dictionary for Chrome
On Fri, Jun 19, 2009 at 1:15 PM, Sverrir Á. Bergsver...@chromium.org wrote: I'm hoping to be able to add Icelandic spelling check to Chrome Before I invest a lot of time I have some quick questions/observations: I found an myspell/aspell dictionary for Open Office here: http://wiki.services.openoffice.org/wiki/Dictionaries#Icelandic_.28Iceland.29 and converted it to bdic format using the instructions here: http://dev.chromium.org/developers/how-tos/editing-the-spell-checking-dictionaries. When I replace a supported bdic with this one I can spell check mostly but seen errors such as: Run-Time Check Failure #2 - Stack around the variable 'candidate' was corrupted. (suggestmgr.cxx:680). Any pointers on where to start? I don't know if the dictionary/conversion are at fault or hunspell. Yikes! I have no good input. Most of these bugs have been caused by the code reading the custom format or in its interaction with the rest of Hunspell. 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: About hunspell
On Fri, Jul 3, 2009 at 12:00 AM, Dmitry.Skibadmitry.sk...@gmail.com wrote: Recently I was playing with hunspell and got very displeased with its code (which claims to be cpp, but in fact 'classified' c). I even started to think about rewriting it in, you know, correct cpp. Then I realized that Chrome also uses hunspell and went to the repository. I saw a very useful patch to HashMgr, which in fact alters its behaviour pretty much. And at the same time all old code were left as is. Glad you like it! So I have a question: why? Why didn't you just remove all that old and unused code? It's unlikely that your patches will be merged into hunspell. We do this so we can tell what we changed so it's easier to merge to future versions of Hunspell. Of course it should be possible to do with SVN logs only, but it makes it a little easier. I believe there are some new capabilities we would like to take advantage of in the newer ones. 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: Watchlist matching any part of a path
On Tue, Jul 7, 2009 at 6:20 PM, Andrew Scherkusscher...@google.com wrote: According to the watchlist file, brettw is watching base from /src kylep sent me a code review that only modifies touches files in /src/media/base, however brettw got added to the cc list. Is this intended behavior for watchlists or should they be more explicit on what they match? I was just wondering the same thing! 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: base::string16 / WebCore::String incompatibility
On Fri, Jul 10, 2009 at 5:04 PM, Jeremy Orlowjor...@chromium.org wrote: WebCore::String has the interesting property of differentiating between an empty string and a null string. In string16, however, there is no such thing as null. The LocalStorage implementation I'm working on proxies data between the rednerer processes and browser process. Some of this data is in the form of WebCore::Strings that need to differentiate between empty and null. I could add a boolean to all IPC messages where the string could be null and then add explicit checking (rather than using the elegant and implicit type conversions that normally happen from WebCore::String - WebKit::WebString - base::string16) but that seems pretty ugly. I think a better solution is adding a new type to base, adding serializing code to common/ipc_message_utils, and adding implicit type conversions between WebKit::WebString and this new type. First of all, is that a good idea? Second of all, what would be a good name for it? NullableString16? What is happening with Darin's WebCore interface stuff. I remember he was planning on having some kind of WebString that could be passed to IPC. Is this still the case, and can we use that instead? 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: base::string16 / WebCore::String incompatibility
On Mon, Jul 13, 2009 at 10:05 AM, Jeremy Orlowjor...@chromium.org wrote: On Mon, Jul 13, 2009 at 8:59 AM, Darin Fisher da...@chromium.org wrote: On Mon, Jul 13, 2009 at 8:20 AM, Brett Wilson bre...@chromium.org wrote: On Fri, Jul 10, 2009 at 5:04 PM, Jeremy Orlowjor...@chromium.org wrote: WebCore::String has the interesting property of differentiating between an empty string and a null string. In string16, however, there is no such thing as null. The LocalStorage implementation I'm working on proxies data between the rednerer processes and browser process. Some of this data is in the form of WebCore::Strings that need to differentiate between empty and null. I could add a boolean to all IPC messages where the string could be null and then add explicit checking (rather than using the elegant and implicit type conversions that normally happen from WebCore::String - WebKit::WebString - base::string16) but that seems pretty ugly. I think a better solution is adding a new type to base, adding serializing code to common/ipc_message_utils, and adding implicit type conversions between WebKit::WebString and this new type. First of all, is that a good idea? Second of all, what would be a good name for it? NullableString16? What is happening with Darin's WebCore interface stuff. I remember he was planning on having some kind of WebString that could be passed to IPC. Is this still the case, and can we use that instead? Brett WebString exists (has for many moons now). It is just a wrapper for StringImpl, the same way WebCore::String is. It is not threadsafe due to the reference counting done for StringImpl. That makes it a very bad candidate for serializing via Chrome IPC. Someone might naively handle such an IPC on the IO thread, and then PostTask the WebString to another thread. We could try to solve that using assertions, but I decided instead to avoid serializing non-POD WebKit API types over Chrome IPC. For reference: http://src.chromium.org/viewvc/chrome/trunk/src/webkit/api/public/WebString.h?view=markup -Darin I guess what I'm proposing is that we have a string data type that can be safely passed across IPC but that has all the possible states a WebCore::String does. I believe adding the null state is the only thing missing. In my code, I've worked around this by passing a boolean along with the string16 which states whether the string is null. This will work, but since AppCache, Database, and maybe others will soon be passing WebStrings through the IPC layer to their backends in the browser process, I'd be surprised if this doesn't come up again. Yes, I think NullableString16 is better than passing a separate bool. 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: Please keep TOOLKIT_VIEWS green!
On Tue, Jul 21, 2009 at 8:42 AM, Nicolas Sylvainnsylv...@chromium.org wrote: On Tue, Jul 21, 2009 at 8:31 AM, Paweł Hajdan Jr. phajdan...@chromium.org wrote: Can we have a trybot with that configuration, which would just compile the code? I think it would really save people's time. I never build with TOOLKIT_VIEWS, and in case of breakage I would have to immediately revert and then investigate. If I got a trybot failure, I would know much more before committing. Ben: How likely is this to break? I'm reluctant to add yet another try bot because they do take a lot of resources. There are maybe 200-300 try changes sent every day. That means we need enough resources to build this configuration that many times, and it needs to answer fast enough. I don't think an automatically-running trybot is very critical for this build at the moment. Most of the things that break it are pretty simple (need to add a file to GYP, or maybe a stub for something) so the breakage won't be super complicated. Most errors will be caught by the regular trybots. 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: [linux] Using system libraries for Chromium
On Wed, Jul 22, 2009 at 2:58 PM, Adam Langleya...@chromium.org wrote: On Wed, Jul 22, 2009 at 9:50 PM, Darin Fisherda...@google.com wrote: Personally, I much prefer the #include png.h approach. Is it a problem to wait for the GYP change that makes this possible? Well, I could make it #include png.h everywhere and then add an include directory in the case that we aren't using the system libpng. If folks are happy with that (it goes against our usual policy of avoid -I flags), then that's cool. Looks like you made this change in the patch, although I don't see the include rule update for chrome.gyp. I guess some obvious ones like png.h and bzlib.h are fine. Some of them give me pause, like event.h, which we also have in views and may also have in other parts of the system, and this will cause compilation problems on Windows (I think). event.h just isn't very clear to me either. You could also have a define USE_SYSTEM_LIBRARIES and you write: #ifdef USE_SYSTEM_LIBRARIES #include png.h #else #include third_party/libpng/png.h #endif Which I find much more readable than your magic define for the png filename. 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: WebContents class in chromium?
On Sat, Jul 25, 2009 at 4:27 PM, n179911n179...@gmail.com wrote: Hi, For http://dev.chromium.org/developers/how-tos/getting-around-the-chrome-source-code, it said For web content, this specialization is WebContents in chrome/browser/web_contents.cc. There are also a few other types for more specialized tabs. We will assume we are creating a web page. But I checkout chromium source code, but I can't find the file web_contents.cc or I can't find the class WebContents anywhere. Can you please tell me what 'tab content' are created for Web content now? Mohammed is correct, the different TabContents types have been removed, and TabContents now handles displaying web pages only. I updated this part of the document. Thanks for pointing this out. 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] How do I filter CC'd code reviews in Gmail?
I added myself to some watchlists and I get a bunch of email about a lot of reviews But this has confused my Gmail filters, which automatic ally label and archive code reviews not addressed directly to me, and all of my CCd code reviews show up in my inbox. Anybody know how to reliably detect the difference between I'm the reviewer and I'm CC'd on a review in a Gmail filter? 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: new hunspell has tons of valgrind warnings... revert?
On Sat, Aug 1, 2009 at 4:34 PM, Dan Kegeld...@kegel.com wrote: I suppose you could try running the hunspell test suite itself under valgrind. Their README tells how to do it, but when I tried, I couldn't get it to work. (Wonder if that means they haven't run it, either?) Hi Dan, Purify has some problems with tracking memory that the OS memory maps, and it ends up giving uninitialized memory reads for any 0xCCs that the OS memory maps (since it doesn't see the write). Does Valgrind have a similar problem? Most of the data is memory mapped. It seems unlikely to me given we didn't have this problem before, but it's worth checking. My main concern is: who is working on this? It's OK like this for a couple of days I guess, but this seems like a potentially serious problem we don't want to file a bug and get to it later. It also seems like Mohammed will need help, and I'll be out part of next week (still figuring out the days). If we can't fond somebody to look at this soon, we should probably back out until there is somebody. 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: How do I filter CC'd code reviews in Gmail?
On Fri, Jul 31, 2009 at 5:44 PM, Rahul Kuchhalkuch...@chromium.org wrote: I thought to:user and -cc:user would work in this case? Have you tried using that in Has the words on Filter edit screen? One problem with this is that it doesn't fix the case where people respond to the review mail via email rather than Reitveld. Then I get put on the to line since I was originally in the reply-to line. I doubt I will have much luck making people stop using email this way, so I will try one of the second email approaches. 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: Do we have any existing code for reading/writing INI files?
On Mon, Aug 10, 2009 at 3:28 PM, Daniel Cowxdaniel.c...@gmail.com wrote: Just wondering if there's any code kicking around somewhere in the codebase for reading/writing INI files? No, we don't deal with ini files at all to my knowledge. We use JSON for that type of thing. What do you need it for? 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: Mac History Menu
On Wed, Aug 12, 2009 at 11:15 AM, Robert Sesekrse...@gmail.com wrote: Two things about the Mac history menu that I'd like people to weigh in on: 1. The Show All History command should have a keyboard shortcut. We can't use the logical Cmd+H because it's bound by the system. Stuart suggested Cmd+Y, as that's what Camino uses. Firefox and Safari both lack keyboard shortcuts for this menu command, so it's really up to us to define it. So Cmd+Y seems as good as any other. Thoughts? 2. Currently, items in the history menu open in the current foreground tab. I'm currently working on a CL to make it so that if you hold down the Cmd modifier while selecting a history/bookmark menu item, it will open in a new tab. I was wondering, though, if we should instead reverse this behavior so that history items (and maybe bookmark?) open in a new tab, and you can use the modifier to open in the current tab. Windows always opens history in a new tab. I think this is the correct behavior: I don't think anybody expects going to history will clobber their current tab. I don't see why there should be a modifier key to clobber the current tab. It seems obscure, useless, and potentially confusing. 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: Mac History Menu
On Wed, Aug 12, 2009 at 12:18 PM, Avi Drissmana...@chromium.org wrote: Brett— Are we talking about the history page, or history items? The history page gets its own tab, sure. But when someone picks an item from the history menu, where does it go? I think current foreground tab is right, with command for background tabs. Yes, I was confused. I think clobbering is OK in that case. My new improved opinion is it should act like the drop-down on the back/forward menus. 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: RenderView page ID
On Mon, Aug 17, 2009 at 2:59 PM, Evan Stadeest...@chromium.org wrote:
I believe Brett meant to say chrome/renderer/render_{view,widget}.{h,cc}
They're both correct, depending on whether you want the browser or the renderer.
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: browser/sync is moving in
On Mon, Aug 17, 2009 at 11:19 PM, estelectronix...@gmail.com wrote: Thanks for the explaination, Nick Carter. I understand now, Chrome is a Google product while Chromium is a open- source project. Any thing else differs between Google Chrome and Chromium? I found dictionaries missing from Chromium. Dictionaries are in chrome/third_party/hunspell/dictionaries You need to copy the one(s) you want to your profile dictionaries directory for them to work. 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: FreeBSD port and ifdefs
On Wed, Aug 19, 2009 at 1:23 PM, Darin Fisherda...@chromium.org wrote: On Wed, Aug 19, 2009 at 1:14 PM, Evan Martin e...@chromium.org wrote: Rad that you're doing this! I think that we shouldn't have any linux-specific directories except for one or two cases where it's *really* Linux-specific (like crash dumping or ). So the same should be true for you. It seems the configurations we'll see most frequently in code are: 1) POSIX (basically, non-Windows -- we have this already) 2) POSIX minus Mac (since Mac has the most extensions, especially at the GUI layer) 3) POSIX minus Linux (aka everything BSD-derived, more or less) Dean proposes a define for #2, agl proposes a define for #3. I think it'd be nice to keep the defines down if possible. I wonder if #2 could be covered by OS_POSIX TOOLKIT_GTK. Or, What about TOOLKIT_VIEWS? Darin is right. There is actually a #4 on Evan's list: POSIX minus Mac minus Views. I did a search for every place we use OS_LINUX. They fall into a couple of cases: - Graphics stuff like X-windows fonts that are shared between TOOLKIT_GTK and TOOLKIT_VIEWS on Linux. - File path handling stuff. Here Linux/BSD are different from Mac, because there is no encoding, while Mac defines one. - Low level stuff like threads, where Mac has something fancy, but we want to use pthreads or whatever on Linux - A very few system info queries that are likely different between Mac, Linux, and *BSD. Maybe this also includes crash reporting? - Some that should be TOOLKIT_GTK instead of OS_LINUX It looks like the vast majority of them fall into the first two categories (graphics and file paths). It would be nice to optimize our ifdefs so these common ones don't get more complicated. 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: FreeBSD port and ifdefs
On Wed, Aug 19, 2009 at 6:00 PM, Dean McNameede...@chromium.org wrote: I kinda feel like this is one of those things you can try hard to premeditate, but in the end you'll just have to deal with it being ugly for a while and hope it eventually converges to something better. The changes in the bulk of the Chrome code are pretty easy to tell in advance. Just search for OS_LINUX. It would be nice if the first pass didn't just tack on OS_FREEBSD to every OS_LINUX in chrome, especially since I think there's a good chance it won't be maintained longer-term. I definitely believe you that there will be a bunch of unknown build/third_party stuff. Sort of a non-answer, but I'd be happy to see this running on a BSD first, and then we can argue about the patch. Are you suggesting getting it running before checking anything in? I think BenL's was planning to do it piecemeal. I just went through some work trying to build it on OpenBSD (promised a friend I'd try). There are a lot of little things we need to do before we even have this debated. Pretty much everything in third_party (icu, libevent), gmock, etc. Some of these will probably require changes upstream. Interesting, thanks for the information. 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: FreeBSD port and ifdefs
On Wed, Aug 19, 2009 at 9:49 PM, Brett Wilsonbre...@chromium.org wrote: On Wed, Aug 19, 2009 at 6:00 PM, Dean McNameede...@chromium.org wrote: I kinda feel like this is one of those things you can try hard to premeditate, but in the end you'll just have to deal with it being ugly for a while and hope it eventually converges to something better. The changes in the bulk of the Chrome code are pretty easy to tell in advance. Just search for OS_LINUX. It would be nice if the first pass didn't just tack on OS_FREEBSD to every OS_LINUX in chrome, especially since I think there's a good chance it won't be maintained longer-term. I definitely believe you that there will be a bunch of unknown build/third_party stuff. Sort of a non-answer, but I'd be happy to see this running on a BSD first, and then we can argue about the patch. Are you suggesting getting it running before checking anything in? I think BenL's was planning to do it piecemeal. Here's an idea I like: Do a pass of porting base and chrome/browser/renderer_host, which are some of the key infrastructure bits that have a bunch of platform-specific stuff in them. It doesn't have to be everything is working perfectly, but rather do a patch to modify the ifdefs as best as you can figure out. Then we can discuss in concrete terms how the ifdefs in the code look and whether they're OK or need rearchitecting. This wouldn't need to block the current build patch, but I think should be done before committing ifdefs to the code. 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: private VM field on the about:memory page
On Mon, Aug 24, 2009 at 6:18 AM, Anand Mistryakmis...@gmail.com wrote: I'm looking at the about:memory page and am wondering how useful is the private VM field? Would it be just as good to have a total VM instead? The reason I ask is because private VM doesn't map easily to Linux where private pages can be shared becuase of copy-on-write. Did you see the memory usage backgrounder? http://dev.chromium.org/memory-usage-backgrounder It's written to be Windows specific, but 80% of the things will apply to Linux. 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: Question about resource_dispatcher_host.h
On Mon, Aug 24, 2009 at 12:49 PM, hap 497hap...@gmail.com wrote: Thanks. But the picture in the document shows there is only 1 ResourceDispatcherHost and there are 2 Renderer Processes: http://dev.chromium.org/developers/design-documents/multi-process-architecture And the ResourceDispatcherHost has access to both Channels for each Renderer Process. Information about each request including the originating renderer is tacked onto each URLRequest in the form of ExtraRequestInfo. See one of the functions in there such as ResourceDispatcherHost::OnResponseCompleted for how this is retrieved. 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] Linux/Mac time epoch change
I just checked in r24417 which changes the 0 for times on Mac Linux (used to mean 1970) to match Windows (1601). This means that the profiles can be copied between systems without getting the dates all wrong, and should also fix some bugs related to cookie expiration ( http://code.google.com/p/chromium/issues/detail?id=14734 ). This will be in the release notes of the next dev version affected, but I wanted to add some extra info for developers who may be affected. When you run a version of Chrome after this, it will do a one-time update of your history database to change the times, so you may notice additional lag before the new tab page or autocomplete results are available. You shouldn't have to worry about exiting in the middle of this process, it should recover. It deletes the full text index of your history, which, because of its size, didn't seem worth updating. You won't get full text search results from before the upgrade, but any browsing after that will be indexed. It also deletes any history older than 3 months rather than migrates it (this won't affect most users of Mac Linux, since they haven't been using it that long). You can load the updated profile in previous versions of Chrome, but the dates will be far into the future, and any data added by the old version will be wrong and won't be fixed when you start using the new one again. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
