Bug 66939

Summary: add a state enumeration to track down cause of null CachedScript execution
Product: WebKit Reporter: Gavin Peters <gavinp>
Component: New BugsAssignee: Gavin Peters <gavinp>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Gavin Peters 2011-08-25 07:29:27 PDT
add a state enumeration to track down cause of null CachedScript execution
Comment 1 Gavin Peters 2011-08-25 07:32:48 PDT
Created attachment 105187 [details]
Patch
Comment 2 Gavin Peters 2011-08-25 07:34:47 PDT
Over in http://code.google.com/p/chromium/issues/detail?id=75604 I
have a bug I cannot reproduce.  I added an earlier crash in
https://bugs.webkit.org/show_bug.cgi?id=65563 , and we've since
gotten many good stacks, all coming in through a failed request that
eventually calls notifyFinished() on a ScriptElement with a NULL
m_cachedScript.

I'd like to know how this got NULL.  This enumeration should let
me find that in stack dumps from reproductions.
Comment 3 Alexey Proskuryakov 2011-08-25 09:05:38 PDT
Comment on attachment 105187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105187&action=review

> Source/WebCore/dom/ScriptElement.h:114
> +      NEVER_SET,
> +      SET,
> +      ZEROED_IN_STOPLOADREQUEST,
> +      ZEROED_IN_NOTIFYFINISHED,

This is not correct coding style. Macros are ALL_CAPS, enum values are AllCaps. Also, I don't believe that volatile can possibly help.
Comment 4 Gavin Peters 2011-08-25 12:39:55 PDT
Created attachment 105232 [details]
Patch
Comment 5 Gavin Peters 2011-08-25 12:41:01 PDT
Comment on attachment 105187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105187&action=review

>> Source/WebCore/dom/ScriptElement.h:114
>> +      ZEROED_IN_NOTIFYFINISHED,
> 
> This is not correct coding style. Macros are ALL_CAPS, enum values are AllCaps. Also, I don't believe that volatile can possibly help.

Done.
Comment 6 Gavin Peters 2011-08-25 12:41:15 PDT
Comment on attachment 105232 [details]
Patch

ap, wdyt?
Comment 7 WebKit Review Bot 2011-08-25 13:50:57 PDT
Comment on attachment 105232 [details]
Patch

Rejecting attachment 105232 [details] from commit-queue.

gavinp@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 8 WebKit Review Bot 2011-08-25 16:18:49 PDT
Comment on attachment 105232 [details]
Patch

Rejecting attachment 105232 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 1

Last 500 characters of output:
cc304bc0192fcc06917ef1c5686a310637392395
r93833 = b65f40e0665dc2934a732051a04f6f2db4852cb1
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9509600
Comment 9 Gavin Peters 2011-08-25 19:43:10 PDT
Created attachment 105294 [details]
Patch
Comment 10 Gavin Peters 2011-08-25 19:44:09 PDT
Comment on attachment 105294 [details]
Patch

Same patch that ap approved, only with "Reviewed by" in the Changelog.
Comment 11 WebKit Review Bot 2011-08-25 19:44:42 PDT
Comment on attachment 105294 [details]
Patch

Rejecting attachment 105294 [details] from review queue.

gavinp@chromium.org does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 12 Alexey Proskuryakov 2011-08-25 21:31:18 PDT
Comment on attachment 105294 [details]
Patch

You don't need r+ for commit queue to land a patch if ChangeLogs have reviewer name.
Comment 13 WebKit Review Bot 2011-08-25 22:30:43 PDT
Comment on attachment 105294 [details]
Patch

Rejecting attachment 105294 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
0020e2bba6888e8d7e39751953a927b2dd8fd492
r93851 = ecf76e2c2a018efee91ce9d2327963bbb29aa34c
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9512783
Comment 14 Gavin Peters 2011-08-26 05:06:19 PDT
Created attachment 105347 [details]
Patch
Comment 15 Gavin Peters 2011-08-26 05:06:45 PDT
Comment on attachment 105347 [details]
Patch

I wonder if this works better when I spell your name correctly, Alexey.
Comment 16 WebKit Review Bot 2011-08-26 05:07:25 PDT
Comment on attachment 105347 [details]
Patch

Rejecting attachment 105347 [details] from review queue.

gavinp@chromium.org does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 17 Gavin Peters 2011-08-26 05:15:34 PDT
Comment on attachment 105347 [details]
Patch

Aha, now I get what you said about r+.  Trying now.
Comment 18 WebKit Review Bot 2011-08-26 06:15:14 PDT
Comment on attachment 105347 [details]
Patch

Clearing flags on attachment: 105347

Committed r93871: <http://trac.webkit.org/changeset/93871>
Comment 19 WebKit Review Bot 2011-08-26 06:15:19 PDT
All reviewed patches have been landed.  Closing bug.