skia/ext: Early out from analysis when we have more than 1 draw op.

skia/ext: Early out from analysis when we have more than 1 draw op.

This patch puts in a heuristic that earlies out from analysis when
we have more than one draw op. The intent here is that in most cases,
if we see more than one draw operation, then analysis is likely not
to be solid.

It also removes HasText from analysis canvas, since the only
remaining use for this was a previous early out heuristic.

BUG=397198
R=enne

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285499

[vmpstr, 2014-07-24 19:10:57]

Please take a look.

enne: general review
senorblanco/junov: owner's review

[enne (OOO), 2014-07-24 19:15:04]

You should mention why you're removing HasText as well in the description.

lgtm

https://codereview.chromium.org/418093003/diff/20001/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/418093003/diff/20001/skia/ext/analysis_canvas...
skia/ext/analysis_canvas.cc:321: return draw_op_count_ > 1;
I think you could be less conservative than 1, but I'm not going to argue too
hard.

[vmpstr, 2014-07-24 19:23:14]

https://codereview.chromium.org/418093003/diff/20001/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/418093003/diff/20001/skia/ext/analysis_canvas...
skia/ext/analysis_canvas.cc:321: return draw_op_count_ > 1;
On 2014/07/24 19:15:03, enne wrote:
> I think you could be less conservative than 1, but I'm not going to argue too
> hard.

I think we should leave it at 1 for now, and then check things like "if we move
this to 2, how many more tiles do we get detected as solid vs how much more time
do we spend in analysis".

At least then we can pinpoint a number that we're comfortable with. I've added a
todo here for myself.

[Justin Novosad, 2014-07-24 20:16:15]

lgtm

https://codereview.chromium.org/418093003/diff/40001/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/418093003/diff/40001/skia/ext/analysis_canvas...
skia/ext/analysis_canvas.cc:324: return draw_op_count_ > 1;
Since this is an heuristic, it feels like the '1' should be a settable member
variable rather than a hard coded constant. Just a thought. I don't feel very
strongly about this, so lgtm anyways

[vmpstr, 2014-07-24 21:03:03]

https://codereview.chromium.org/418093003/diff/40001/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/418093003/diff/40001/skia/ext/analysis_canvas...
skia/ext/analysis_canvas.cc:324: return draw_op_count_ > 1;
On 2014/07/24 20:16:15, junov wrote:
> Since this is an heuristic, it feels like the '1' should be a settable member
> variable rather than a hard coded constant. Just a thought. I don't feel very
> strongly about this, so lgtm anyways

Sounds good... For now I'd rather keep this hard coded, since I don't want to
burden the system using this with figuring out what to set this value to.
However, it might make sense that we might even have different values for
different analysis types in the future. I'll keep this in mind.

[vmpstr, 2014-07-24 21:03:07]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-07-24 21:06:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vmpstr@chromium.org/418093003/40001

[commit-bot: I haz the power, 2014-07-25 02:27:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-25 06:04:45]

Change committed as 285499

[lushnikov, 2014-07-25 10:15:51]

A revert of this CL has been created in
https://codereview.chromium.org/416333003/ by lushnikov@chromium.org.

The reason for reverting is: This change broke some tests in Blink
Additionally, it renders DevTools shortcuts screen unusable (Open devTools -> F1
-> Shortcuts tab)

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpec....