Re: [webkit-dev] Early deletion of DocumentLoader instances

[Maciej Stachowiak]

On May 25, 2011, at 9:30 PM, Adam Barth wrote:

> 
> Reading my comments on the bug, I was happy that the document had a
> pointer to the DocumentLoader.  My apologies for misunderstanding the
> ownership relations between these objects.  I thought that
> DocumentLoader had Document-lifetime, but it appears that it has
> neither Document-lifetime nor Frame-lifetime.
> 
> The correct fix is likely to change the way to locate the writer() as follows:
> 
> - document->loader()->writer()
> + frame->loader()->activeDocumentLoader()->writer()
> 
> I'll put this at the top of my TODO list for Friday.  (Unfortunately,
> I'm at the W2SP workshop tomorrow and can't fix the issue
> immediately.)

Thanks for the suggestion. I think Brady can probably take a shot at this 
before Friday, but we'll keep you in the loop.

Cheers,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Early deletion of DocumentLoader instances

[Adam Barth]

On Wed, May 25, 2011 at 6:26 PM, Brady Eidson  wrote:
> On May 24, 2011, at 09:31 , Darin Adler wrote:
>> On May 24, 2011, at 6:56 AM, Raphael Kubo da Costa wrote:
 we should fix it by making some better relationship between the Document 
 and DocumentLoader that guarantees we won’t have a dangling pointer. 
 Either reference counting to keep the object alive, or code to zero out 
 the pointer at some point before the object is deleted.
>>>
>>> r80507 added a check for m_frame before using it (Document also has a raw 
>>> Frame pointer). Perhaps the same should be done here?
>>
>> That fix is not a good quality one. It’s a fragile approach. The 
>> relationship of a document to its frame is that the association between the 
>> two is explicitly broken by the detach function in document. But it’s not 
>> clear this is the right way to break the association with a document loader. 
>> Here are three specific points:
>>
>>    1) It’s poor design that the document grabs and keeps a pointer to the 
>> document loader in its constructor. The document is not in a position to 
>> monitor the lifetime of the loader. It would be far more maintainable if the 
>> same code/class both set up and maintained the pointer.
>>
>>    2) It's not clear that detach time is the right moment to invalidate the 
>> pointer from a document to its document loader. It would be better to study 
>> the lifetime of document loader and loading process to get a clearer idea of 
>> exactly what the right time is and what the best relationship between these 
>> objects is.
>>
>>    3) Keeping a dangling m_documentLoader pointer around with no guarantee 
>> that it points to a live object is a bad design pattern. If the loader is no 
>> longer valid when the document is not associated with a frame, then right 
>> way to deal with that is to zero out m_documentLoader in the detach 
>> function, not to check m_frame each time before checking m_documentLoader.
>>
>> This fragile design was introduced just three months ago in 
>> . It might be worth asking Nate 
>> Chapin or Adam Barth if they had some plans for further refinement. They may 
>> have overlooked these design mistakes, but it’s likely they have future 
>> plans that will rectify this.
>>
>> Maybe we could continue this discussion in a bug report?
>
> We now have crash report data showing that this is hitting the Mac port in 
> the field.
>
> I filed https://bugs.webkit.org/show_bug.cgi?id=61494
>
> At this point, I will be working to see how easy it is to simply roll out 
> 78342 since it was only refactoring and not fixing any particular bug.

Reading my comments on the bug, I was happy that the document had a
pointer to the DocumentLoader.  My apologies for misunderstanding the
ownership relations between these objects.  I thought that
DocumentLoader had Document-lifetime, but it appears that it has
neither Document-lifetime nor Frame-lifetime.

The correct fix is likely to change the way to locate the writer() as follows:

- document->loader()->writer()
+ frame->loader()->activeDocumentLoader()->writer()

I'll put this at the top of my TODO list for Friday.  (Unfortunately,
I'm at the W2SP workshop tomorrow and can't fix the issue
immediately.)

