Closed Bug 1479591 Opened 6 years ago Closed 6 years ago

Introduce scrolling accessibility event

Categories

(Core :: Disability Access APIs, enhancement)

Unspecified
Android
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 6 obsolete files)

In Android there is a TYPE_VIEW_SCOLLED[1] event. The name is misleading, this is a scrolling event that is regularly fired as a view is being scrolled. A typical android view will dispatch this event every 100ms while a scroll is occurring.

The view also provides the current scroll position and the max position in both x and y.

https://developer.android.com/reference/android/view/accessibility/AccessibilityEvent#TYPE_VIEW_SCROLLED
This patch slightly changes EVENT_SCROLLING_END for simplicity sake.

Currently, on each scroll callback a time is (re)set for 50 ms. If the timer is called twice (aka 100ms), without any more scroll callbacks being called, an EVENT_SCROLLING_END is fired. I'm not sure why it is set up that way, and why not simply have a 100ms timer that is called once.

This patch changes the timer to not be reset on each scroll callback, and to be fired on a 100ms interval. If there were no scroll callbacks since the last timer fired, we dispatch an EVENT_SCROLLING_END event. This means that the event will be dispatched between +100ms and -200ms after the last scroll callback.

If this doesn't seem reasonable, I think the only way to harmonize these two events is to have to timers. I can do that, but was hoping on cutting some of the complexity here.
Attachment #8996097 - Flags: review?(surkov.alexander)
Oops. I think I fixed some build issues I found in try.
Attachment #8996139 - Flags: review?(surkov.alexander)
Attachment #8996097 - Attachment is obsolete: true
Attachment #8996097 - Flags: review?(surkov.alexander)
a generic question: why do you need accessibility scrolling event, i.e why you cannot use DOM onscroll? Is it something related to cross-process boundaries?
That's right. We used to be able to do this with DOM scroll in a frame script (although the screen bounds would not be right).

To do this natively we need to formalize this in an ipdl and since it is an accessibility event..
Assignee: nobody → eitan
Comment on attachment 8996139 [details] [diff] [review]
Introduced accessibility scrolling event and interface. r?surkov

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

it appears browser_events_scroll.js was accidentally added

