On Wed, Oct 14, 2009 at 4:32 PM, Michael Nordman <[email protected]>wrote:
> > > On Wed, Oct 14, 2009 at 4:03 PM, Jeremy Orlow <[email protected]> wrote: > >> On Wed, Oct 14, 2009 at 3:58 PM, Dumitru Daniliuc <[email protected]>wrote: >> >>> >>> >>> On Wed, Oct 14, 2009 at 2:47 PM, Darin Fisher <[email protected]>wrote: >>> >>>> On Wed, Oct 14, 2009 at 2:30 PM, Jeremy Orlow <[email protected]>wrote: >>>> >>>>> On Wed, Oct 14, 2009 at 2:23 PM, Darin Fisher <[email protected]>wrote: >>>>> >>>>>> On Wed, Oct 14, 2009 at 2:08 PM, Jeremy Orlow <[email protected]>wrote: >>>>>> >>>>>>> On Wed, Oct 14, 2009 at 2:00 PM, Michael Nordman < >>>>>>> [email protected]> 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: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
