[PDF] Speculative fix for chrome crash w/ SRC xfer mode.

[PDF] Speculative fix for chrome crash w/ SRC xfer mode.

BUG=chromium:316546
Committed: http://code.google.com/p/skia/source/detail?r=12253

[reed1, 2013-11-12 18:13:58]

without the 'else's its hard to see when and why you want to permit
fall-through. Can you add comments or reorg the code to make that clearer?

[vandebo (ex-Chrome), 2013-11-12 18:20:07]

On 2013/11/12 18:13:58, reed1 wrote:
> without the 'else's its hard to see when and why you want to permit
> fall-through. Can you add comments or reorg the code to make that clearer?

Not sure what you mean by fall-through.

The else's in the current code is unnecessary since the xfer modes are
mutually exclusive.  The thing causing the crash is that srcFormXObject is
NULL, but is being used.  I reordered the if blocks so that all the
consumers of srcFormXObject are after the block that now checks for it.

[reed1, 2013-11-12 18:22:16]

On 2013/11/12 18:20:07, vandebo wrote:
> On 2013/11/12 18:13:58, reed1 wrote:
> > without the 'else's its hard to see when and why you want to permit
> > fall-through. Can you add comments or reorg the code to make that clearer?
> 
> Not sure what you mean by fall-through.
> 
> The else's in the current code is unnecessary since the xfer modes are
> mutually exclusive.  The thing causing the crash is that srcFormXObject is
> NULL, but is being used.  I reordered the if blocks so that all the
> consumers of srcFormXObject are after the block that now checks for it.

1. Sometimes I can get to line 1986, and sometimes I can't (i.e. there is a
return in the if-block)
2. A series of ifs that happen to have mutually exclusive predicates is not
nearly as clear as if-else blocks. If it is meant to be exclusive blocks (i.e. a
switch with no fall-through), then I vote for using that or if/else blocks.

[vandebo (ex-Chrome), 2013-11-12 19:22:39]

On 2013/11/12 18:22:16, reed1 wrote:
> On 2013/11/12 18:20:07, vandebo wrote:
> > On 2013/11/12 18:13:58, reed1 wrote:
> > > without the 'else's its hard to see when and why you want to permit
> > > fall-through. Can you add comments or reorg the code to make that clearer?
> > 
> > Not sure what you mean by fall-through.
> > 
> > The else's in the current code is unnecessary since the xfer modes are
> > mutually exclusive.  The thing causing the crash is that srcFormXObject is
> > NULL, but is being used.  I reordered the if blocks so that all the
> > consumers of srcFormXObject are after the block that now checks for it.
> 
> 1. Sometimes I can get to line 1986, and sometimes I can't (i.e. there is a
> return in the if-block)
> 2. A series of ifs that happen to have mutually exclusive predicates is not
> nearly as clear as if-else blocks. If it is meant to be exclusive blocks (i.e.
a
> switch with no fall-through), then I vote for using that or if/else blocks.

Changed back to an if-else construct.  But I'm not sure that really addresses
your
concern. The Assert on line 1974 documents the cases that should fall through.

[reed1, 2013-11-12 19:26:47]

I find this patch more readable, thanks. lgtm

[commit-bot: I haz the power, 2013-11-12 19:27:35]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/vandebo@chromium.org/61153003/70001

[commit-bot: I haz the power, 2013-11-12 19:36:16]

Change committed as 12253