[webkit-changes] cvs commit: WebCore/khtml/html htmltokenizer.cpp htmltokenizer.h

Geoffrey ggaren at opensource.apple.com
Thu Oct 13 12:11:24 PDT 2005


ggaren      05/10/13 12:11:23

  Modified:    .        ChangeLog
               khtml/html htmltokenizer.cpp htmltokenizer.h
  Log:
          - Fixed <rdar://problem/4259434> Safari crashes in HTMLTokenizer::~HTMLTokenizer()
            at http://www.timewarner.com/corp/careers/jobtools_us/index.html
  
          I changed the test for whether to put a script in the "to be executed" queue to
          match the test for whether to ref a script, so that scripts can't end up in the
          queue without being refed.
  
          I also renamed cachedScript to pendingScripts.
  
          Reviewed by Darin.
  
          No test case because the crash isn't deterministically reproducible.
          However, I did add assertions that should catch the underlying bug
          in the future.
  
          * khtml/html/htmltokenizer.cpp:
          (khtml::HTMLTokenizer::reset):
          (khtml::HTMLTokenizer::scriptHandler):
          (khtml::HTMLTokenizer::write):
          (khtml::HTMLTokenizer::notifyFinished):
          * khtml/html/htmltokenizer.h:
  
  Revision  Changes    Path
  1.239     +24 -0     WebCore/ChangeLog
  
  Index: ChangeLog
  ===================================================================
  RCS file: /cvs/root/WebCore/ChangeLog,v
  retrieving revision 1.238
  retrieving revision 1.239
  diff -u -r1.238 -r1.239
  --- ChangeLog	13 Oct 2005 08:29:19 -0000	1.238
  +++ ChangeLog	13 Oct 2005 19:11:20 -0000	1.239
  @@ -1,3 +1,27 @@
  +2005-10-13  Geoffrey Garen  <ggaren at apple.com>
  +
  +        - Fixed <rdar://problem/4259434> Safari crashes in HTMLTokenizer::~HTMLTokenizer() 
  +          at http://www.timewarner.com/corp/careers/jobtools_us/index.html
  +
  +        I changed the test for whether to put a script in the "to be executed" queue to
  +        match the test for whether to ref a script, so that scripts can't end up in the
  +        queue without being refed.
  +
  +        I also renamed cachedScript to pendingScripts.
  +
  +        Reviewed by Darin.
  +
  +        No test case because the crash isn't deterministically reproducible.
  +        However, I did add assertions that should catch the underlying bug
  +        in the future.
  +
  +        * khtml/html/htmltokenizer.cpp:
  +        (khtml::HTMLTokenizer::reset):
  +        (khtml::HTMLTokenizer::scriptHandler):
  +        (khtml::HTMLTokenizer::write):
  +        (khtml::HTMLTokenizer::notifyFinished):
  +        * khtml/html/htmltokenizer.h:
  +
   2005-10-13  Rob Buis  <rwlbuis at xs4all.nl>
   
           Reviewed by eseidel.
  
  
  
  1.117     +22 -14    WebCore/khtml/html/htmltokenizer.cpp
  
  Index: htmltokenizer.cpp
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/html/htmltokenizer.cpp,v
  retrieving revision 1.116
  retrieving revision 1.117
  diff -u -r1.116 -r1.117
  --- htmltokenizer.cpp	3 Oct 2005 21:12:30 -0000	1.116
  +++ htmltokenizer.cpp	13 Oct 2005 19:11:23 -0000	1.117
  @@ -205,9 +205,12 @@
       assert(m_executingScript == 0);
       assert(onHold == false);
   
  -    while (!cachedScript.isEmpty())
  -        cachedScript.dequeue()->deref(this);
  -
  +    while (!pendingScripts.isEmpty()) {
  +      CachedScript *cs = pendingScripts.dequeue();
  +      assert(cs->accessCount() > 0);
  +      cs->deref(this);
  +    }
  +    
       if ( buffer )
           KHTML_DELETE_QCHAR_VEC(buffer);
       buffer = dest = 0;
  @@ -381,17 +384,22 @@
   {
       // We are inside a <script>
       bool doScriptExec = false;
  +
  +    // (3837) Scripts following a frameset element should not execute or, 
  +    // in the case of extern scripts, even load.
  +    bool followingFrameset = (parser->doc()->body() && parser->doc()->body()->hasTagName(framesetTag));
  +  
       CachedScript* cs = 0;
       // don't load external scripts for standalone documents (for now)
       if (!scriptSrc.isEmpty() && parser->doc()->part()) {
           // forget what we just got; load from src url instead
  -        if ( !parser->skipMode() ) {
  +        if (!parser->skipMode() && !followingFrameset) {
   #ifdef INSTRUMENT_LAYOUT_SCHEDULING
               if (!parser->doc()->ownerElement())
                   printf("Requesting script at time %d\n", parser->doc()->elapsedTime());
   #endif
               if ( (cs = parser->doc()->docLoader()->requestScript(scriptSrc, scriptSrcCharset) ))
  -                cachedScript.enqueue(cs);
  +                pendingScripts.enqueue(cs);
           }
           scriptSrc=QString::null;
       }
  @@ -411,8 +419,6 @@
       currToken.beginTag = false;
       processToken();
   
  -    // Scripts following a frameset element should not be executed or even loaded in the case of extern scripts.
  -    bool followingFrameset = (parser->doc()->body() && parser->doc()->body()->hasTagName(framesetTag));
       TokenizerString *savedPrependingSrc = currentPrependingSrc;
       TokenizerString prependingSrc;
       currentPrependingSrc = &prependingSrc;
  @@ -430,7 +436,7 @@
               scriptCodeSize = scriptCodeResync = 0;
               cs->ref(this);
               // will be 0 if script was already loaded and ref() executed it
  -            if (!cachedScript.isEmpty())
  +            if (!pendingScripts.isEmpty())
                   loadingExtScript = true;
           }
           else if (view && doScriptExec && javascript ) {
  @@ -1337,7 +1343,7 @@
       if (loadStopped)
           return;
   
  -    if ( ( m_executingScript && appendData ) || !cachedScript.isEmpty() ) {
  +    if ( ( m_executingScript && appendData ) || !pendingScripts.isEmpty() ) {
           // don't parse; we will do this later
   	if (currentPrependingSrc) {
   	    currentPrependingSrc->append(str);
  @@ -1773,13 +1779,15 @@
           printf("script loaded at %d\n", parser->doc()->elapsedTime());
   #endif
   
  -    assert(!cachedScript.isEmpty());
  +    assert(!pendingScripts.isEmpty());
       bool finished = false;
  -    while (!finished && cachedScript.head()->isLoaded()) {
  +    while (!finished && pendingScripts.head()->isLoaded()) {
   #ifdef TOKEN_DEBUG
           kdDebug( 6036 ) << "Finished loading an external script" << endl;
   #endif
  -        CachedScript* cs = cachedScript.dequeue();
  +        CachedScript* cs = pendingScripts.dequeue();
  +        assert(cs->accessCount() > 0);
  +
           DOMString scriptSource = cs->script();
   #ifdef TOKEN_DEBUG
           kdDebug( 6036 ) << "External script is:" << endl << scriptSource.qstring() << endl;
  @@ -1798,9 +1806,9 @@
   
   	scriptExecution( scriptSource.qstring(), cachedScriptUrl );
   
  -        // The state of cachedScript.isEmpty() can change inside the scriptExecution()
  +        // The state of pendingScripts.isEmpty() can change inside the scriptExecution()
           // call above, so test afterwards.
  -        finished = cachedScript.isEmpty();
  +        finished = pendingScripts.isEmpty();
           if (finished) {
               loadingExtScript = false;
   #ifdef INSTRUMENT_LAYOUT_SCHEDULING
  
  
  
  1.41      +1 -1      WebCore/khtml/html/htmltokenizer.h
  
  Index: htmltokenizer.h
  ===================================================================
  RCS file: /cvs/root/WebCore/khtml/html/htmltokenizer.h,v
  retrieving revision 1.40
  retrieving revision 1.41
  diff -u -r1.40 -r1.41
  --- htmltokenizer.h	23 Sep 2005 17:38:06 -0000	1.40
  +++ htmltokenizer.h	13 Oct 2005 19:11:23 -0000	1.41
  @@ -308,7 +308,7 @@
       // true if we are executing a script while parsing a document. This causes the parsing of
       // the output of the script to be postponed until after the script has finished executing
       int m_executingScript;
  -    QPtrQueue<CachedScript> cachedScript;
  +    QPtrQueue<CachedScript> pendingScripts;
       // you can pause the tokenizer if you need to display a dialog or something
       bool onHold;
   
  
  
  



More information about the webkit-changes mailing list