Closed Bug 1013097 Opened 6 years ago Closed 6 years ago

Remove IPC for Switch Event Because Only Chrome Process Can Receive It

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S3 (6june)

People

(Reporter: mchen, Assigned: kershaw)

Details

Attachments

(1 file, 2 obsolete files)

This bug is intended to remove IPC part of [1], and it should be similar to [2].

Currently there are two ways to receive switch event for headset/headphone insertion: 

  First is from GonkSwitch which listens to UEvent; 

  Second is from nsAppShell which detect it by input device;

This bug focuses on second one. While nsAppShell detected switch event, it will call HAL function to notify underlying module to handle it (GonkSwitch). The currentl HAL function includes part of IPC but actually there is no case to use this IPC because only chrome process can access input device.

[1] http://dxr.mozilla.org/mozilla-central/source/hal/sandbox/SandboxHal.cpp#333

[2] http://dxr.mozilla.org/mozilla-central/source/hal/sandbox/SandboxHal.cpp#339
Assignee: nobody → kechang
Hi Marco,

Would you please help to review my patch?

Thanks.
Attachment #8430649 - Flags: review?(mchen)
Comment on attachment 8430649 [details] [diff] [review]
Bug 1013097-Remove IPC-for Switch Event

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

Thanks for helping this issue.
Could you help to add "AssertMainThread();" into the function as below.

http://dxr.mozilla.org/mozilla-central/source/hal/Hal.cpp?from=hal.cpp&case=true#755

And please ask :dhyland for reveiw.
Attachment #8430649 - Flags: review?(mchen) → feedback+
Attached patch Remove IPC for Switch Event (obsolete) — Splinter Review
Hi Dave,

Would you please help to review this patch?

Thanks.
Attachment #8430649 - Attachment is obsolete: true
Attachment #8431292 - Flags: review?(dhylands)
Comment on attachment 8431292 [details] [diff] [review]
Remove IPC for Switch Event

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

Looks good. r=me with nit resolved.

::: hal/sandbox/SandboxHal.cpp
@@ -329,5 @@
>  
>  void
>  NotifySwitchStateFromInputDevice(SwitchDevice aDevice, SwitchState aState)
>  {
> -  Hal()->SendNotifySwitchStateFromInputDevice(aDevice, aState);

nit: You should add:

unused << aDevice;
unused << aState;

to prevent unused variable warnings.
Attachment #8431292 - Flags: review?(dhylands) → review+
Thanks for the good suggestion.
The patch is updated and pushed to try server.
https://tbpl.mozilla.org/?tree=Try&rev=574cdc71b132
Attachment #8431292 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/207facf2772f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S3 (6june)
You need to log in before you can comment on or make changes to this bug.