Canvas2d: Fix a bug to update incorrect rect bounds for accessibility.

Canvas2d: Fix a bug to update incorrect rect bounds for accessibility.

This CL is fixing the bug to update incorrect rect bounds for accessibility in
drawFocusIfNeeded() and addHitRegion() APIs. The bug couldn't reproduce in
existing tests because layout tree was updated by other apis incidentally.
So, this CL adds two unit tests for the issue to resolve.

BUG=328961, 513763
Committed: https://crrev.com/f97956ed8c863babc49da036c2bb0d2195834e0e
git-svn-id: svn://svn.chromium.org/blink/trunk@199608 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[zino, 2015-07-20 11:50:48]

jinho.bang@samsung.com changed reviewers:
+ junov@chromium.org

[zino, 2015-07-20 11:51:02]

Please take a look.

Thank you.

[zino, 2015-07-23 17:18:31]

Ping, Justin

Please take a look.

Thank you.

[Justin Novosad, 2015-07-23 19:33:15]

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
File Source/modules/canvas2d/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2111:
element->document().updateLayoutTreeIfNeeded();
I recently found out that it is much preferable to call
updateLayoutTreeForNodeIfNeeded(elementYouAreInterestedIn) This way, you avoid
triggering a layout and style computation in cases where the computed style of
the canvas is known to be up to date (though other parts of the document may not
be).

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2123:
element->document().updateLayoutIgnorePendingStylesheets();
We should not add new call to this function, unless you have a really good
reason. See the comments in this function in Document.cpp.  Also why is this
necessary? I don't anything that would invalidate layout or style between the
above update and here.

[zino, 2015-07-27 02:19:48]

jinho.bang@samsung.com changed reviewers:
+ dmazzoni@chromium.org

[zino, 2015-07-27 13:59:45]

Thank you for review.

Please take a look again.

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
File Source/modules/canvas2d/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2111:
element->document().updateLayoutTreeIfNeeded();
On 2015/07/23 19:33:15, Justin Novosad wrote:
> I recently found out that it is much preferable to call
> updateLayoutTreeForNodeIfNeeded(elementYouAreInterestedIn) This way, you avoid
> triggering a layout and style computation in cases where the computed style of
> the canvas is known to be up to date (though other parts of the document may
not
> be).

Thank you for information.

BTW, I removed this line in a new patch set.
I'm not sure but we don't need it (to resolve this issue).

https://codereview.chromium.org/1242263002/diff/1/Source/modules/canvas2d/Can...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2123:
element->document().updateLayoutIgnorePendingStylesheets();
Please see the http://crbug.com/513763.

We will have to update the |absolute bounding rect| of canvas path.
If there isn't the line, we can't get the correct location(canvas location) from
|renderer->absoluteBoundingBoxRect()|.

[Justin Novosad, 2015-07-27 14:36:36]

https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
File Source/modules/canvas2d/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2182:
element->document().updateLayoutIgnorePendingStylesheets();
You should to do this update update before calling canvas()->layoutObject().
OTherwise, the early return above may exit in cases where it should not.

[dmazzoni, 2015-07-27 15:07:47]

lgtm

[zino, 2015-07-28 00:38:48]

I addressed your comment.

Thank you for correcting me.

Could you review again?

https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
File Source/modules/canvas2d/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2182:
element->document().updateLayoutIgnorePendingStylesheets();
On 2015/07/27 14:36:36, Justin Novosad wrote:
> You should to do this update update before calling canvas()->layoutObject().
> OTherwise, the early return above may exit in cases where it should not.

Done.

[Justin Novosad, 2015-07-28 20:15:37]

On 2015/07/28 00:38:48, zino wrote:
> I addressed your comment.
> 
> Thank you for correcting me.
> 
> Could you review again?
> 
>
https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
> File Source/modules/canvas2d/CanvasRenderingContext2D.cpp (right):
> 
>
https://codereview.chromium.org/1242263002/diff/20001/Source/modules/canvas2d...
> Source/modules/canvas2d/CanvasRenderingContext2D.cpp:2182:
> element->document().updateLayoutIgnorePendingStylesheets();
> On 2015/07/27 14:36:36, Justin Novosad wrote:
> > You should to do this update update before calling canvas()->layoutObject().
> > OTherwise, the early return above may exit in cases where it should not.
> 
> Done.

lgtm

[Justin Novosad, 2015-07-28 20:15:46]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2015-07-28 20:15:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/1242263002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-07-28 20:16:08]

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

[commit-bot: I haz the power, 2015-07-28 21:39:29]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199608

[commit-bot: I haz the power, 2015-09-23 11:51:46]

Patchset 3 (id:??) landed as
https://crrev.com/f97956ed8c863babc49da036c2bb0d2195834e0e