|Summary:||REGRESSION (r34883): Google Reader doesn't show up feed list on sidebar|
|Product:||WebKit||Reporter:||Ismail Donmez <ismail>|
|Component:||New Bugs||Assignee:||Cameron Zwarich (cpst) <zwarich>|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Ismail Donmez 2008-06-30 12:38:26 PDT
This is a regression introduced in last 6 hours. How to reproduce: - Go to http://reader.google.com - Make sure Show: Updated is selected Now the new feeds entries are visible on the right side but the sidebar shows no feed title/count.
Comment 1 Ismail Donmez 2008-06-30 12:41:24 PDT
Created attachment 22010 [details] Screenshot showing the issue.
Comment 2 Cameron Zwarich (cpst) 2008-06-30 13:46:56 PDT
I can confirm that this regresses with r34883.
Comment 3 Cameron Zwarich (cpst) 2008-06-30 16:31:31 PDT
This only happens when I turn on the optimization for the emitJumpIfFalse from ConditionalNode, but I am not sure why. The optimization should be totally safe. If I reset the last opcode ID at the top of ConditionalNode::emitCode(), the bug is fixed, which seems to suggest that there is a bug in our register ref counting scheme.
Comment 4 Cameron Zwarich (cpst) 2008-06-30 17:15:38 PDT
The bug is fixed by resetting generator.m_lastOpcodeID between the two calls to generator.emitNode() in CommaNode::emitCode(). At the moment, I am not sure why.
Comment 5 Cameron Zwarich (cpst) 2008-06-30 17:37:07 PDT
There is never a CommaNode with a ConditionalNode on the right, so the ConditionalNode must be emitted by the first emitNode call for some other node type. I'll dump backtraces of ConditionalNode::emitCode() to try and find it. I need to narrow this down, because so much is put in a CommaNode in the obfuscated Google JS.
Comment 6 Cameron Zwarich (cpst) 2008-06-30 17:54:30 PDT
I found the problem. I was merging less and jfalse even when the destination of less was a local. I have a fix, but I need to make a layout test.
Comment 7 Cameron Zwarich (cpst) 2008-06-30 18:02:49 PDT
Created attachment 22012 [details] Proposed patch I will write some layout tests, but I'll put this up for review first.
Comment 8 Oliver Hunt 2008-06-30 18:04:28 PDT
Comment on attachment 22012 [details] Proposed patch r=me with tests