Fix a possible renderer hang due to an WebAccessibilityObject becoming invalid.

Fix a possible renderer hang due to an WebAccessibilityObject becoming invalid.


BUG=106016
TEST=manual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112749

[dmazzoni, 2011-12-01 18:01:13]

This looks like a good fix.

http://codereview.chromium.org/8772006/diff/1/content/renderer/renderer_acces...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/8772006/diff/1/content/renderer/renderer_acces...
content/renderer/renderer_accessibility.cc:250: while (!parent.isNull() &&
parent.accessibilityIsIgnored())
Maybe check for validity here, too?

[David Tseng, 2011-12-01 18:36:39]

>
>
http://codereview.chromium.org/8772006/diff/1/content/renderer/renderer_acces...
> content/renderer/renderer_accessibility.cc:250: while (!parent.isNull()
> && parent.accessibilityIsIgnored())
> Maybe check for validity here, too?

Done.

[dmazzoni, 2011-12-01 18:59:56]

lgtm

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
content/renderer/renderer_accessibility.cc:236: LOG(WARNING) << "Got
notification on invalid object id " << obj.axID();
Maybe the warning should say that the we got a notification on an invalid object
OR the descendant of an invalid object?

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
content/renderer/renderer_accessibility.cc:254: if (parent.isNull()) {
I think if parent is null or invalid we should continue and not send a
notification, otherwise the code below will likely send the notification on the
invalid parent. Though that probably won't lead to a crash, so it's not as
critical.

[David Tseng, 2011-12-01 19:48:55]

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
File content/renderer/renderer_accessibility.cc (right):

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
content/renderer/renderer_accessibility.cc:236: LOG(WARNING) << "Got
notification on invalid object id " << obj.axID();
On 2011/12/01 18:59:56, Dominic Mazzoni wrote:
> Maybe the warning should say that the we got a notification on an invalid
object
> OR the descendant of an invalid object?

Done.

http://codereview.chromium.org/8772006/diff/7001/content/renderer/renderer_ac...
content/renderer/renderer_accessibility.cc:254: if (parent.isNull()) {
On 2011/12/01 18:59:56, Dominic Mazzoni wrote:
> I think if parent is null or invalid we should continue and not send a
> notification, otherwise the code below will likely send the notification on
the
> invalid parent. Though that probably won't lead to a crash, so it's not as
> critical.

Done.

[David Tseng, 2011-12-01 22:57:20]

+ jam for OWNERS approval.

[jam, 2011-12-01 23:40:43]

On 2011/12/01 22:57:20, David Tseng wrote:
> + jam for OWNERS approval.

lgtm

small request: please try to cycle through OWNERs for rubberstamps :)

[commit-bot: I haz the power, 2011-12-02 00:12:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtseng@chromium.org/8772006/5

[commit-bot: I haz the power, 2011-12-02 01:44:34]

Try job failure for 8772006-5 (retry) on win_rel for step "update".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2011-12-02 17:40:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dtseng@chromium.org/8772006/5

[commit-bot: I haz the power, 2011-12-02 19:08:48]

Change committed as 112749