::: accessible/base/AccEvent.cpp
@@ +277,5 @@
>    }
>  
> +  if (eventGroup & (1 << AccEvent::eScrollingEvent)) {
> +    AccScrollingEvent* sa = downcast_accEvent(aEvent);
> +    xpEvent = new xpcAccScrollingEvent(type,ToXPC(acc), ToXPCDocument(doc), node,

nit: space after "type,"

@@ +279,5 @@
> +  if (eventGroup & (1 << AccEvent::eScrollingEvent)) {
> +    AccScrollingEvent* sa = downcast_accEvent(aEvent);
> +    xpEvent = new xpcAccScrollingEvent(type,ToXPC(acc), ToXPCDocument(doc), node,
> +                                    fromUser, sa->ScrollX(), sa->ScrollY(),
> +                                    sa->MaxScrollX(), sa->MaxScrollY());

nit: indentatation

::: accessible/base/AccEvent.h
@@ +554,5 @@
> +class AccScrollingEvent : public AccEvent
> +{
> +public:
> +  AccScrollingEvent(Accessible* aAccessible, int32_t aScrollX, int32_t aScrollY,
> +                 int32_t aMaxScrollX, int32_t aMaxScrollY) :

nit: indentation

@@ +573,5 @@
> +
> +  int32_t ScrollX() { return mScrollX; }
> +  int32_t ScrollY() { return mScrollY; }
> +  int32_t MaxScrollX() { return mMaxScrollX; }
> +  int32_t MaxScrollY() { return mMaxScrollY; }

please document these

::: accessible/generic/DocAccessible.cpp
@@ +610,5 @@
>  {
>    DocAccessible* docAcc = reinterpret_cast<DocAccessible*>(aClosure);
>  
> +  if (docAcc) {
> +    nsIPresShell *presShell = docAcc->PresShell();

nit: type* name

@@ -610,5 @@
>  {
>    DocAccessible* docAcc = reinterpret_cast<DocAccessible*>(aClosure);
>  
> -  if (docAcc && docAcc->mScrollPositionChangedTicks &&
> -      ++docAcc->mScrollPositionChangedTicks > 2) {

while the logic seems has been preserved, I would prefer for sure to land the change separately from the main patch

@@ +651,5 @@
>    // then the ::Notify() method will fire the accessibility event for scroll position changes
> +  const uint32_t kScrollPosCheckWait = 100;
> +  mScrollPositionChanged = true;
> +  if (!mScrollWatchTimer) {
> +    ScrollTimerCallback(nullptr, this);

this is weird you call a callback directly, not sure I catch why previous approach didn't work out. It seems reasonable, if no timers, then create one, if there's a timer on stack, then delay it.

::: accessible/generic/DocAccessible.h
@@ +603,5 @@
>      mNodeToAccessibleMap;
>  
>    nsIDocument* mDocumentNode;
>      nsCOMPtr<nsITimer> mScrollWatchTimer;
> +    bool mScrollPositionChanged; // Used for tracking scroll events

nit: indentation
use mDocFlags to store value

::: accessible/interfaces/nsIAccessibleEvent.idl
@@ +418,5 @@
>     */
>    const unsigned long EVENT_TEXT_VALUE_CHANGE = 0x0057;
>  
> +  /**
> +   * An accessible's viewport is scrolling.

is -> was? due to async nature of events

::: accessible/interfaces/nsIAccessibleScrollingEvent.idl
@@ +12,5 @@
> +[scriptable, builtinclass, uuid(f75f0b32-5342-4d60-b1a5-b7bd6888eef5)]
> +interface nsIAccessibleScrollingEvent : nsIAccessibleEvent
> +{
> +  /**
> +   * New x scroll position in device pixels.

maybe add: within a scrollable target?

@@ +14,5 @@
> +{
> +  /**
> +   * New x scroll position in device pixels.
> +   */
> +  readonly attribute long scrollX;

these are not unsigned longs, because negative values indicate scrolling to top? if so, then it's be nice to document that as well

@@ +17,5 @@
> +   */
> +  readonly attribute long scrollX;
> +
> +  /**
> +   * New y scroll position in device pixels

nit: dot is missed, here and below

::: accessible/ipc/DocAccessibleParent.cpp
@@ +435,5 @@
> +{
> +  ProxyAccessible* target = GetAccessible(aID);
> +
> +#if defined(ANDROID)
> +  ProxyScrollingEvent(target, aScrollX, aScrollY, aMaxScrollX, aMaxScrollY);

hm, ProxyScrollingEvent implementation is in other platforms only and it is empty, not sure how it works

@@ +441,5 @@
> +
> +  xpcAccessibleGeneric* xpcAcc = GetXPCAccessible(target);
> +  xpcAccessibleDocument* doc = GetAccService()->GetXPCDocument(this);
> +  nsINode* node = nullptr;
> +  bool fromUser = true; // XXX fix me

more detailed XXX comment will be nice, maybe point out how it can be fixed

@@ +444,5 @@
> +  nsINode* node = nullptr;
> +  bool fromUser = true; // XXX fix me
> +  RefPtr<xpcAccScrollingEvent> event =
> +    new xpcAccScrollingEvent(nsIAccessibleEvent::EVENT_SCROLLING, xpcAcc, doc,
> +      node, fromUser, aScrollX, aScrollY, aMaxScrollX, aMaxScrollY);

nit: usually arguments are indented relative each other, thus 'node' should be under 'nsIAccessibleEvent::EVENT_SCROLLING argument)

::: accessible/tests/browser/events/browser_test_scrolling.js
@@ +15,5 @@
> +      content.location.hash = "#two";
> +    });
> +    let [scrollEvent1] = await onScrolling;
> +    scrollEvent1.QueryInterface(nsIAccessibleScrollingEvent);
> +    ok(scrollEvent1.maxScrollY >= scrollEvent1.scrollY, "scrollY is within max");

test for scrollX as well?
Comment on attachment 8996139 [details] [diff] [review]
Introduced accessibility scrolling event and interface. r?surkov

cancelling review until comments are addressed
Flags: needinfo?(eitan)
Attachment #8996139 - Flags: review?(surkov.alexander)
(In reply to alexander :surkov (:asurkov) from comment #5)
> Comment on attachment 8996139 [details] [diff] [review]
> @@ +651,5 @@
> >    // then the ::Notify() method will fire the accessibility event for scroll position changes
> > +  const uint32_t kScrollPosCheckWait = 100;
> > +  mScrollPositionChanged = true;
> > +  if (!mScrollWatchTimer) {
> > +    ScrollTimerCallback(nullptr, this);
> 
> this is weird you call a callback directly, not sure I catch why previous
> approach didn't work out. It seems reasonable, if no timers, then create
> one, if there's a timer on stack, then delay it.
> 

The previous approach kept delaying the timer until there were no more scroll callbacks for 100ms. We need to fire an event every 100ms. So we can't delay the timer. I have an idea for another way to implement this. I'll come back with a very different patch soon. I'll incorporate your nits into it.
> 
> ::: accessible/interfaces/nsIAccessibleEvent.idl
> @@ +418,5 @@
> >     */
> >    const unsigned long EVENT_TEXT_VALUE_CHANGE = 0x0057;
> >  
> > +  /**
> > +   * An accessible's viewport is scrolling.
> 
> is -> was? due to async nature of events
> 

"is" == SROLLING
"was" == SCROLLING_END

Of course, we can't promise that the scrolling is still happening when the AT gets a SCROLLING, but we can promise that a SCROLLING_END will follow when it's done.
Flags: needinfo?(eitan)
(In reply to Eitan Isaacson [:eeejay] from comment #7)
> (In reply to alexander :surkov (:asurkov) from comment #5)
> is weird you call a callback directly, not sure I catch why previous
> > approach didn't work out. It seems reasonable, if no timers, then create
> > one, if there's a timer on stack, then delay it.
> > 
> 
> The previous approach kept delaying the timer until there were no more
> scroll callbacks for 100ms. We need to fire an event every 100ms.

what do you mean we should fire an event 100ms, is it some requirement we have to meet?

> So we
> can't delay the timer. I have an idea for another way to implement this.
> I'll come back with a very different patch soon. I'll incorporate your nits
> into it.

ok, looking forward to see it, thanks! :)

> > 
> > ::: accessible/interfaces/nsIAccessibleEvent.idl
> > @@ +418,5 @@
> > >     */
> > >    const unsigned long EVENT_TEXT_VALUE_CHANGE = 0x0057;
> > >  
> > > +  /**
> > > +   * An accessible's viewport is scrolling.
> > 
> > is -> was? due to async nature of events
> > 
> 
> "is" == SROLLING
> "was" == SCROLLING_END
> 
> Of course, we can't promise that the scrolling is still happening when the
> AT gets a SCROLLING, but we can promise that a SCROLLING_END will follow
> when it's done.

fair enough
(In reply to alexander :surkov (:asurkov) from comment #5)
> Comment on attachment 8996139 [details] [diff] [review]
> @@ +14,5 @@
> > +{
> > +  /**
> > +   * New x scroll position in device pixels.
> > +   */
> > +  readonly attribute long scrollX;
> 
> these are not unsigned longs, because negative values indicate scrolling to
> top? if so, then it's be nice to document that as well

Android uses signed int32s, so I am using that in our internal code too.

What do you think?

> ::: accessible/ipc/DocAccessibleParent.cpp
> @@ +435,5 @@
> > +{
> > +  ProxyAccessible* target = GetAccessible(aID);
> > +
> > +#if defined(ANDROID)
> > +  ProxyScrollingEvent(target, aScrollX, aScrollY, aMaxScrollX, aMaxScrollY);
> 
> hm, ProxyScrollingEvent implementation is in other platforms only and it is
> empty, not sure how it works
> 

I shared a link above with the basic API docs. An Android implementation will follow.
A much simpler implementation, doh.
Attachment #8997529 - Flags: review?(surkov.alexander)
Attachment #8996139 - Attachment is obsolete: true
OK, maybe I'm getting ahead of myself here. But I made SCROLLING_END support the event interface as well. Seems like this could be useful for knowing the final scroll position.
Attachment #8997555 - Flags: review?(surkov.alexander)
Attachment #8997529 - Attachment is obsolete: true
Attachment #8997529 - Flags: review?(surkov.alexander)
Comment on attachment 8997555 [details] [diff] [review]
Introduced accessibility scrolling event and interface. r?surkov

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

::: accessible/generic/DocAccessible.cpp
@@ +635,5 @@
>  {
> +  TimeStamp timestamp = TimeStamp::Now();
> +  if ((timestamp - mLastScrollingDispatch).ToMilliseconds() >= 100) {
> +    DispatchScrollingEvent(nsIAccessibleEvent::EVENT_SCROLLING);
> +    mLastScrollingDispatch = timestamp;

so whenever scrolling starts, then you send event_scrolling event, and then you repeat it each next 100ms. Correct?

Also, I'm not sure I understand why event_scrolling_event gets fired at all. It is not related with the patch, but since you revisit the code, then a question :) Every time when ScrollPositionDidChange is triggered, then ScrollTimerCallback is scheduled, and mScrollPositionChangedTicks is set to 1. Thus it should never reach > 2 value. What do I miss?

::: accessible/generic/DocAccessible.h
@@ +606,5 @@
>  
>    nsIDocument* mDocumentNode;
> +  nsCOMPtr<nsITimer> mScrollWatchTimer;
> +  uint16_t mScrollPositionChangedTicks; // Used for tracking scroll events
> +  TimeStamp mLastScrollingDispatch;

it makes sense to keep these in the initialization list
ni? to make sure it's not lost in bugmail
Flags: needinfo?(eitan)
(In reply to Eitan Isaacson [:eeejay] from comment #9)
> (In reply to alexander :surkov (:asurkov) from comment #5)
> > Comment on attachment 8996139 [details] [diff] [review]
> > @@ +14,5 @@
> > > +{
> > > +  /**
> > > +   * New x scroll position in device pixels.
> > > +   */
> > > +  readonly attribute long scrollX;
> > 
> > these are not unsigned longs, because negative values indicate scrolling to
> > top? if so, then it's be nice to document that as well
> 
> Android uses signed int32s, so I am using that in our internal code too.
> 
> What do you think?

well, if they are never negative, then I would use unsigned types, less questions to implementation. Unsigned is casted to signed just fine.
(In reply to alexander :surkov (:asurkov) from comment #12)
> Comment on attachment 8997555 [details] [diff] [review]
> Introduced accessibility scrolling event and interface. r?surkov
> 
> Review of attachment 8997555 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/generic/DocAccessible.cpp
> @@ +635,5 @@
> >  {
> > +  TimeStamp timestamp = TimeStamp::Now();
> > +  if ((timestamp - mLastScrollingDispatch).ToMilliseconds() >= 100) {
> > +    DispatchScrollingEvent(nsIAccessibleEvent::EVENT_SCROLLING);
> > +    mLastScrollingDispatch = timestamp;
> 
> so whenever scrolling starts, then you send event_scrolling event, and then
> you repeat it each next 100ms. Correct?
> 
> Also, I'm not sure I understand why event_scrolling_event gets fired at all.
> It is not related with the patch, but since you revisit the code, then a
> question :) Every time when ScrollPositionDidChange is triggered, then
> ScrollTimerCallback is scheduled, and mScrollPositionChangedTicks is set to
> 1. Thus it should never reach > 2 value. What do I miss?

The timer callback is repeated at the given interval (50ms). So when the scrolling callback stops being called, it will tick twice. Does that make sense? I'm not sure why it isn't simply a one-time callback at a 100ms delay. I'm assuming there is a good reason.. let me git blame that.. wow. It is from bug 135482. 17 years ago.

I can change it, but honestly I'm not sure if that weirdness is there intentionally. I'll give it a try!

> 
> ::: accessible/generic/DocAccessible.h
> @@ +606,5 @@
> >  
> >    nsIDocument* mDocumentNode;
> > +  nsCOMPtr<nsITimer> mScrollWatchTimer;
> > +  uint16_t mScrollPositionChangedTicks; // Used for tracking scroll events
> > +  TimeStamp mLastScrollingDispatch;
> 
> it makes sense to keep these in the initialization list

I only introduced mLastScrollingDispatch, and its public constructor doesn't take arguments (initializes to null/0).
Not sure what else I should do here.
Flags: needinfo?(eitan)
I reworked how SCROLLING_END works too. Because, of course.
Attachment #8999011 - Flags: review?(surkov.alexander)
Attachment #8997555 - Attachment is obsolete: true
Attachment #8997555 - Flags: review?(surkov.alexander)
(In reply to Eitan Isaacson [:eeejay] from comment #15)

> > >    nsIDocument* mDocumentNode;
> > > +  nsCOMPtr<nsITimer> mScrollWatchTimer;
> > > +  uint16_t mScrollPositionChangedTicks; // Used for tracking scroll events
> > > +  TimeStamp mLastScrollingDispatch;
> > 
> > it makes sense to keep these in the initialization list
> 
> I only introduced mLastScrollingDispatch, and its public constructor doesn't
> take arguments (initializes to null/0).
> Not sure what else I should do here.

yeah, it makes sense to initialize the others too. It's a good practice to initialize class members explicitly, since sometimes it saves from hard-to-find bugs. Also I think static analysis should be unhappy about these (see bug 525063 and bug 1412641).
Comment on attachment 8999011 [details] [diff] [review]
Introduced accessibility scrolling event and interface. r?surkov

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

assuming you didn't change the files other than DocAccessible, I skip those. r=me with the comments addressed.

::: accessible/generic/DocAccessible.cpp
@@ +640,2 @@
>    if (mScrollWatchTimer) {
> +    mScrollWatchTimer->SetDelay(kScrollEventInterval);  // Create new timer, to avoid leaks

it appears you don't need to have multiple timers. If you cancel the exiting one, and re-schedule a new one, then it's all you need. In this case if the callback gets triggered, then it means, no new events was received. Correct?
Attachment #8999011 - Flags: review?(surkov.alexander) → review+
I guess you mean update the comment? It looks wrong, yeah.
Attachment #8999011 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dad95d849302
Introduced accessibility scrolling event and interface. r=surkov
Keywords: checkin-needed
Backed out changeset for causing linux debug leaks and security issues.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&fromchange=dad95d849302c966da9a710e96c80e7882eec5c2&tochange=9e6ef0b94077b72e1b9bb4bb310bfb561cd94f07&filter-searchStr=bc5&selectedJob=193992110

Failure log: 
https://treeherder.mozilla.org/logviewer.html#?job_id=193992110&repo=mozilla-inbound&lineNumber=1409 - security issue
https://treeherder.mozilla.org/logviewer.html#?job_id=193991397&repo=mozilla-inbound&lineNumber=1748 - leaks 

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/9e6ef0b94077b72e1b9bb4bb310bfb561cd94f07

[task 2018-08-15T00:22:05.077Z] 00:22:05     INFO - GECKO(1076) | SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/a11y/ProxyAccessibleBase.h:133:31 in IsDoc
[task 2018-08-15T00:22:05.078Z] 00:22:05     INFO - GECKO(1076) | Shadow bytes around the buggy address:
[task 2018-08-15T00:22:05.079Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
[task 2018-08-15T00:22:05.080Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a970: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
[task 2018-08-15T00:22:05.082Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a980: 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
[task 2018-08-15T00:22:05.083Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a990: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
[task 2018-08-15T00:22:05.084Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a9a0: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa
[task 2018-08-15T00:22:05.085Z] 00:22:05     INFO - GECKO(1076) | =>0x0c1c8001a9b0: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
[task 2018-08-15T00:22:05.086Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a9c0: fd fd fd fa fa fa fa fa fa fa fa fa 00 00 00 00
[task 2018-08-15T00:22:05.086Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
[task 2018-08-15T00:22:05.087Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a9e0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
[task 2018-08-15T00:22:05.088Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001a9f0: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
[task 2018-08-15T00:22:05.089Z] 00:22:05     INFO - GECKO(1076) |   0x0c1c8001aa00: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
[task 2018-08-15T00:22:05.090Z] 00:22:05     INFO - GECKO(1076) | Shadow byte legend (one shadow byte represents 8 application bytes):
[task 2018-08-15T00:22:05.090Z] 00:22:05     INFO - GECKO(1076) |   Addressable:           00
[task 2018-08-15T00:22:05.091Z] 00:22:05     INFO - GECKO(1076) |   Partially addressable: 01 02 03 04 05 06 07
[task 2018-08-15T00:22:05.091Z] 00:22:05     INFO - GECKO(1076) |   Heap left redzone:       fa
[task 2018-08-15T00:22:05.093Z] 00:22:05     INFO - GECKO(1076) |   Freed heap region:       fd
[task 2018-08-15T00:22:05.094Z] 00:22:05     INFO - GECKO(1076) |   Stack left redzone:      f1
[task 2018-08-15T00:22:05.095Z] 00:22:05     INFO - GECKO(1076) |   Stack mid redzone:       f2
[task 2018-08-15T00:22:05.096Z] 00:22:05     INFO - GECKO(1076) |   Stack right redzone:     f3
[task 2018-08-15T00:22:05.097Z] 00:22:05     INFO - GECKO(1076) |   Stack after return:      f5
[task 2018-08-15T00:22:05.097Z] 00:22:05     INFO - GECKO(1076) |   Stack use after scope:   f8
[task 2018-08-15T00:22:05.098Z] 00:22:05     INFO - GECKO(1076) |   Global redzone:          f9
[task 2018-08-15T00:22:05.098Z] 00:22:05     INFO - GECKO(1076) |   Global init order:       f6
[task 2018-08-15T00:22:05.099Z] 00:22:05     INFO - GECKO(1076) |   Poisoned by user:        f7
[task 2018-08-15T00:22:05.100Z] 00:22:05     INFO - GECKO(1076) |   Container overflow:      fc
[task 2018-08-15T00:22:05.101Z] 00:22:05     INFO - GECKO(1076) |   Array cookie:            ac
[task 2018-08-15T00:22:05.102Z] 00:22:05     INFO - GECKO(1076) |   Intra object redzone:    bb
[task 2018-08-15T00:22:05.104Z] 00:22:05     INFO - GECKO(1076) |   ASan internal:           fe
[task 2018-08-15T00:22:05.105Z] 00:22:05     INFO - GECKO(1076) |   Left alloca redzone:     ca
[task 2018-08-15T00:22:05.105Z] 00:22:05     INFO - GECKO(1076) |   Right alloca redzone:    cb
[task 2018-08-15T00:22:05.106Z] 00:22:05     INFO - GECKO(1076) | ==1076==ABORTING
[task 2018-08-15T00:22:05.136Z] 00:22:05     INFO - GECKO(1076) | AddressSanitizer:DEADLYSIGNAL
[task 2018-08-15T00:22:05.142Z] 00:22:05     INFO - GECKO(1076) | =================================================================
[task 2018-08-15T00:22:05.145Z] 00:22:05    ERROR - GECKO(1076) | ==1263==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe267b1df53 bp 0x7fe263c0f300 sp 0x7fe263c0f2e0 T2)
[task 2018-08-15T00:22:05.173Z] 00:22:05     INFO - GECKO(1076) | ==1263==The signal is caused by a WRITE memory access.
[task 2018-08-15T00:22:05.179Z] 00:22:05     INFO - GECKO(1076) | ==1263==Hint: address points to the zero page.
[task 2018-08-15T00:22:05.183Z] 00:22:05     INFO - GECKO(1076) | Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=2.53965) Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=6.06538) [GFX1-]: Receive IPC close with reason=AbnormalShutdown
[task 2018-08-15T00:22:05.187Z] 00:22:05     INFO - GECKO(1076) | [Child 1233, Chrome_ChildThread] WARNING: pipe error (3): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 356
[task 2018-08-15T00:22:05.190Z] 00:22:05     INFO - GECKO(1076) | AddressSanitizer:DEADLYSIGNAL
[task 2018-08-15T00:22:05.193Z] 00:22:05     INFO - GECKO(1076) | =================================================================
[task 2018-08-15T00:22:05.198Z] 00:22:05    ERROR - GECKO(1076) | ==1233==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fdb2281df53 bp 0x7fdb1e90f300 sp 0x7fdb1e90f2e0 T2)
[task 2018-08-15T00:22:05.203Z] 00:22:05     INFO - GECKO(1076) | ==1233==The signal is caused by a WRITE memory access.
[task 2018-08-15T00:22:05.205Z] 00:22:05     INFO - GECKO(1076) | ==1233==Hint: address points to the zero page.
[task 2018-08-15T00:22:05.211Z] 00:22:05     INFO - GECKO(1076) | AddressSanitizer:DEADLYSIGNAL
[task 2018-08-15T00:22:05.214Z] 00:22:05     INFO - GECKO(1076) | =================================================================
[task 2018-08-15T00:22:05.216Z] 00:22:05    ERROR - GECKO(1076) | ==1205==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe3e0a1df53 bp 0x7fe3dcb0f300 sp 0x7fe3dcb0f2e0 T2)
[task 2018-08-15T00:22:05.218Z] 00:22:05     INFO - GECKO(1076) | ==1205==The signal is caused by a WRITE memory access.
[task 2018-08-15T00:22:05.224Z] 00:22:05     INFO - GECKO(1076) | Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=9.13202) ==1205==Hint: address points to the zero page.
[task 2018-08-15T00:22:05.225Z] 00:22:05     INFO - GECKO(1076) | AddressSanitizer:DEADLYSIGNAL
[task 2018-08-15T00:22:05.227Z] 00:22:05     INFO - GECKO(1076) | =================================================================
[task 2018-08-15T00:22:05.232Z] 00:22:05     INFO - GECKO(1076) | [Child 1146, Chrome_ChildThread] WARNING: pipe error (3): Connection reset by peer: file /builds/worker/workspace/build/src/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 356
[task 2018-08-15T00:22:05.234Z] 00:22:05     INFO - GECKO(1076) | AddressSanitizer:DEADLYSIGNAL
[task 2018-08-15T00:22:05.236Z] 00:22:05     INFO - GECKO(1076) | =================================================================
[task 2018-08-15T00:22:05.240Z] 00:22:05    ERROR - GECKO(1076) | ==1146==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7efe3501df53 bp 0x7efe3110f300 sp 0x7efe3110f2e0 T2)
[task 2018-08-15T00:22:05.244Z] 00:22:05    ERROR - GECKO(1076) | ==1171==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f2e5a71df53 bp 0x7f2e5680f300 sp 0x7f2e5680f2e0 T2)
[task 2018-08-15T00:22:05.252Z] 00:22:05     INFO - GECKO(1076) | ==1171==The signal is caused by a WRITE memory access.
[task 2018-08-15T00:22:05.254Z] 00:22:05     INFO - GECKO(1076) | ==1171==Hint: address points to the zero page.
[task 2018-08-15T00:22:05.257Z] 00:22:05     INFO - GECKO(1076) | ==1146==The signal is caused by a WRITE memory access.
[task 2018-08-15T00:22:05.258Z] 00:22:05     INFO - GECKO(1076) | ==1146==Hint: address points to the zero page.
Flags: needinfo?(eitan)
Added a check if xpc observers exist. The other recv event methods do this already..
Attachment #9000030 - Attachment is obsolete: true
Flags: needinfo?(eitan)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/da7e0c710c7d
Introduced accessibility scrolling event and interface. r=surkov
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/da7e0c710c7d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1483911
Depends on: 1484778
Depends on: 1519922
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: