Invalidate children of remote mozAccessible's remote parents on destruction/creation

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: lsocks, Assigned: lsocks)

Tracking

unspecified
mozilla50
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Summary: Invalidate children of remote mozAccessible's remote parents on destruction → Invalidate children of remote mozAccessible's remote parents on destruction/creation
(Assignee)

Comment 1

2 years ago
Created attachment 8754261 [details] [diff] [review]
invalidate case where proxy has proxy parents
Attachment #8754261 - Flags: review?(tbsaunde+mozbugs)
(Assignee)

Updated

2 years ago
Depends on: 1199755
Comment on attachment 8754261 [details] [diff] [review]
invalidate case where proxy has proxy parents

># HG changeset patch
># User Lorien Hu <lorien@lorienhu.com>
># Date 1463646939 25200
>#      Thu May 19 01:35:39 2016 -0700
># Node ID 2570f42e9bb6c0b626129fa6377611f7a2ca3a1c
># Parent  a5a5f6802544a42a628e3a9dbbdf2123c5264e05
>Bug 1274195 - Invalidate children of remote mozAccessible's remote parents
>
>diff --git a/accessible/ipc/DocAccessibleParent.h b/accessible/ipc/DocAccessibleParent.h
>--- a/accessible/ipc/DocAccessibleParent.h
>+++ b/accessible/ipc/DocAccessibleParent.h
>@@ -36,16 +36,18 @@ public:
>     MOZ_COUNT_DTOR_INHERITED(DocAccessibleParent, ProxyAccessible);
>     MOZ_ASSERT(mChildDocs.Length() == 0);
>     MOZ_ASSERT(!ParentDoc());
>   }
> 
>   void SetTopLevel() { mTopLevel = true; }
>   bool IsTopLevel() const { return mTopLevel; }
> 
>+  bool IsShutdown() const { return mShutdown; }
>+
>   /*
>    * Called when a message from a document in a child process notifies the main
>    * process it is firing an event.
>    */
>   virtual bool RecvEvent(const uint64_t& aID, const uint32_t& aType)
>     override;
> 
>   virtual bool RecvShowEvent(const ShowEventData& aData, const bool& aFromUser)
>diff --git a/accessible/mac/Platform.mm b/accessible/mac/Platform.mm
>--- a/accessible/mac/Platform.mm
>+++ b/accessible/mac/Platform.mm
>@@ -51,42 +51,63 @@ ProxyCreated(ProxyAccessible* aProxy, ui
>     type = [mozTableCellAccessible class];
>   else
>     type = GetTypeFromRole(aProxy->Role());
> 
>   uintptr_t accWrap = reinterpret_cast<uintptr_t>(aProxy) | IS_PROXY;
>   mozAccessible* mozWrapper = [[type alloc] initWithAccessible:accWrap];
>   aProxy->SetWrapper(reinterpret_cast<uintptr_t>(mozWrapper));
> 
>-  // Invalidate native parent in parent process's children on proxy creation
>+  mozAccessible* nativeParent = nullptr;
>   if (aProxy->IsDoc() && aProxy->AsDoc()->IsTopLevel()) {
>+    // Invalidate native parent in parent process's children on proxy creation

I'd word that like
the parent of a top level document must be a nonremote accessible.

>     }
>+  } else if (ProxyAccessible* parent = aProxy->Parent()) {

when is it null?

>+    // same for proxy proxy parents

I'd word this one differently too, probably sort of similar to above.

>   if (aProxy->IsDoc() && aProxy->AsDoc()->IsTopLevel()) {
>+    // Invalidate native parent in parent process's children on proxy destruction

similar

>+    }
>+  } else {
>+    if (!aProxy->Document()->IsShutdown()) {
>+      // don't try to inval parent if doc has already been shutdown

that's saying what not why, I'd probably say
when documents are shutdown the children are not shutdown top down, so Parent() may return garbage.

That said I'm not sure its a great idea to not do shutdown top down, maybe we should change that.
Attachment #8754261 - Flags: review?(tbsaunde+mozbugs) → review+

Comment 3

2 years ago
Pushed by lorien@lorienhu.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba2d2122cc04
Invalidate children of remote mozAccessible's remote parents r=tbsaunde

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ba2d2122cc04
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Assignee: nobody → lorien
You need to log in before you can comment on or make changes to this bug.