Thanks,
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Early deletion of DocumentLoader instances

[Brady Eidson]

On May 24, 2011, at 09:31 , Darin Adler wrote:

> On May 24, 2011, at 6:56 AM, Raphael Kubo da Costa wrote:

>>> we should fix it by making some better relationship between the Document 
>>> and DocumentLoader that guarantees we won’t have a dangling pointer. Either 
>>> reference counting to keep the object alive, or code to zero out the 
>>> pointer at some point before the object is deleted.
>> 
>> r80507 added a check for m_frame before using it (Document also has a raw 
>> Frame pointer). Perhaps the same should be done here?
> 
> That fix is not a good quality one. It’s a fragile approach. The relationship 
> of a document to its frame is that the association between the two is 
> explicitly broken by the detach function in document. But it’s not clear this 
> is the right way to break the association with a document loader. Here are 
> three specific points:
> 
>1) It’s poor design that the document grabs and keeps a pointer to the 
> document loader in its constructor. The document is not in a position to 
> monitor the lifetime of the loader. It would be far more maintainable if the 
> same code/class both set up and maintained the pointer.
> 
>2) It's not clear that detach time is the right moment to invalidate the 
> pointer from a document to its document loader. It would be better to study 
> the lifetime of document loader and loading process to get a clearer idea of 
> exactly what the right time is and what the best relationship between these 
> objects is.
> 
>3) Keeping a dangling m_documentLoader pointer around with no guarantee 
> that it points to a live object is a bad design pattern. If the loader is no 
> longer valid when the document is not associated with a frame, then right way 
> to deal with that is to zero out m_documentLoader in the detach function, not 
> to check m_frame each time before checking m_documentLoader.
> 
> This fragile design was introduced just three months ago in 
> . It might be worth asking Nate 
> Chapin or Adam Barth if they had some plans for further refinement. They may 
> have overlooked these design mistakes, but it’s likely they have future plans 
> that will rectify this.
> 
> Maybe we could continue this discussion in a bug report?

We now have crash report data showing that this is hitting the Mac port in the 
field.

I filed https://bugs.webkit.org/show_bug.cgi?id=61494

At this point, I will be working to see how easy it is to simply roll out 78342 
since it was only refactoring and not fixing any particular bug.

~Brady

> 
>-- Darin
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] SVG font file location

[Saba Taseer]

I am winLauncher to debug the svg fonts course. I have to define my script as a 
string but the svg font is in a seperate file. The glyphs file you mentioned in 
layouttests/svg/custom is also giving different font than in chrome. I think 
this is a file location issue, but I cant find a way to resolve it. I have 
tried copying it into all locations including debug.



Saba Taseer



> Date: Wed, 25 May 2011 16:37:47 -0400
> Subject: Re: [webkit-dev] SVG font file location
> From: rwlb...@gmail.com
> To: stehs...@hotmail.com
> CC: webkit-dev@lists.webkit.org; webkit-h...@lists.webkit.org
> 
> Hi Saba,
> 
> On 25 May 2011 16:08, Saba Taseer  wrote:
> > I am trying to study SVG fonts rendering in webkit. I followed the
> > instructions at http://frabru.de/c.php/article/SVGFonts-usage and got my SVG
> > font running for Chrome. I added the same script as a string in winlauncher
> > to see the course of events SVG text goes through to render SVG text, but
> > the text displays in sans-serif and not in Tomson Talks font. I have
> > copied TomsonTalks.svg file in winLauncher folder. I tried giving the
> > absolute path to the font file too but it still doesnt work. Please let me
> > know where should I put my svg font file for winlauncher application. Here
> > is the script I am using
> > 
> > http://www.w3.org/2000/svg";
> > xmlns:xlink="http://www.w3.org/1999/xlink"; viewBox="0 0 400 300">
> > 
> > 
> >   
> > 
> > 
> >
> >   
> >  
> > 
> >  hello
> >   You cause
> >  
> > 
> >
> > I am not good at scripting, plz help me if I have done smthing wrong in the
> > script.
> 
> If TomsonTalks.svg is in the same dir as the listed svg I think it
> should work. You can try removing the spaces in the name as well, I
> think it looks valid how you did it but maybe there is a bug there.
> Finally, as a sanity check you could try
> LayoutTests/svg/custom/glyph-transformation-with-hkern.svg as a SVG
> font test that should work. If you don't have source checkout you can
> get it here:
> 
> http://svn.webkit.org/repository/webkit/trunk/LayoutTests/svg/custom/
> Cheers,
> 
> Rob.
  ___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] SVG font file location

