Closed Bug 733848 Opened 13 years ago Closed 12 years ago

Intermittent failures in a11y/accessible/name/test_markup.html | Can't get accessible for [object HTMLImageElement]

Categories

(Core :: Disability Access APIs, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox14 --- wontfix
firefox15 --- wontfix
firefox16 --- wontfix
firefox17 --- wontfix
firefox18 --- wontfix

People

(Reporter: msucan, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(5 files)

https://tbpl.mozilla.org/?tree=Fx-Team&rev=5d523da125b0

Rev3 WINNT 6.1 fx-team pgo test mochitest-other

5515 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/name/test_markup.html | Can't get accessible for [object HTMLImageElement]
5516 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/name/test_markup.html | Can't get accessible for [object HTMLImageElement]
5517 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/name/test_markup.html | Can't get accessible for [object HTMLImageElement]
5518 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/name/test_markup.html | Can't get accessible for [object HTMLImageElement] 

Brief Log:
https://tbpl.mozilla.org/php/getParsedLog.php?id=9885501&tree=Fx-Team
Blocks: 438871
Attached patch patch:valid image URL — — Splinter Review
under certain circumstances we can end up with no nsImageFrame for html:img having not valid URL. If it doesn't help then we should wait for html:img accessible creation since nsImageFrame can be constructed late (thanks to bz for discussion).
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #615626 - Flags: review?(marco.zehe)
Comment on attachment 615626 [details] [diff] [review]
patch:valid image URL

Ah OK, this was overlooked when moving the files to a different directory depth. r=me
Attachment #615626 - Flags: review?(marco.zehe) → review+
(In reply to Marco Zehe (:MarcoZ) from comment #69)
> Comment on attachment 615626 [details] [diff] [review]
> patch:valid image URL
> 
> Ah OK, this was overlooked when moving the files to a different directory
> depth. r=me

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb4d4fc1f84f

let's keep it open to see if the patch helps
Whiteboard: [orange] → [leave open][orange]
Target Milestone: --- → mozilla14
no failures on inbound/central after landing on inbound/central. closing as fixed
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Version: Trunk → Other Branch
Whiteboard: [leave open][orange] → [orange]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Alexander, do you have any more ideas about this? :-)
(This is one of the top oranges for the last few days)
We're upto 350 failures now; I'd like this part of the test to be disabled, since this is one of the top oranges on trunk (especially given that whatever is intermittent on mozilla17 will be with us for >year in ESR).
Attachment #648321 - Flags: review?(surkov.alexander)
Attachment #648321 - Flags: review?(surkov.alexander) → review+
Disable failing parts of test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a870e6e0b517
Whiteboard: [orange] → [orange][parts of test disabled][leave open]
Target Milestone: mozilla14 → ---
Version: Other Branch → Trunk
Comment on attachment 648321 [details] [diff] [review]
Disable failing part of the test

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Unknown
User impact if declined: Greater [orange]
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Test-only change
String or UUID changes made by this patch: None
Attachment #648321 - Flags: approval-mozilla-beta?
Attachment #648321 - Flags: approval-mozilla-aurora?
Comment on attachment 648321 [details] [diff] [review]
Disable failing part of the test

test only, NPTOB.
Attachment #648321 - Flags: approval-mozilla-beta?
Attachment #648321 - Flags: approval-mozilla-beta+
Attachment #648321 - Flags: approval-mozilla-aurora?
Attachment #648321 - Flags: approval-mozilla-aurora+
test was reenabled in bug 805373
Whiteboard: [orange][parts of test disabled][leave open] → [orange][leave open]
Attached patch wave over images — — Splinter Review
Attachment #675561 - Flags: review?(trev.saunders)
Attachment #675561 - Flags: review?(trev.saunders) → review+
Comment on attachment 675561 [details] [diff] [review]
wave over images

Review of attachment 675561 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/tests/mochitest/name/markup.js
@@ +111,5 @@
>      child = child.nextSibling;
>    }
>  
> +  // Wave over images to create frames.
> +  var imgElms = div.getElementsByTagName("html:img");

This won't work in HTML documents; you can use getElementsByTagNameNS instead.
(In reply to :Ms2ger from comment #404)

> > +  // Wave over images to create frames.
> > +  var imgElms = div.getElementsByTagName("html:img");
> 
> This won't work in HTML documents; you can use getElementsByTagNameNS
> instead.

due to some reason it worked, i.e. imgElms.length > 0.
when testing markup block is inserted then image accessible is created but then due to some reason it's destroyed and new image insertion is planned. So mouse waves shouldn't make any difference.

more experiments:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9e04e7f6a130
add stacks logging for that bogus image accessible recreation - http://hg.mozilla.org/integration/mozilla-inbound/rev/f75e5dcdbb44 (see comment #417)
Attached patch wait for images — — Splinter Review
Attachment #676648 - Flags: review?(trev.saunders)
f75e5dcdbb44 broke Linux mochitest-a11y for the --disable-profiling builds on https://tbpl.mozilla.org/?tree=Profiling (https://tbpl.mozilla.org/php/getParsedLog.php?id=16592658&tree=Profiling etc.) which means it absolutely has to come out before the next merge since that's how aurora/beta/release build, and it really really needs to come out right away, since it's preventing us from spotting any other mochitest-a11y bustage for --disable-profiling in the meantime.
Sorry, but you can't expect to be able to walk the stack on non-profiling builds which lack frame pointers.  I backed out the patch in comment 453 to fix the non-profiling Linux opt Moth bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/012d146779e2
(In reply to Ehsan Akhgari [:ehsan] from comment #463)
> Sorry, but you can't expect to be able to walk the stack on non-profiling
> builds which lack frame pointers.  I backed out the patch in comment 453 to
> fix the non-profiling Linux opt Moth bustage:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/012d146779e2

Ehsan, do you know how can I detect non-profiling builds on build time to not enable stack walking when it doesn't make sense?
(In reply to comment #468)
> (In reply to Ehsan Akhgari [:ehsan] from comment #463)
> > Sorry, but you can't expect to be able to walk the stack on non-profiling
> > builds which lack frame pointers.  I backed out the patch in comment 453 to
> > fix the non-profiling Linux opt Moth bustage:
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/012d146779e2
> 
> Ehsan, do you know how can I detect non-profiling builds on build time to not
> enable stack walking when it doesn't make sense?

Hide the code that does stackwalking behind #ifdef MOZ_PROFILING.  That should make sure that the code only gets builds in build with --enable-profiling (all inbound/central builds/nightlies.)
Comment on attachment 676648 [details] [diff] [review]
wait for images

I'm not really happy to keep working around whatever is causing the image accessible to get recreated, but I guess I'll live
Attachment #676648 - Flags: review?(trev.saunders) → review+
(In reply to Ehsan Akhgari [:ehsan] from comment #463)
> I backed out the patch in comment 453

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/012d146779e2
landing workaround thing - http://hg.mozilla.org/integration/mozilla-inbound/rev/7a395623e387. I'll enable stack trace logging on builds where it makes sense and change the test to catch up that.
(In reply to Ehsan Akhgari [:ehsan] from comment #470)

> Hide the code that does stackwalking behind #ifdef MOZ_PROFILING.  That
> should make sure that the code only gets builds in build with
> --enable-profiling (all inbound/central builds/nightlies.)

locally I don't have --enable-profiling option and MOZ_PROFILING is not defined. But stacks work ok. My config is
ac_add_options --enable-debug
ac_add_options --disable-optimize
ac_add_options --enable-accessibility
ac_add_options --enable-debug-symbols
ac_add_options --enable-extensions=default,inspector

Perhaps MOZ_PROFILING doesn't cover all possible cases when stack can be meaningful?
(In reply to comment #492)
> (In reply to Ehsan Akhgari [:ehsan] from comment #470)
> 
> > Hide the code that does stackwalking behind #ifdef MOZ_PROFILING.  That
> > should make sure that the code only gets builds in build with
> > --enable-profiling (all inbound/central builds/nightlies.)
> 
> locally I don't have --enable-profiling option and MOZ_PROFILING is not
> defined. But stacks work ok. My config is
> ac_add_options --enable-debug
> ac_add_options --disable-optimize
> ac_add_options --enable-accessibility
> ac_add_options --enable-debug-symbols
> ac_add_options --enable-extensions=default,inspector
> 
> Perhaps MOZ_PROFILING doesn't cover all possible cases when stack can be
> meaningful?

Our tinderbox builds are --enable-optimize, which is what omits the frame pointer.  That's the major difference to your local config I believe.
(In reply to Ehsan Akhgari [:ehsan] from comment #494)

> Our tinderbox builds are --enable-optimize, which is what omits the frame
> pointer.  That's the major difference to your local config I believe.

Should I use MOZ_OPTIMIZE instead or both perhaps (I don't know much about build config stuff)?
(In reply to comment #503)
> (In reply to Ehsan Akhgari [:ehsan] from comment #494)
> 
> > Our tinderbox builds are --enable-optimize, which is what omits the frame
> > pointer.  That's the major difference to your local config I believe.
> 
> Should I use MOZ_OPTIMIZE instead or both perhaps (I don't know much about
> build config stuff)?

It depends on what your goal here is.
I wanted to log the stack where the stack is meaning. Do you concern performance?
(In reply to comment #515)
> I wanted to log the stack where the stack is meaning. Do you concern
> performance?

If you only want it on central, MOZ_PROFILING will do it.  If you want it in debug builds, you can make it conditional on MOZ_DEBUG && MOZ_OPTIMIZE as well, I guess.
Attachment #677645 - Flags: review?(trev.saunders)
Attachment #677645 - Flags: review?(ehsan)
Comment on attachment 677645 [details] [diff] [review]
enable logging on stack enabled builds

>+      if (strncmp(token, sModuleMap[idx].mStr, tokenLen) == 0) {
>+#if !defined(MOZ_PROFILING) && (!defined(MOZ_DEBUG) || defined(MOZ_OPTIMIZE))

So, you'll try to get stacks on builds that are debug and optimized?  I guess that's fine with me after all the  worst you can do is break profiling for a day or two until we notice and backout or change the ifdef

>+        // Stack tracing on profiling enabled or debug not optimized builds.

I think it would be clearer if the comment was "we can't walk the stack on optimized non debug builds that don't have frame pointers" or something I looked at this for a couple minutes thinking the if was for the case we could walk the stack and was  confused.
Attachment #677645 - Flags: review?(trev.saunders) → review+
(In reply to Trevor Saunders (:tbsaunde) from comment #521)
> Comment on attachment 677645 [details] [diff] [review]
> enable logging on stack enabled builds
> 
> >+      if (strncmp(token, sModuleMap[idx].mStr, tokenLen) == 0) {
> >+#if !defined(MOZ_PROFILING) && (!defined(MOZ_DEBUG) || defined(MOZ_OPTIMIZE))
> 
> So, you'll try to get stacks on builds that are debug and optimized?

on enabled profiling builds (I guess that can be both debug and release builds) and on debug and not optimized builds.

> >+        // Stack tracing on profiling enabled or debug not optimized builds.
> 
> I think it would be clearer if the comment was "we can't walk the stack on
> optimized non debug builds that don't have frame pointers"

it sounds good but i'm not sure whether "optimized non debug" is equivalent to "enabled profling or debug non optimized".
Attachment #677645 - Flags: review?(ehsan) → review+
reenable test and enable logging on stack enabled builds: http://hg.mozilla.org/integration/mozilla-inbound/rev/53c6e565cfbe
Ehsan, it doems't seem any of stacks is correct
(In reply to comment #542)
> Ehsan, it doems't seem any of stacks is correct

I don't know why that is.  But at least it doesn't crash any more.  ;-)
I'd assume it's for the same reason that assertion stacks are always wrong (or more accurately, useless, since they correctly give the offset from something miles and miles away) - we download symbols on demand rather than downloading them before every run, and the only thing that knows how to demand that they be downloaded is the crash reporter.
(In reply to comment #556)
> I'd assume it's for the same reason that assertion stacks are always wrong (or
> more accurately, useless, since they correctly give the offset from something
> miles and miles away) - we download symbols on demand rather than downloading
> them before every run, and the only thing that knows how to demand that they be
> downloaded is the crash reporter.

Oh, right, yeah, since the assertion stacks are also captured using NS_StackWalk.
(In reply to Ehsan Akhgari [:ehsan] from comment #565)
> (In reply to comment #556)
> > I'd assume it's for the same reason that assertion stacks are always wrong (or
> > more accurately, useless, since they correctly give the offset from something
> > miles and miles away) - we download symbols on demand rather than downloading
> > them before every run, and the only thing that knows how to demand that they be
> > downloaded is the crash reporter.
> 
> Oh, right, yeah, since the assertion stacks are also captured using
> NS_StackWalk.

Is there a way to workaround it?
(In reply to comment #585)
> Is there a way to workaround it?

Not that I know of.
Depends on: 686400
closing the bug as fixed (test was fixed, no failures anymore)

introduced workaround should be removed when bug 686400 is fixed.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Whiteboard: [orange][leave open] → [orange]
Target Milestone: --- → mozilla19
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: