Closed Bug 1368571 Opened 2 years ago Closed 2 years ago

Marshal proxies from chrome process to remote clients sans handler

Categories

(Core :: Disability Access APIs, enhancement)

Unspecified
Windows
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: aklotz, Assigned: aklotz)

Details

(Whiteboard: aes+)

Attachments

(1 file)

This is to deal with situations like 32-bit nvda.exe against 64-bit Firefox.
When I test this against inspect.exe, the handler is loaded in our chrome process but not in inspect.exe itself.
Attachment #8875083 - Flags: review?(jmathies)
Comment on attachment 8875083 [details] [diff] [review]
Strip handler out of marshaled object references

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

Looks great. One question, non-issue based.

::: ipc/mscom/Objref.cpp
@@ +187,5 @@
> +  if (FAILED(hr) || bytesRead != sizeof(numEntries)) {
> +    return false;
> +  }
> +
> +  // We'll try to use a stack buffer if resAddrSize <= kMinDualStringArraySize

I don't understand why you chose two different methods for creating this target buffer. Is the use of the static buf for perf reasons?
Attachment #8875083 - Flags: review?(jmathies) → review+
(In reply to Jim Mathies [:jimm] from comment #2)
> Comment on attachment 8875083 [details] [diff] [review]
> Strip handler out of marshaled object references
> 
> Review of attachment 8875083 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great. One question, non-issue based.
> 
> ::: ipc/mscom/Objref.cpp
> @@ +187,5 @@
> > +  if (FAILED(hr) || bytesRead != sizeof(numEntries)) {
> > +    return false;
> > +  }
> > +
> > +  // We'll try to use a stack buffer if resAddrSize <= kMinDualStringArraySize
> 
> I don't understand why you chose two different methods for creating this
> target buffer. Is the use of the static buf for perf reasons?

That is correct. Ideally we will avoid the heap allocation in most cases. From what I've seen, it is most likely that resAddrSize will fall within the kMinDualStringArraySize limit, however that is not guaranteed, hence the heap option.
https://hg.mozilla.org/integration/mozilla-inbound/rev/46699016ed9c06e82c88f4b255246430db2c9dd3
Bug 1368571: Modify handler and interceptor marshaling code so that it strips out any handlers from proxies that are destined for non-Gecko processes; r=jimm
https://hg.mozilla.org/mozilla-central/rev/46699016ed9c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.