[Rob Buis]

Hi Saba,

On 25 May 2011 16:08, Saba Taseer  wrote:
> I am trying to study SVG fonts rendering in webkit. I followed the
> instructions at http://frabru.de/c.php/article/SVGFonts-usage and got my SVG
> font running for Chrome. I added the same script as a string in winlauncher
> to see the course of events SVG text goes through to render SVG text, but
> the text displays in sans-serif and not in Tomson Talks font. I have
> copied TomsonTalks.svg file in winLauncher folder. I tried giving the
> absolute path to the font file too but it still doesnt work. Please let me
> know where should I put my svg font file for winlauncher application. Here
> is the script I am using
> 
> http://www.w3.org/2000/svg";
> xmlns:xlink="http://www.w3.org/1999/xlink"; viewBox="0 0 400 300">
> 
> 
>   
> 
>     
>    
>   
>  
> 
>  hello
>   You cause
>  
> 
>
> I am not good at scripting, plz help me if I have done smthing wrong in the
> script.

If TomsonTalks.svg is in the same dir as the listed svg I think it
should work. You can try removing the spaces in the name as well, I
think it looks valid how you did it but maybe there is a bug there.
Finally, as a sanity check you could try
LayoutTests/svg/custom/glyph-transformation-with-hkern.svg as a SVG
font test that should work. If you don't have source checkout you can
get it here:

http://svn.webkit.org/repository/webkit/trunk/LayoutTests/svg/custom/
Cheers,

Rob.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] SVG font file location

[Saba Taseer]

I am trying to study SVG fonts rendering in webkit. I followed the instructions 
at http://frabru.de/c.php/article/SVGFonts-usage and got my SVG font running 
for Chrome. I added the same script as a string in winlauncher to see the 
course of events SVG text goes through to render SVG text, but the text 
displays in sans-serif and not in Tomson Talks font. I have copied 
TomsonTalks.svg file in winLauncher folder. I tried giving the absolute path to 
the font file too but it still doesnt work. Please let me know where should I 
put my svg font file for winlauncher application. Here is the script I am using 
http://www.w3.org/2000/svg"; 
xmlns:xlink="http://www.w3.org/1999/xlink"; viewBox="0 0 400 300"> 
hello  You cause 

I am not good at scripting, plz help me if I have done smthing wrong in the 
script. 


Saba Taseer
  ___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] SVG Fonts in webkit

[Nikolas Zimmermann]

Am 25.05.2011 um 00:42 schrieb Saba Taseer:

> It executes the code in the following #if ENable but the if statement returns 
> false. Thus I presume that SVG_FONTS is enabled. It is instead going to 
> drawSimpleText. 
> #if ENABLE(SVG_FONTS)
> if (primaryFont()->isSVGFont()) {
> drawTextUsingSVGFont(context, run, point, from, to);
> return;
> }
> #endif
> 
> if (codePath(run) != Complex)
> return drawSimpleText(context, run, point, from, to);
> I am running a simple svg script 
>  
>  'http://www.w3.org/2000/svg'>   
>  
>   Text using web safe font
>   
> 

You're not using SVG Fonts at all, you're using "impact, georgia..." which are 
native platform fonts.
Google for existing examples using SVG Fonts :-)

Cheers,
Niko

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev