Add handling for Skia's SkPath::intersect's return value

Add handling for Skia's SkPath::intersect's return value

Skia https://codereview.chromium.org/833943002/ (It is
dangerous to ignore SkRect::intersect's return value) will
make ignoring SkPath::intersect's return value a warning.
This CL proactively cleans up such call sites.

skia-side: reed@
chrome/browser/ui: sky@
test_runner/test_runner: pfeldman@ or dpranke@ or creis@

Committed: https://crrev.com/19263a222694c0bf4daf4a66bcf75622ffb0b567
Cr-Commit-Position: refs/heads/master@{#310292}

[robertphillips, 2015-01-02 21:06:18]

robertphillips@google.com changed reviewers:
+ reed@chromium.org

[robertphillips, 2015-01-02 21:12:08]

robertphillips@google.com changed reviewers:
+ pfeldman@chromium.org, sky@chromium.org

[reed1, 2015-01-05 14:39:19]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2015-01-05 14:39:19]

lgtm

[sky, 2015-01-05 16:32:44]

sky@chromium.org changed reviewers:
+ tdanderson@chromium.org

[sky, 2015-01-05 16:32:44]

+tdanderson for tab.cc changes.

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
chrome/browser/ui/views/tabs/tab.cc:714: if
(intersection.intersect(RectToSkRect(clip))) {
If the two don't intersect, then I think this function should return false.

[tdanderson, 2015-01-05 17:16:44]

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
chrome/browser/ui/views/tabs/tab.cc:714: if
(intersection.intersect(RectToSkRect(clip))) {
On 2015/01/05 16:32:44, sky wrote:
> If the two don't intersect, then I think this function should return false.

I agree. Change the block starting at line 711 to this:

if (clip.size().GetArea()) {
  SkRect intersection(mask->getBounds());
  mask->reset();
  if (!intersection.intersect(RectToSkRect(clip)))
    return false;
  mask->addRect(intersection);
}

[robertphillips, 2015-01-05 19:31:56]

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
File chrome/browser/ui/views/tabs/tab.cc (right):

https://codereview.chromium.org/814123003/diff/1/chrome/browser/ui/views/tabs...
chrome/browser/ui/views/tabs/tab.cc:714: if
(intersection.intersect(RectToSkRect(clip))) {
On 2015/01/05 17:16:43, tdanderson wrote:
> On 2015/01/05 16:32:44, sky wrote:
> > If the two don't intersect, then I think this function should return false.
> 
> I agree. Change the block starting at line 711 to this:
> 
> if (clip.size().GetArea()) {
>   SkRect intersection(mask->getBounds());
>   mask->reset();
>   if (!intersection.intersect(RectToSkRect(clip)))
>     return false;
>   mask->addRect(intersection);
> }
> 

Done.

[tdanderson, 2015-01-05 19:34:39]

Changes to tab.cc lgtm.

[sky, 2015-01-05 20:43:01]

LGTM

[robertphillips, 2015-01-06 12:41:18]

robertphillips@google.com changed reviewers:
+ mkwst@chromium.org

[robertphillips, 2015-01-06 12:42:23]

robertphillips@google.com changed reviewers:
+ dpranke@google.com
- mkwst@chromium.org

[robertphillips, 2015-01-06 20:05:52]

robertphillips@google.com changed reviewers:
+ mkwst@chromium.org

[robertphillips, 2015-01-06 20:09:03]

robertphillips@google.com changed reviewers:
+ creis@chromium.org
- mkwst@chromium.org

[Charlie Reis, 2015-01-06 22:21:41]

dpranke@ is probably a better reviewer for this, but it looks simple enough that
I'll offer a rubber stamp LGTM.

(nit: What code style is mock_web_theme_engine.cc using?  It's neither Blink nor
Chromium style.  I would normally suggest no braces on the if, but this is
unfamiliar to me.)

[Dirk Pranke, 2015-01-07 02:16:55]

On 2015/01/06 22:21:41, Charlie Reis wrote:
> dpranke@ is probably a better reviewer for this, but it looks simple enough
that
> I'll offer a rubber stamp LGTM.
> 
> (nit: What code style is mock_web_theme_engine.cc using?  It's neither Blink
nor
> Chromium style.  I would normally suggest no braces on the if, but this is
> unfamiliar to me.)

It should be following the Chromium style; I'm not sure why it isn't, except
since it was originally in Blink perhaps it only got partially converted when it
moved over.

At any rate, I would agree that nominally you should skip the curly braces as
well, but otherwise this also LGTM.

[robertphillips, 2015-01-07 15:21:15]

braces removed.

[robertphillips, 2015-01-07 15:21:20]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-01-07 15:22:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/814123003/40001

[commit-bot: I haz the power, 2015-01-07 16:08:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-07 16:09:22]

Patchset 3 (id:??) landed as
https://crrev.com/19263a222694c0bf4daf4a66bcf75622ffb0b567
Cr-Commit-Position: refs/heads/master@{#310292}