[chromium-dev] Re: [LTTF] Flaky tests and setTimeout
On Tue, Oct 13, 2009 at 3:55 PM, Ojan Vafai o...@chromium.org wrote: On Tue, Oct 13, 2009 at 3:50 PM, Andrew Scherkus scher...@chromium.orgwrote: What's happening is loadstart fires and the video reloads which should cause an abort event. For some reason load will occasionally fire after loadstart, ending the test. I know we can patch the test, but I've been digging through the event dispatching code to verify that the flakiness isn't limited to Chromium. Even if it is just limited to Chromium, it is still a bug, right? Unless I'm not understanding, there's no case under which we should be patching the test in the above case. I don't know anything about video events, so I don't really know what event order guarantees there should be. Ojan Yeah it's has to be a bug somewhere, just tricky to track down due to the thread interaction between the video subsystem and the renderer thread. Andrew --~--~-~--~~~---~--~~ 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: if you get gcl error ['svn', 'cat', somefile]
I'm getting such an error message for 'chrome/browser/sync/resources/merge_and_sync.html' come up after the presubmit checks have run (output shows snip ** Presubmit Warnings ** Found line ending with white spaces in ... chrome\browser\sync\resources\gaia_login.html, line 353 Presubmit checks took 2.2s to calculate. There were presubmit warnings. Are you sure you wish to continue? (y/N): y Got error status from ['svn', 'cat', 'chrome/browser/sync/resources/merge_and_sync.html']: snip svn:eol-style is set to LF for the file, and I went to 'Advanced Save Options' in visual studio, selected LF, and saved, and I still get this error :( Is there something else I can try? On Wed, Jun 10, 2009 at 9:59 AM, Marc-Antoine Ruel mar...@chromium.orgwrote: No, Marc-Antoine will enable a presubmit check today (tentative) so it won't happen anymore. And if you are a git user, ya better not break anything. M-A On Wed, Jun 10, 2009 at 12:32 PM, cpuc...@chromium.org wrote: I spent a ton of time tracking this so for the record: If you made a change to some files and gcl change works but when you try to do gcl upload you get this error: Got error status from ['svn', 'cat', 'somefile'] // Copyright (c) 2006-2009 The Chromim authors .. partial dump of the file here Then there are two likely possibilities: 1) the file somefile has inconsistent line endings in your copy 2) the file somefile has a problem as it exists in the svn server For #1 you can fix in your copy (not my case), for #2 you need to contact Marc-Antoine :) Actually Marc-Antoine will post the magic trick here so he would not get pestered again. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] chromium with toolkit_views=1 crashes when inputting anything in location entry.
Hi,
I managed to build a chromium with toolkit_views=1, however it crashes
when inputting anything in location entry. Digging into the code I found in
views/widget/widget_gtk.cc:
void WidgetGtk::Init(GtkWidget* parent,
const gfx::Rect bounds) {
if (type_ != TYPE_CHILD)
ActiveWindowWatcherX::AddObserver(this);
// Force creation of the RootView if it hasn't been created yet.
GetRootView();
#if !defined(OS_CHROMEOS)
default_theme_provider_.reset(new DefaultThemeProvider());
#endif
...
Then default_theme_provider_ will be NULL when OS_CHROMEOS is defined, and
seems that it's defined when using toolkit_views=1. However, there are
many places call WidgetGtk::GetThemeProvider() and access the theme provider
without check, thus cause crash. I'm wondering if it's a bug or I did
something wrong?
Regards
James Su
--~--~-~--~~~---~--~~
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: if you get gcl error ['svn', 'cat', somefile]
Not really sure since the snips you list are two different files, but the warning is not about eol-style, it's about lines ending with spaces, looks on the lines listed for things like: [space][space][real_code][space][space][LF] [space][space][LF] And take the spaces at the *end* of those lines out. TVL On Wed, Oct 14, 2009 at 5:13 AM, Tim Steele t...@chromium.org wrote: I'm getting such an error message for 'chrome/browser/sync/resources/merge_and_sync.html' come up after the presubmit checks have run (output shows snip ** Presubmit Warnings ** Found line ending with white spaces in ... chrome\browser\sync\resources\gaia_login.html, line 353 Presubmit checks took 2.2s to calculate. There were presubmit warnings. Are you sure you wish to continue? (y/N): y Got error status from ['svn', 'cat', 'chrome/browser/sync/resources/merge_and_sync.html']: snip svn:eol-style is set to LF for the file, and I went to 'Advanced Save Options' in visual studio, selected LF, and saved, and I still get this error :( Is there something else I can try? On Wed, Jun 10, 2009 at 9:59 AM, Marc-Antoine Ruel mar...@chromium.orgwrote: No, Marc-Antoine will enable a presubmit check today (tentative) so it won't happen anymore. And if you are a git user, ya better not break anything. M-A On Wed, Jun 10, 2009 at 12:32 PM, cpuc...@chromium.org wrote: I spent a ton of time tracking this so for the record: If you made a change to some files and gcl change works but when you try to do gcl upload you get this error: Got error status from ['svn', 'cat', 'somefile'] // Copyright (c) 2006-2009 The Chromim authors .. partial dump of the file here Then there are two likely possibilities: 1) the file somefile has inconsistent line endings in your copy 2) the file somefile has a problem as it exists in the svn server For #1 you can fix in your copy (not my case), for #2 you need to contact Marc-Antoine :) Actually Marc-Antoine will post the magic trick here so he would not get pestered again. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: chromium with toolkit_views=1 crashes when inputting anything in location entry.
This was fixed in r28916.
-Scott
-Scott
On Wed, Oct 14, 2009 at 2:13 AM, James Su su...@chromium.org wrote:
Hi,
I managed to build a chromium with toolkit_views=1, however it crashes
when inputting anything in location entry. Digging into the code I found in
views/widget/widget_gtk.cc:
void WidgetGtk::Init(GtkWidget* parent,
const gfx::Rect bounds) {
if (type_ != TYPE_CHILD)
ActiveWindowWatcherX::
AddObserver(this);
// Force creation of the RootView if it hasn't been created yet.
GetRootView();
#if !defined(OS_CHROMEOS)
default_theme_provider_.reset(new DefaultThemeProvider());
#endif
...
Then default_theme_provider_ will be NULL when OS_CHROMEOS is defined, and
seems that it's defined when using toolkit_views=1. However, there are
many places call WidgetGtk::GetThemeProvider() and access the theme provider
without check, thus cause crash. I'm wondering if it's a bug or I did
something wrong?
Regards
James Su
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] MacOS link option being accidentally passed on Windows?
Sometime in the last day or two I noticed the following in my build output in Visual Studio when linking chrome_dll: LINK : warning LNK4044: unrecognized option '/System/Library/Frameworks/IOKit.framework'; ignored A gyp problem perhaps? Is anyone else seeing this? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Need your help
Also http://dev.chromium.org/developers/testing/chromium-build-infrastructure/tour-of-the-chromium-buildbot http://dev.chromium.org/developers/testing/chromium-build-infrastructure/performance-test-plots http://dev.chromium.org/developers/testing/chromium-build-infrastructure/performance-test-plots- Pam On Wed, Oct 14, 2009 at 9:22 AM, Paweł Hajdan Jr. phajdan...@chromium.orgwrote: You can read these: http://dev.chromium.org/developers/testing http://code.google.com/p/chromium/wiki/RunningChromeUITests http://dev.chromium.org/developers/how-tos/reliability-tests I also suggest reading code in src/chrome/test. On Tue, Oct 13, 2009 at 09:21, Landon Xue xueyunl...@gmail.com wrote: HI, I want to explore Chromium's auto-testing mechanism, include: auto- test, auto collect performance data. Can you provide me some document about it? thanks! --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: if you get gcl error ['svn', 'cat', somefile]
The snips were two different files 'cause the actual output was about two different files :) The presubmit check doesn't complain about line endings with spaces for merge_and_sync.html So from cpu's original email in this thread (I just searched my mail for the error), I thought it had to do with inconsistent line endings :| Running 'svn revert merge_and_sync.html' does nothing. The file remains edited. I tried deleting every single line in the file, using the advanced-save option to set LF (which is what I've used for this sort of thing for years, so I'm pretty sure it's supposed to work) and svn diff correctly shows the removal, but svn revert still just does nothing :( On Wed, Oct 14, 2009 at 5:42 AM, Thomas Van Lenten thoma...@chromium.orgwrote: Not really sure since the snips you list are two different files, but the warning is not about eol-style, it's about lines ending with spaces, looks on the lines listed for things like: [space][space][real_code][space][space][LF] [space][space][LF] And take the spaces at the *end* of those lines out. TVL On Wed, Oct 14, 2009 at 5:13 AM, Tim Steele t...@chromium.org wrote: I'm getting such an error message for 'chrome/browser/sync/resources/merge_and_sync.html' come up after the presubmit checks have run (output shows snip ** Presubmit Warnings ** Found line ending with white spaces in ... chrome\browser\sync\resources\gaia_login.html, line 353 Presubmit checks took 2.2s to calculate. There were presubmit warnings. Are you sure you wish to continue? (y/N): y Got error status from ['svn', 'cat', 'chrome/browser/sync/resources/merge_and_sync.html']: snip svn:eol-style is set to LF for the file, and I went to 'Advanced Save Options' in visual studio, selected LF, and saved, and I still get this error :( Is there something else I can try? On Wed, Jun 10, 2009 at 9:59 AM, Marc-Antoine Ruel mar...@chromium.orgwrote: No, Marc-Antoine will enable a presubmit check today (tentative) so it won't happen anymore. And if you are a git user, ya better not break anything. M-A On Wed, Jun 10, 2009 at 12:32 PM, cpuc...@chromium.org wrote: I spent a ton of time tracking this so for the record: If you made a change to some files and gcl change works but when you try to do gcl upload you get this error: Got error status from ['svn', 'cat', 'somefile'] // Copyright (c) 2006-2009 The Chromim authors .. partial dump of the file here Then there are two likely possibilities: 1) the file somefile has inconsistent line endings in your copy 2) the file somefile has a problem as it exists in the svn server For #1 you can fix in your copy (not my case), for #2 you need to contact Marc-Antoine :) Actually Marc-Antoine will post the magic trick here so he would not get pestered again. --~--~-~--~~~---~--~~ 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: [LTTF][WebKit Gardening]: Keeping up with the weeds.
+1 On Tue, Oct 13, 2009 at 10:20 PM, Pam Greene p...@chromium.org wrote: If there are areas that nobody knows anything about, that's a lack that's hobbling us. Suppose we take the entire list of directories, slap it into a doc, and assign at least one owner to everything. For the areas that don't yet have anyone knowledgeable, we take volunteers to become knowledgeable if needed. It will be a valuable investment. Tests often fail due to problems outside their nominal tested areas, but the area owner would still be better than an arbitrary gardener at recognizing that and reassigning the bug. - Pam On Tue, Oct 13, 2009 at 10:09 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Ownership is a great concept. I started out planning LTTF as ownership-based. Unfortunately, the types of failures are scattered far and wide across the directories, some clustered, some not. After a few initial passes, I walked away thinking that it's not as simple as drawing the lines and basically gave up. That's how Finders/Fixers idea was born. :DG On Tue, Oct 13, 2009 at 4:24 PM, Yaar Schnitman y...@chromium.org wrote: I think ownership might actually help with flakiness. Today, in order to distinguish flakiness from real bugs, the gardener needs to have intimate knowledge of the relevant part of the code base and its history. That is beyond the capabilities of the average webkit gardener. Now, imagine a world were every layout test has an owner who can decide intelligently that the bug is flakey and advise the gardener what to do with it. Wouldn't it make gardening much easier? [Flakiness dashboard is very helpful in making the decision, but specialized knowledge topples generic statistics, especially if a test just started flaking] On Tue, Oct 13, 2009 at 1:21 PM, Julie Parent jpar...@chromium.org wrote: I like the idea of ownership of groups of layout tests. Maybe these directory owners could be more like the finders? An owner shouldn't have to necessarily fix everything in a group/directory, but they should be responsible for triaging and getting meaningful bugs filled for them, to keep things moving along. (I volunteer for editing/) Another complicating factor - The state of the main Chrome tree has a lot of effect on the gardener. If the tree is already filled with flakiness, then the webkit roll is likely to show failures, which may or may not have been there before the roll. This was largely the case in the situation pkasting was referring to, when he took over as sheriff, he inherited a tree with a lot of flakiness not reflected in test_expectations/disabled ui tests. I think very few (if any) of the tests he added to test_expectations had anything to do with the roll. Any policy we make needs to keep in mind that main tree sheriffs deal with flakiness differently; some cross their fingers and hope it goes away, and some do clean up. Maybe we need to get better at enforcing (or automating) adding flaky tests to expectations, so we at least have a clean slate for gardeners to start with. On Tue, Oct 13, 2009 at 11:53 AM, Stephen White senorbla...@chromium.org wrote: I agree with Dimitri that we're fighting a losing battle here. In my last stint as gardener, I did informally what I proposed formally last time: I spent basically 1 full day just triaging failures from my 2 days gardening. Not fixing, but just running tests locally, analyzing, grouping, creating bugs, assigning to appropriate people (when I knew who they were, cc'ing poor dglazkov when I didn't). So at least I didn't leave a monster bug with layout tests broken by merge #foo but at least grouped by area. That was manageable, but I don't know if another day would actually be enough for a meaningful amount of fixing. I also agree with Drew that actively fixing all the broken tests is usually beyond the skills of any one gardener. Perhaps we should start taking ownership of particular groups of layout tests? And maybe automatically assign them (or least cc them), the same way Area-Foo causes automatic cc'ing in bugs.chromium.org (I think?) That way, the gardener wouldn't have to know who to assign to. I've basically taken responsibility for fixing all layout tests broken by Skia rolls, which can pretty heavy on its own, but I'm willing to take ownership of a directory or two. BTW, the layout test flakiness dashboard has become an invaluable tool for analyzing failures: searching for a test by name is lightning-fast, and you can clearly see if a test has become flaky, on which platforms, and which WebKit merge was responsible, which can also help with grouping. (Props to Ojan for that). Also, it may be Gilbert-and-Sullivan-esque of me, but I think everyone who contributes patches to WebKit for chromium should be on the WebKit gardener rotation.
[chromium-dev] Re: my feelings on rebuilding v8bindings
+123.45e100! On Wed, Oct 14, 2009 at 10:46 AM, Elliot Glaysher (Chromium) e...@chromium.org wrote: This is the best rage comic ever. -- Elliot On Wed, Oct 14, 2009 at 10:40 AM, Evan Martin e...@chromium.org wrote: See attachment. That is all. --~--~-~--~~~---~--~~ 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: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
(chromium-dev-izing thread) Are we still a month from having the platforms build? If we aren't do we have another way to disable a feature per platform, or just command line flags? I'm cool with ENABLE_BROWSER_SYNC though. On Wed, Oct 14, 2009 at 12:13 PM, Nick Carter ncar...@google.com wrote: The #define CHROME_PERSONALIZATION is the last remaining vestige of sync obfuscation. We will need a feature define until sync is enabled by default for all builds (mac, linux, chromeos). That's a month or more. In the meantime, CHROME_PERSONALIZATION is an unintuitive name. I propose changing it to ENABLE_BROWSER_SYNC. Any objections or counter-proposals? - nick -- You received this message because you are subscribed to the Google Groups chrome-sync-dev group. To post to this group, send email to chrome-sync-...@google.com. To unsubscribe from this group, send email to chrome-sync-dev+unsubscr...@google.comchrome-sync-dev%2bunsubscr...@google.com . For more options, visit this group at http://groups.google.com/a/google.com/group/chrome-sync-dev?hl=en. --~--~-~--~~~---~--~~ 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: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
I like ENABLE_BROWSER_SYNC too. -Munjal On Wed, Oct 14, 2009 at 12:17 PM, Tim Steele t...@chromium.org wrote: (chromium-dev-izing thread) Are we still a month from having the platforms build? If we aren't do we have another way to disable a feature per platform, or just command line flags? I'm cool with ENABLE_BROWSER_SYNC though. On Wed, Oct 14, 2009 at 12:13 PM, Nick Carter ncar...@google.com wrote: The #define CHROME_PERSONALIZATION is the last remaining vestige of sync obfuscation. We will need a feature define until sync is enabled by default for all builds (mac, linux, chromeos). That's a month or more. In the meantime, CHROME_PERSONALIZATION is an unintuitive name. I propose changing it to ENABLE_BROWSER_SYNC. Any objections or counter-proposals? - nick -- You received this message because you are subscribed to the Google Groups chrome-sync-dev group. To post to this group, send email to chrome-sync-...@google.com. To unsubscribe from this group, send email to chrome-sync-dev+unsubscr...@google.comchrome-sync-dev%2bunsubscr...@google.com . For more options, visit this group at http://groups.google.com/a/google.com/group/chrome-sync-dev?hl=en. --~--~-~--~~~---~--~~ 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: [LTTF][WebKit Gardening]: Keeping up with the weeds.
Ok, I have lousy docs-sharing-fu, but I gave it a shot. This should be writeable by anyone @chromium.org: http://spreadsheets.google.com/a/chromium.org/ccc?key=0AmBv0BNymMh5dHlHZnJDSjR1X0ZzNnRYOFZTVWtvb0Ehl=en (To edit, you'll probably have to log into http://docs.google.com/a/chromium.org first, then click the link above.). I basically split up the big directories, and the ones which obviously contained more than one type of test. Feel free to split/merge at will. It's probably a good idea that each directory has more than one expert/owner, so don't be shy. Stephen On Wed, Oct 14, 2009 at 2:43 PM, Dirk Pranke dpra...@chromium.org wrote: +1 On Tue, Oct 13, 2009 at 10:20 PM, Pam Greene p...@chromium.org wrote: If there are areas that nobody knows anything about, that's a lack that's hobbling us. Suppose we take the entire list of directories, slap it into a doc, and assign at least one owner to everything. For the areas that don't yet have anyone knowledgeable, we take volunteers to become knowledgeable if needed. It will be a valuable investment. Tests often fail due to problems outside their nominal tested areas, but the area owner would still be better than an arbitrary gardener at recognizing that and reassigning the bug. - Pam On Tue, Oct 13, 2009 at 10:09 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Ownership is a great concept. I started out planning LTTF as ownership-based. Unfortunately, the types of failures are scattered far and wide across the directories, some clustered, some not. After a few initial passes, I walked away thinking that it's not as simple as drawing the lines and basically gave up. That's how Finders/Fixers idea was born. :DG On Tue, Oct 13, 2009 at 4:24 PM, Yaar Schnitman y...@chromium.org wrote: I think ownership might actually help with flakiness. Today, in order to distinguish flakiness from real bugs, the gardener needs to have intimate knowledge of the relevant part of the code base and its history. That is beyond the capabilities of the average webkit gardener. Now, imagine a world were every layout test has an owner who can decide intelligently that the bug is flakey and advise the gardener what to do with it. Wouldn't it make gardening much easier? [Flakiness dashboard is very helpful in making the decision, but specialized knowledge topples generic statistics, especially if a test just started flaking] On Tue, Oct 13, 2009 at 1:21 PM, Julie Parent jpar...@chromium.org wrote: I like the idea of ownership of groups of layout tests. Maybe these directory owners could be more like the finders? An owner shouldn't have to necessarily fix everything in a group/directory, but they should be responsible for triaging and getting meaningful bugs filled for them, to keep things moving along. (I volunteer for editing/) Another complicating factor - The state of the main Chrome tree has a lot of effect on the gardener. If the tree is already filled with flakiness, then the webkit roll is likely to show failures, which may or may not have been there before the roll. This was largely the case in the situation pkasting was referring to, when he took over as sheriff, he inherited a tree with a lot of flakiness not reflected in test_expectations/disabled ui tests. I think very few (if any) of the tests he added to test_expectations had anything to do with the roll. Any policy we make needs to keep in mind that main tree sheriffs deal with flakiness differently; some cross their fingers and hope it goes away, and some do clean up. Maybe we need to get better at enforcing (or automating) adding flaky tests to expectations, so we at least have a clean slate for gardeners to start with. On Tue, Oct 13, 2009 at 11:53 AM, Stephen White senorbla...@chromium.org wrote: I agree with Dimitri that we're fighting a losing battle here. In my last stint as gardener, I did informally what I proposed formally last time: I spent basically 1 full day just triaging failures from my 2 days gardening. Not fixing, but just running tests locally, analyzing, grouping, creating bugs, assigning to appropriate people (when I knew who they were, cc'ing poor dglazkov when I didn't). So at least I didn't leave a monster bug with layout tests broken by merge #foo but at least grouped by area. That was manageable, but I don't know if another day would actually be enough for a meaningful amount of fixing. I also agree with Drew that actively fixing all the broken tests is usually beyond the skills of any one gardener. Perhaps we should start taking ownership of particular groups of layout tests? And maybe automatically assign them (or least cc them), the same way Area-Foo causes automatic cc'ing in bugs.chromium.org (I
[chromium-dev] Re: Proposal: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
Why the ENABLE? Why not BROWSER_SYNC? -Scott On Wed, Oct 14, 2009 at 12:23 PM, Munjal Doshi mun...@chromium.org wrote: I like ENABLE_BROWSER_SYNC too. -Munjal On Wed, Oct 14, 2009 at 12:17 PM, Tim Steele t...@chromium.org wrote: (chromium-dev-izing thread) Are we still a month from having the platforms build? If we aren't do we have another way to disable a feature per platform, or just command line flags? I'm cool with ENABLE_BROWSER_SYNC though. On Wed, Oct 14, 2009 at 12:13 PM, Nick Carter ncar...@google.com wrote: The #define CHROME_PERSONALIZATION is the last remaining vestige of sync obfuscation. We will need a feature define until sync is enabled by default for all builds (mac, linux, chromeos). That's a month or more. In the meantime, CHROME_PERSONALIZATION is an unintuitive name. I propose changing it to ENABLE_BROWSER_SYNC. Any objections or counter-proposals? - nick -- You received this message because you are subscribed to the Google Groups chrome-sync-dev group. To post to this group, send email to chrome-sync-...@google.com. To unsubscribe from this group, send email to chrome-sync-dev+unsubscr...@google.com. For more options, visit this group at http://groups.google.com/a/google.com/group/chrome-sync-dev?hl=en. --~--~-~--~~~---~--~~ 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: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
BROWSER_SYNC++ I always thought PERSONALIZATION was weird. - Mohamed Mansour On Wed, Oct 14, 2009 at 3:33 PM, Scott Violet s...@chromium.org wrote: Why the ENABLE? Why not BROWSER_SYNC? -Scott On Wed, Oct 14, 2009 at 12:23 PM, Munjal Doshi mun...@chromium.org wrote: I like ENABLE_BROWSER_SYNC too. -Munjal On Wed, Oct 14, 2009 at 12:17 PM, Tim Steele t...@chromium.org wrote: (chromium-dev-izing thread) Are we still a month from having the platforms build? If we aren't do we have another way to disable a feature per platform, or just command line flags? I'm cool with ENABLE_BROWSER_SYNC though. On Wed, Oct 14, 2009 at 12:13 PM, Nick Carter ncar...@google.com wrote: The #define CHROME_PERSONALIZATION is the last remaining vestige of sync obfuscation. We will need a feature define until sync is enabled by default for all builds (mac, linux, chromeos). That's a month or more. In the meantime, CHROME_PERSONALIZATION is an unintuitive name. I propose changing it to ENABLE_BROWSER_SYNC. Any objections or counter-proposals? - nick -- You received this message because you are subscribed to the Google Groups chrome-sync-dev group. To post to this group, send email to chrome-sync-...@google.com. To unsubscribe from this group, send email to chrome-sync-dev+unsubscr...@google.comchrome-sync-dev%2bunsubscr...@google.com . For more options, visit this group at http://groups.google.com/a/google.com/group/chrome-sync-dev?hl=en. --~--~-~--~~~---~--~~ 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: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
I always thought PERSONALIZATION was weird. I believe that was the point. --~--~-~--~~~---~--~~ 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: [LTTF][WebKit Gardening]: Keeping up with the weeds.
Excellent, thanks Stephen. I'm sure not all the slots will be filled. I'd say the best way to complete the list is lazily, in the software sense -- that is, when a test fails, if it has no owner, find the best person (or a random volunteer, if nobody has any knowledge about the topic) and then add them to the list. - Pam On Wed, Oct 14, 2009 at 12:24 PM, Stephen White senorbla...@chromium.orgwrote: Ok, I have lousy docs-sharing-fu, but I gave it a shot. This should be writeable by anyone @chromium.org: http://spreadsheets.google.com/a/chromium.org/ccc?key=0AmBv0BNymMh5dHlHZnJDSjR1X0ZzNnRYOFZTVWtvb0Ehl=en (To edit, you'll probably have to log into http://docs.google.com/a/chromium.org first, then click the link above.). I basically split up the big directories, and the ones which obviously contained more than one type of test. Feel free to split/merge at will. It's probably a good idea that each directory has more than one expert/owner, so don't be shy. Stephen On Wed, Oct 14, 2009 at 2:43 PM, Dirk Pranke dpra...@chromium.org wrote: +1 On Tue, Oct 13, 2009 at 10:20 PM, Pam Greene p...@chromium.org wrote: If there are areas that nobody knows anything about, that's a lack that's hobbling us. Suppose we take the entire list of directories, slap it into a doc, and assign at least one owner to everything. For the areas that don't yet have anyone knowledgeable, we take volunteers to become knowledgeable if needed. It will be a valuable investment. Tests often fail due to problems outside their nominal tested areas, but the area owner would still be better than an arbitrary gardener at recognizing that and reassigning the bug. - Pam On Tue, Oct 13, 2009 at 10:09 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Ownership is a great concept. I started out planning LTTF as ownership-based. Unfortunately, the types of failures are scattered far and wide across the directories, some clustered, some not. After a few initial passes, I walked away thinking that it's not as simple as drawing the lines and basically gave up. That's how Finders/Fixers idea was born. :DG On Tue, Oct 13, 2009 at 4:24 PM, Yaar Schnitman y...@chromium.org wrote: I think ownership might actually help with flakiness. Today, in order to distinguish flakiness from real bugs, the gardener needs to have intimate knowledge of the relevant part of the code base and its history. That is beyond the capabilities of the average webkit gardener. Now, imagine a world were every layout test has an owner who can decide intelligently that the bug is flakey and advise the gardener what to do with it. Wouldn't it make gardening much easier? [Flakiness dashboard is very helpful in making the decision, but specialized knowledge topples generic statistics, especially if a test just started flaking] On Tue, Oct 13, 2009 at 1:21 PM, Julie Parent jpar...@chromium.org wrote: I like the idea of ownership of groups of layout tests. Maybe these directory owners could be more like the finders? An owner shouldn't have to necessarily fix everything in a group/directory, but they should be responsible for triaging and getting meaningful bugs filled for them, to keep things moving along. (I volunteer for editing/) Another complicating factor - The state of the main Chrome tree has a lot of effect on the gardener. If the tree is already filled with flakiness, then the webkit roll is likely to show failures, which may or may not have been there before the roll. This was largely the case in the situation pkasting was referring to, when he took over as sheriff, he inherited a tree with a lot of flakiness not reflected in test_expectations/disabled ui tests. I think very few (if any) of the tests he added to test_expectations had anything to do with the roll. Any policy we make needs to keep in mind that main tree sheriffs deal with flakiness differently; some cross their fingers and hope it goes away, and some do clean up. Maybe we need to get better at enforcing (or automating) adding flaky tests to expectations, so we at least have a clean slate for gardeners to start with. On Tue, Oct 13, 2009 at 11:53 AM, Stephen White senorbla...@chromium.org wrote: I agree with Dimitri that we're fighting a losing battle here. In my last stint as gardener, I did informally what I proposed formally last time: I spent basically 1 full day just triaging failures from my 2 days gardening. Not fixing, but just running tests locally, analyzing, grouping, creating bugs, assigning to appropriate people (when I knew who they were, cc'ing poor dglazkov when I didn't). So at least I didn't leave a monster bug with layout tests broken by merge #foo but at least grouped by area. That was manageable, but I don't
[chromium-dev] Is it time to create a SecurityOrigin class in Chromium?
Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
+1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. If we have one circular dependency on base, why not add more? Anyhow, they're already #if'ed. So if someone wanted to use the API without base, they easily could change that #else to a #elif, right? Maybe we should just do that now? I agree the circular dependencies are bad, but they're already there. And, honestly, the WebKit API (in its current form) is not useful unless you are including base. Whatsmore, if we output stuff as a string, then we're just hoping someone goes ahead and immediately converts that to a SecurityOrigin. But there's no guarantee they won't. Or that they won't do something incredibly stupid before such a conversion. By forcing things to go straight to a base::SecurityOrigin (and comments in that code explaining why) it'll be much harder for someone to do something dangerous without doing it willfully. Personally, I think we should have more functions for converting to base:: types rather than less. -Darin Does this sound like a good plan? J --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe:
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. If we have one circular dependency on base, why not add more? Because they can be a source of great pain. This is a slippery slope. We can basically never change base/string16.h or base/nullable_string16.h. I don't want more of that. Things like ChromiumBridge exist so we can avoid having more of these. Anyhow, they're already #if'ed. So if someone wanted to use the API without base, they easily could change that #else to a #elif, right? Maybe we should just do that now? Right, that was my intent. They are currently defined when WEBKIT_IMPLEMENTATION is not defined, but we should probably make consumers opt-in by defining something explicit. I agree the circular dependencies are bad, but they're already there. And, honestly, the WebKit API (in its current form) is not useful unless you are including base. I disagree. Whatsmore, if we output stuff as a string, then we're just hoping someone goes ahead and
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi needs to form file path elements, yes. Dumi also needs to store a canonical string representation of an 'origin' in the tracker database which will equate to the canonical string represetation 6 months from now (either that or upgrade the column values whenever that representation changes). Q: What is the canonical string representation used in the localstorage.db which has the similar requirement to track things per origin? Probably WebCore::SecurityOrigin::toString(), is that right? Those two things probably shouldn't be confounded. At some point in the not too distant future, we'll need to interrogate from a ChromeUI database, localstorage, appcache, and (filesystem) for what 'origins' are making how heavy a use of those systems. An important point is that code today is writing string values, and code 6 months from now has to interpret those values and match against them. ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. As mentioned f2f, this falls apart as soon as Chrome tries to manufacture a security origin. I'm not sure, may already have instances of that in the code base for all I know. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. -Darin Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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: my feelings on rebuilding v8bindings
+1. is this fixable? what would one need to do to fix this? On Wed, Oct 14, 2009 at 12:01 PM, Tim Steele t...@chromium.org wrote: +123.45e100! On Wed, Oct 14, 2009 at 10:46 AM, Elliot Glaysher (Chromium) e...@chromium.org wrote: This is the best rage comic ever. -- Elliot On Wed, Oct 14, 2009 at 10:40 AM, Evan Martin e...@chromium.org wrote: See attachment. That is all. --~--~-~--~~~---~--~~ 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: rename CHROME_PERSONALIZATION to ENABLE_BROWSER_SYNC
Without objection, I'll move forward with renaming to BROWSER_SYNC. - nick On Wed, Oct 14, 2009 at 1:10 PM, Evan Stade est...@chromium.org wrote: I always thought PERSONALIZATION was weird. I believe that was the point. --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:48 PM, Michael Nordman micha...@google.com wrote: As mentioned f2f, this falls apart as soon as Chrome tries to manufacture a security origin. I'm not sure, may already have instances of that in the code base for all I know. I'm not sure Chrome is smart enough to manufacture a SecurityOrigin. There's a lot of tricky work in the canonicalization that we don't want to duplicate. Adam --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 3:50 PM, Adam Barth aba...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:48 PM, Michael Nordman micha...@google.com wrote: As mentioned f2f, this falls apart as soon as Chrome tries to manufacture a security origin. I'm not sure, may already have instances of that in the code base for all I know. I'm not sure Chrome is smart enough to manufacture a SecurityOrigin. There's a lot of tricky work in the canonicalization that we don't want to duplicate. I agree. I think the solution is to (if/when we actually need this!) add a hook to the WebKit API to ask WebCore to manufacture one for us. --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 we decided to use GURLs instead of string16s to represent SecurityOrigins on the chromium side, so we don't need a (Web)SecurityOrigin::toFilePath()-like method anymore; we can just do GURL(SecurityOrigin::toString()), and then create a file path from GURL::scheme(), GURL::host() and GURL::port(). also, i'd argue that no class representing an origin should have a toFilePath()-like method: origins and file paths have nothing in common; using the origin URL as part of the DB file name is a database-specific decision and the code for that conversion should be kept in some database-specific class, or a separate origin_to_file_path_util.h file at best. (It was very tempting to use SecurityOrigin::databaseIdentifier() only because that method was already there.) --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] layout test dashboard goofup
For those of you who rely on the layout test dashboard for identifying flakiness, my apologies. I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. We'll just need to wait for enough runs for it to be reliable again with respect to flakiness monitoring. Ojan --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. If we have one circular dependency on base, why not add more? Because they can be a source of great pain. This is a slippery slope. We can basically never change base/string16.h or base/nullable_string16.h. I don't want more of that. Things like ChromiumBridge exist so we can avoid having more of these. If we're making a DLL out of WebKit, then you're right. Every time we changed base, we'd need to take extra care to make sure base is rolled properly. How are we going to provide developers such a DLL, though? If it's checked in, then whenever someone changes base they can check in a new copy of WebKit.dll. And, if we do things some other way, I'm sure we can find other reasonable solutions. Anyhow, they're already #if'ed. So if someone wanted to use the API without base, they easily could change that #else to a #elif, right? Maybe we
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 3:58 PM, Dumitru Daniliuc d...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 we decided to use GURLs instead of string16s to represent SecurityOrigins on the chromium side, so we don't need a (Web)SecurityOrigin::toFilePath()-like method anymore; we can just do GURL(SecurityOrigin::toString()), and then create a file path from GURL::scheme(), GURL::host() and GURL::port(). The point of this thread is that we should not be converting SecurityOrigins of GURLs. I believe michaeln was the primary proponent of this and I believe we convinced him that we shouldn't be doing it. And I believe most if not all the reasons why were in my original email. (Michael, correct me if I'm wrong.) also, i'd argue that no class representing an origin should have a toFilePath()-like method: origins and file paths have nothing in common; using the origin URL as part of the DB file name is a database-specific decision and the code for that conversion should be kept in some database-specific class, or a separate origin_to_file_path_util.h file at best. (It was very tempting to use SecurityOrigin::databaseIdentifier() only because that method was already there.) Well, SecurityOrigin has a method that creates a database file name. I don't see why we can't have a ::databasePath method of our own. And if we do, then it does make sense for it to return a FilePath. That said, I think what Darin suggested in the code review is actually the cleanest way to do it. And I think returning a String is not a big deal. --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 2:48 PM, Michael Nordman micha...@google.comwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.comwrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi needs to form file path elements, yes. Dumi also needs to store a canonical string representation of an 'origin' in the tracker database which will equate to the canonical string represetation 6 months from now (either that or upgrade the column values whenever that representation changes). Q: What is the canonical string representation used in the localstorage.db which has the similar requirement to track things per origin? Probably WebCore::SecurityOrigin::toString(), is that right? LocalStorage uses SecurityOrigin::databaseIdentifier() from within WebCore. Those two things probably shouldn't be confounded. At some point in the not too distant future, we'll need to interrogate from a ChromeUI database, localstorage, appcache, and (filesystem) for what 'origins' are making how heavy a use of those systems. We can add these methods as necessary. We may need to add the methods to the WebSecurityOrigin since base::SecurityOrigin will be dumb. An important point is that code today is writing string values, and code 6 months from now has to interpret those values and match against them. ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. As mentioned f2f, this falls apart as soon as Chrome tries to manufacture a security origin. I'm not sure, may already have instances of that in the code base for all I know. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). Does this sound like a good plan? J --~--~-~--~~~---~--~~ 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 dashboard goofup
On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: layout test dashboard goofup
I assume we're going to start backing this data up from now on? On Wed, Oct 14, 2009 at 4:13 PM, Peter Kasting pkast...@google.com wrote: On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: layout test dashboard goofup
Sounds like we've got a volunteer! :D :D :D On Wed, Oct 14, 2009 at 4:15 PM, Jeremy Orlow jor...@chromium.org wrote: I assume we're going to start backing this data up from now on? On Wed, Oct 14, 2009 at 4:13 PM, Peter Kasting pkast...@google.com wrote: On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: layout test dashboard goofup
I haven't actually gotten anything done on LocalStorage this week because I've been doing so many small side projects like this.but if it's a priority, sure. How about a cron job on some machine that ssh's via a cert into whatever machines the data is stored on, pulls it back, and dumps it into some dir? When we start filling up the hard drive, we can look at doing something smarter, deleting old data, or putting it somewhere like GFS. What server can I use and where's the data stored? On Wed, Oct 14, 2009 at 4:17 PM, Evan Martin e...@chromium.org wrote: Sounds like we've got a volunteer! :D :D :D On Wed, Oct 14, 2009 at 4:15 PM, Jeremy Orlow jor...@chromium.org wrote: I assume we're going to start backing this data up from now on? On Wed, Oct 14, 2009 at 4:13 PM, Peter Kasting pkast...@google.com wrote: On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 4:03 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 3:58 PM, Dumitru Daniliuc d...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 we decided to use GURLs instead of string16s to represent SecurityOrigins on the chromium side, so we don't need a (Web)SecurityOrigin::toFilePath()-like method anymore; we can just do GURL(SecurityOrigin::toString()), and then create a file path from GURL::scheme(), GURL::host() and GURL::port(). The point of this thread is that we should not be converting SecurityOrigins of GURLs. I believe michaeln was the primary proponent of this and I believe we convinced him that we shouldn't be doing it. And I believe most if not all the reasons why were in my original email. (Michael, correct me if I'm wrong.) I think i have two primary concerns. 1) What is the format of the data written to disk that we need to support going forward since it is on disk. We need a decision that we can stick with. 2) What measures are in place to ensure that we're actually persisting data in that prescribed format today. Having 'strings' float around makes me uneasy on that second point. If chrome can't validate these string values are in the prescribed format (its not smart enough to reason about them), how can we assert we've got it right (inspection doesn't work well... working backwards from a callsite in chrome browser code storing an 'origin' string to the source of that string being produced is just not practical). Given the current state of affairs, i still think GURL is a better option. Given a GURL, we can reason about it (produce path elements, produce a canonical string representation, test if another GURL falls in that origin or not (we do that in appcache code sans webkit)). The null security origin is an odd corner case that gives the GURL representation grief. also, i'd argue that no class representing an origin should have a toFilePath()-like method: origins and file paths have nothing in common; using the origin URL as part of the DB file name is a database-specific decision and the code for that conversion should be kept in some database-specific class, or a separate origin_to_file_path_util.h file at best. (It was very tempting to use SecurityOrigin::databaseIdentifier() only because that method was already there.) Well, SecurityOrigin has a method that creates a database file name. I don't see why we can't have a ::databasePath method of our own. And if we do, then it does make sense for it to return a FilePath. That said, I think what Darin suggested in the code review is actually the cleanest way to do it. And I think returning a String is not a big deal. --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. If we have one circular dependency on base, why not add more? Because they can be a source of great pain. This is a slippery slope. We can basically never change base/string16.h or base/nullable_string16.h. I don't want more of that. Things like ChromiumBridge exist so we can avoid having more of these. If we're making a DLL out of WebKit, then you're right. We are (for debug builds). That has always been the plan :-) But this is not the reason. Every time we changed base, we'd need to take extra care to make sure base is rolled properly. What does rolling base mean? base is part of chromium. chromium depends on webkit. See what I mean? base is not a separately versioned module. How are we going to provide developers such a DLL, though? If it's
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 4:32 PM, Michael Nordman micha...@google.comwrote: On Wed, Oct 14, 2009 at 4:03 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 3:58 PM, Dumitru Daniliuc d...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 we decided to use GURLs instead of string16s to represent SecurityOrigins on the chromium side, so we don't need a (Web)SecurityOrigin::toFilePath()-like method anymore; we can just do GURL(SecurityOrigin::toString()), and then create a file path from GURL::scheme(), GURL::host() and GURL::port(). The point of this thread is that we should not be converting SecurityOrigins of GURLs. I believe michaeln was the primary proponent of this and I believe we convinced him that we shouldn't be doing it. And I believe most if not all the reasons why were in my original email. (Michael, correct me if I'm wrong.) I think i have two primary concerns. 1) What is the format of the data written to disk that we need to support going forward since it is on disk. We need a decision that we can stick with. 2) What measures are in place to ensure that we're actually persisting data in that prescribed format today. Having 'strings' float around makes me uneasy on that second point. If chrome can't validate these string values are in the prescribed format (its not smart enough to reason about them), how can we assert we've got it right (inspection doesn't work well... working backwards from a callsite in chrome browser code storing an 'origin' string to the source of that string being produced is just not practical). Given the current state of affairs, i still think GURL is a better option. Given a GURL, we can reason about it (produce path elements, produce a canonical string representation, test if another GURL falls in that origin or not (we do that in appcache code sans webkit)). The null security origin is an odd corner case that gives the GURL representation grief. We don't need to reason about it. WebCore::SecurityOrigin can do it for us. As Adam said, duplicating this kind of code in Chromium is not really useful. Let's let WebCore take care of it for us. also, i'd argue that no class representing an origin should have a toFilePath()-like method: origins and file paths have nothing in common; using the origin URL as part of the DB file name is a database-specific decision and the code for that conversion should be kept in some database-specific class, or a separate origin_to_file_path_util.h file at best. (It was very tempting to use SecurityOrigin::databaseIdentifier() only because that method was already there.) Well, SecurityOrigin has a method that creates a database file name. I don't see why we can't have a ::databasePath method of our own. And if we do, then it does make sense for it to return a FilePath. That said, I think what Darin suggested in the code review is actually the cleanest way to do it. And I think returning a String is not a big deal. --~--~-~--~~~---~--~~ 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 it time to create a SecurityOrigin class in Chromium?
If this is the case, then I don't think a SecurityOrigin wrapper buys us anything. Never mind. On Wed, Oct 14, 2009 at 4:37 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the respective repositories relate). Circular dependencies are evil. We have them for string16 and NullableString16. Let's not add more. If we have one circular dependency on base, why not add more? Because they can be a source of great pain. This is a slippery slope. We can basically never change base/string16.h or base/nullable_string16.h. I don't want more of that. Things like ChromiumBridge exist so we can avoid having more of these. If we're making a DLL out of WebKit, then you're right. We are (for debug builds). That has always been the plan :-) But this is not the reason. Every time we changed base, we'd need to take extra care to make sure base is rolled properly. What does rolling base mean? base
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 3:50 PM, Adam Barth aba...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:48 PM, Michael Nordman micha...@google.com wrote: As mentioned f2f, this falls apart as soon as Chrome tries to manufacture a security origin. I'm not sure, may already have instances of that in the code base for all I know. I'm not sure Chrome is smart enough to manufacture a SecurityOrigin. There's a lot of tricky work in the canonicalization that we don't want to duplicate. Adam Agreed, and we shouldn't be in that business. I think for all our use cases, the factory for security origins can be WebCore. Chrome just needs to be able to serialize / de-serialize a security origin, compare them, and possibly access some component parts (though I'm not certain of this requirement). -Darin --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
Hmm... it seems useful as a means of making the code more self-documenting anda bit safer. I'd rather not have people passing strings for origins since they might be tempted to parse the strings. The momentary translation to strings on the boundary of the WebKit API does not completely make this fragile. If the end-points in Chrome use SecurityOrigin and the end-points in the WebKit API use WebSecurityOrigin, then developers will be naturally inclined to convert between SecurityOrigin and WebSecurityOrigin, ignoring the toString() getter on WebSecurityOrigin. This is a case where existing code should help guide people in the right direction. -Darin On Wed, Oct 14, 2009 at 4:39 PM, Jeremy Orlow jor...@chromium.org wrote: If this is the case, then I don't think a SecurityOrigin wrapper buys us anything. Never mind. On Wed, Oct 14, 2009 at 4:37 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.orgwrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary). Finally, I'll remove WebSecurityOrigin::toString(). As I mentioned in person, I'm not happy having WebKit API depend on base for too many things. I would prefer to not introduce this dependency since it is a circular dependency (in the way the
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
To be clear: I only weakly advocate Chrome having a SecurityOrigin. I'm also OKwith just using std::string. I think either is better than using GURL. -Darin On Wed, Oct 14, 2009 at 4:45 PM, Darin Fisher da...@chromium.org wrote: Hmm... it seems useful as a means of making the code more self-documenting anda bit safer. I'd rather not have people passing strings for origins since they might be tempted to parse the strings. The momentary translation to strings on the boundary of the WebKit API does not completely make this fragile. If the end-points in Chrome use SecurityOrigin and the end-points in the WebKit API use WebSecurityOrigin, then developers will be naturally inclined to convert between SecurityOrigin and WebSecurityOrigin, ignoring the toString() getter on WebSecurityOrigin. This is a case where existing code should help guide people in the right direction. -Darin On Wed, Oct 14, 2009 at 4:39 PM, Jeremy Orlow jor...@chromium.org wrote: If this is the case, then I don't think a SecurityOrigin wrapper buys us anything. Never mind. On Wed, Oct 14, 2009 at 4:37 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method canonicalizes it. If, in the future, we need to do anything more with SecurityOrigins (besides transporting them, testing equality, and using them in sets/maps) then we can make the class more complex. Why not use GURL? For one, the SecurityOrigin has a null state which is significant and which isn't represented in GURL. In addition, there's a lot of operations you can do with a GURL which don't really make sense in the context of a SecurityOrigin. Passing around a SecurityOrigin object is also much more self-documenting. But, the fact that GURL looks like a tempting way to store a SecurityOrigin is actually one of the biggest reasons why I think we should make a dedicated type. If people agree with this, my plan is to create such a type in base and modify WebKit::WebSecurityOrigin to do conversions to base::SecurityOrigin. I'll then convert everything over (or ask people to do the conversion if it looks scary).
[chromium-dev] Re: layout test dashboard goofup
The data is stored in a single file per bot. For example, the webkit release bot's results are at http://build.chromium.org/buildbot/layout_test_results/webkit-rel/results.json. That file holds all the historical data for that bot and is copied over during the archive step of each run. We intentionally limit the number of results we keep in that file to 750 runs to keep filesize down. In my accidental checking, I changed 750 to 9. :( A trivial to implement backup would be to also copy the file to the archive location for just that run (same place as where we copy layout_test_results.zip), e.g. also copy it to http://build.chromium.org/buildbot/layout_test_results/webkit-rel/29056/. The downside is that this uses up disk space (e.g. the largest results.json file was 25mb before being clobbered). Another problem with backing up is that you'd also have to find a way to restore from backup that didn't lose data from runs that happened since the problem occurred. Merging the two files results.json should be pretty relatively trivial code, but it's all code that someone would need to write and test. While it sucks, I don't think backing up this data is worth the effort. It's a temporary productivity hit for the team, but we get enough new data to make reasonable decisions relatively quickly. Mistakes like this are very rare. It will become even more rare as coding work on the dashboard winds down. Feel free to have at it if you disagree. Creates the results.json file and it's content: trunk/src/webkit/tools/layout_tests/layout_package/json_results_generator.py Copies the results.json file to the right place: trunk/tools/buildbot/scripts/slave/chromium/archive_layout_test_results.py Ojan On Wed, Oct 14, 2009 at 4:24 PM, Jeremy Orlow jor...@chromium.org wrote: I haven't actually gotten anything done on LocalStorage this week because I've been doing so many small side projects like this.but if it's a priority, sure. How about a cron job on some machine that ssh's via a cert into whatever machines the data is stored on, pulls it back, and dumps it into some dir? When we start filling up the hard drive, we can look at doing something smarter, deleting old data, or putting it somewhere like GFS. What server can I use and where's the data stored? On Wed, Oct 14, 2009 at 4:17 PM, Evan Martin e...@chromium.org wrote: Sounds like we've got a volunteer! :D :D :D On Wed, Oct 14, 2009 at 4:15 PM, Jeremy Orlow jor...@chromium.org wrote: I assume we're going to start backing this data up from now on? On Wed, Oct 14, 2009 at 4:13 PM, Peter Kasting pkast...@google.com wrote: On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
On Wed, Oct 14, 2009 at 4:51 PM, Jeremy Orlow jor...@chromium.org wrote: You mean string16, right? I see instances of std::string and string16. I would be happiest if we unified on one. string16 is probably the path of least resistance. std::string has the benefit of being more compact, which for something like this which is really just a bag of bytes is probably a good thing. I really don't think it buys us much if it's purely optional that people put the security origin (in string representation) into a wrapper that then blocks them from doing anything silly with it. we could give it a ToString() method. i think the point of SecurityOrigin would be to guide people in the right direction. -darin More to the point, I don't think it's useful enough that I'm going to bother implementing it. If someone else wants to, I'd probably use it. On Wed, Oct 14, 2009 at 4:47 PM, Darin Fisher da...@chromium.org wrote: To be clear: I only weakly advocate Chrome having a SecurityOrigin. I'm also OKwith just using std::string. I think either is better than using GURL. -Darin On Wed, Oct 14, 2009 at 4:45 PM, Darin Fisher da...@chromium.org wrote: Hmm... it seems useful as a means of making the code more self-documenting anda bit safer. I'd rather not have people passing strings for origins since they might be tempted to parse the strings. The momentary translation to strings on the boundary of the WebKit API does not completely make this fragile. If the end-points in Chrome use SecurityOrigin and the end-points in the WebKit API use WebSecurityOrigin, then developers will be naturally inclined to convert between SecurityOrigin and WebSecurityOrigin, ignoring the toString() getter on WebSecurityOrigin. This is a case where existing code should help guide people in the right direction. -Darin On Wed, Oct 14, 2009 at 4:39 PM, Jeremy Orlow jor...@chromium.orgwrote: If this is the case, then I don't think a SecurityOrigin wrapper buys us anything. Never mind. On Wed, Oct 14, 2009 at 4:37 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the plan to do more than opaquely pass around and test webkit generated representations. Also, I think dumi has a use case to crack it open in order to form file path elements of the form 'scheme_host_port' Actually, Dumi's case is slightly different. He wants to get SecurityOrigin::databaseIdentifier, right? Maybe WebSecurityOrigin should have a databaseIdentifier() method that outputs a FilePath object? Dumi has such a method in a CL that he is working on at the moment. Also, note: we don't have a way to use FilePath from the WebKit API, and I'm not sure that we should. We use WebString for file paths in the WebKit API. So then he's adding such a method to WebSecurityOrigin that returns a string? If so, sounds good. What's the CL, btw? Yes: http://codereview.chromium.org/256073/diff/11001/11029 ... and why not use strings? * does the string contain a trailing slash, or not? * in the default port case, does the string contain the default port number or not? WebCore::SecurityOrigin handles these for us. I'll make it difficult for a base::SecurityOrigin to be constructed any way besides it coming from WebKit::WebSecurityOrigin (which only comes from WebCore::WebSecurityOrigin). We can then deal with these details only if/when we need to. On Wed, Oct 14, 2009 at 1:36 PM, Jeremy Orlow jor...@chromium.org wrote: Right now, we don't have a good story for what to do with WebCore::SecurityOrigins in Chromium. We now have a WebSecurityOrigin in WebKit, but if you want to move the data between processes, you need to convert it to a string and then send that. In some cases we then convert the string to a GURL, but this seems like the wrong thing to do (more on this in a sec). To me, the right answer is to create a type in base called SecurityOrigin that wraps a string and does equality checks. The equality checks can be done as string comparisons since the WebCore::SecurityOrigin::toString() method
[chromium-dev] Re: layout test dashboard goofup
On Wed, Oct 14, 2009 at 4:53 PM, Ojan Vafai o...@google.com wrote: The data is stored in a single file per bot. For example, the webkit release bot's results are at http://build.chromium.org/buildbot/layout_test_results/webkit-rel/results.json. That file holds all the historical data for that bot and is copied over during the archive step of each run. We intentionally limit the number of results we keep in that file to 750 runs to keep filesize down. In my accidental checking, I changed 750 to 9. :( A little bit unrelated: This data, along with all the data on build.chromium.org, is replicated on at least 4 machines. It would be easy to recover the data if the server dies for example. We are also planning to do daily backups, but the data is huge. For example, we archive 25GB of new layout test results every day. Nicolas A trivial to implement backup would be to also copy the file to the archive location for just that run (same place as where we copy layout_test_results.zip), e.g. also copy it to http://build.chromium.org/buildbot/layout_test_results/webkit-rel/29056/. The downside is that this uses up disk space (e.g. the largest results.json file was 25mb before being clobbered). Another problem with backing up is that you'd also have to find a way to restore from backup that didn't lose data from runs that happened since the problem occurred. Merging the two files results.json should be pretty relatively trivial code, but it's all code that someone would need to write and test. While it sucks, I don't think backing up this data is worth the effort. It's a temporary productivity hit for the team, but we get enough new data to make reasonable decisions relatively quickly. Mistakes like this are very rare. It will become even more rare as coding work on the dashboard winds down. Feel free to have at it if you disagree. Creates the results.json file and it's content: trunk/src/webkit/tools/layout_tests/layout_package/json_results_generator.py Copies the results.json file to the right place: trunk/tools/buildbot/scripts/slave/chromium/archive_layout_test_results.py Ojan On Wed, Oct 14, 2009 at 4:24 PM, Jeremy Orlow jor...@chromium.org wrote: I haven't actually gotten anything done on LocalStorage this week because I've been doing so many small side projects like this.but if it's a priority, sure. How about a cron job on some machine that ssh's via a cert into whatever machines the data is stored on, pulls it back, and dumps it into some dir? When we start filling up the hard drive, we can look at doing something smarter, deleting old data, or putting it somewhere like GFS. What server can I use and where's the data stored? On Wed, Oct 14, 2009 at 4:17 PM, Evan Martin e...@chromium.org wrote: Sounds like we've got a volunteer! :D :D :D On Wed, Oct 14, 2009 at 4:15 PM, Jeremy Orlow jor...@chromium.org wrote: I assume we're going to start backing this data up from now on? On Wed, Oct 14, 2009 at 4:13 PM, Peter Kasting pkast...@google.com wrote: On Wed, Oct 14, 2009 at 3:58 PM, Ojan Vafai o...@google.com wrote: I accidentally checked in some test code (one number was wrong!) and clobbered all but 10 of the runs of data for each builder. There's no way to recover it. Do you moonlight for the Danger team at Microsoft? PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Is it time to create a SecurityOrigin class in Chromium?
I think the main points of contention regarding SecurityOrigin::toString() are the name toString and what type it should return. In terms of the names: I think we should provide constructors and getters for each component of each WebKit API type. So, for example, WebSecurityOrigin should have a constructor that takes in a single string and a getter called |origin|. (We can argue about the name later.) base::SecurityOrigin can then have a method to produce WebSecurityOrigins (using that constructor that takes in the base componentswhich is just origin in this classes case) and it can have a constructor that takes in a WebSecurityOrigin (which will construct itself by using the aforementioned getter(s)). In terms of what it should return: Each component should really be a C++ primitive. This of course leaves the question of how to handle strings. Personally, I think we should do one of 2 things: Either WebString should expose its components (an array of unsigned shorts + a size_t) or we should just put string16 into the WebKit API. The latter is not as insane as it sounds: it's just a special case of std::string. The former isn't that insane either since it can be fed directly to a std::string(16) constructor. If we add the proper constructors and factories for WebKit types to NullableString16, GURL, SecurityOrigin, and any other types we might want to connect to WebKit types in the future, then we can actually make the dependency 100% one way. I guess I feel like we should either say that depending on base types is OK (as long as we think they'll be very stable implementation wise) or we should say they're never allowed. This middle ground just feels wrong. Note that the difference between toString and a single getter that returns a string is subtle, but if all WebKit types follow this same convention, then it's much less likely that people will do stupid things with them. J On Wed, Oct 14, 2009 at 5:15 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 4:51 PM, Jeremy Orlow jor...@chromium.org wrote: You mean string16, right? I see instances of std::string and string16. I would be happiest if we unified on one. string16 is probably the path of least resistance. std::string has the benefit of being more compact, which for something like this which is really just a bag of bytes is probably a good thing. I really don't think it buys us much if it's purely optional that people put the security origin (in string representation) into a wrapper that then blocks them from doing anything silly with it. we could give it a ToString() method. i think the point of SecurityOrigin would be to guide people in the right direction. -darin More to the point, I don't think it's useful enough that I'm going to bother implementing it. If someone else wants to, I'd probably use it. On Wed, Oct 14, 2009 at 4:47 PM, Darin Fisher da...@chromium.org wrote: To be clear: I only weakly advocate Chrome having a SecurityOrigin. I'm also OKwith just using std::string. I think either is better than using GURL. -Darin On Wed, Oct 14, 2009 at 4:45 PM, Darin Fisher da...@chromium.orgwrote: Hmm... it seems useful as a means of making the code more self-documenting anda bit safer. I'd rather not have people passing strings for origins since they might be tempted to parse the strings. The momentary translation to strings on the boundary of the WebKit API does not completely make this fragile. If the end-points in Chrome use SecurityOrigin and the end-points in the WebKit API use WebSecurityOrigin, then developers will be naturally inclined to convert between SecurityOrigin and WebSecurityOrigin, ignoring the toString() getter on WebSecurityOrigin. This is a case where existing code should help guide people in the right direction. -Darin On Wed, Oct 14, 2009 at 4:39 PM, Jeremy Orlow jor...@chromium.orgwrote: If this is the case, then I don't think a SecurityOrigin wrapper buys us anything. Never mind. On Wed, Oct 14, 2009 at 4:37 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 3:59 PM, Jeremy Orlow jor...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:48 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher da...@chromium.orgwrote: On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher da...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow jor...@chromium.org wrote: On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman micha...@google.com wrote: +1 SecurityOrigin class Sounds like a reasonable plan. I suspect there may already be cases where we're actually comparing a chrome generated security origin, as produced by GURL.GetOrigin(), with a webkit generated security origin, as produced by WebSecurityOrigin.toString(). So we may want to accelerate the part of the
[chromium-dev] [announce] git-cl now has presubmit support.. read on to find out how to enable it!
*If you don't use git-cl, you can stop reading now.* Attention git-cl users: git-cl now has presubmit supporthttp://code.google.com/p/chromium/issues/detail?id=5339! New users that set up their git checkout on or after Wednesday (2009/10/14) are already configured with presubmit support. Feel free to skip the setup instructions since you've already run them but read on to learn more about the recent changes. *Presubmit support for Existing Users (if you set up your git checkout before 2009/10/14):* Run these commands to install the required git hooks in your repository: 1. cd /work/chromium/src # *where /work/chromium/src is the path to your git repository* 2. gclient sync # *to upgrade your copy of depot_tools and git-cl* 3. git cl config http://src.chromium.org/svn/ # *to install the git-cl presubmit hooks* You must rerun the git cl config command in each of your local git repositories. * * *New and updated git-cl commands:* * * git-cl presubmit Runs upload and commit presubmit checks on the current changelist. git-cl upload Runs presubmit tests on upload, continues even if tests fail. git-cl dcommit Run presubmit tests on commit, halts if tests fail. To bypass the presubmit tests in upload or dcommit, use the --bypass-hooks flag. git-cl dcommit's -f now implies --bypass-hooks along with skipping the commit confirm prompt. *Questions?* Feel free to reply to me off-list with any questions you have. Thanks to Evan Martin, Marc-Antoine Ruel, and Nicolas Sylvain for their help making git-cl presubmit support a reality. Chase --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
