1175460 - Remove private.broadcast.stop_recording hacks

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

With bug 1154231 we should be able to remove most of them, if not all.

$ grep -r private.broadcast apps/
apps//calendar/js/ext/threads.js:  this.private.broadcast(type, data);
apps//fm/js/fm.js:    'private.broadcast.attention_screen_opening',
apps//sharedtest/test/unit/stop_recording_event_test.js:  test('setting private.broadcast.stop_recording triggers event',
apps//sharedtest/test/unit/stop_recording_event_test.js:           'private.broadcast.stop_recording': true
apps//sharedtest/test/unit/stop_recording_event_test.js:           'private.broadcast.stop_recording': false
apps//sharedtest/test/unit/stop_recording_event_test.js:  test('setting private.broadcast.attention_screen_opening triggers event',
apps//sharedtest/test/unit/stop_recording_event_test.js:           'private.broadcast.attention_screen_opening': true
apps//sharedtest/test/unit/stop_recording_event_test.js:           'private.broadcast.attention_screen_opening': false
apps//sharedtest/test/unit/stop_recording_event_test.js:      'private.broadcast.attention_screen_opening': true
apps//sharedtest/test/unit/stop_recording_event_test.js:      'private.broadcast.stop_recording': true
apps//sms/lib/bridge.js:  this.private.broadcast(type, data, clients);
apps//system/js/app_window_manager.js:     * private.broadcast.attention_screen_opening setting hack in
apps//system/js/app_window_manager.js:        'private.broadcast.stop_recording': true
apps//system/js/app_window_manager.js:          'private.broadcast.stop_recording': false
apps//system/js/attention_window_manager.js:            'private.broadcast.attention_screen_opening': false
apps//system/js/attention_window_manager.js:            'private.broadcast.attention_screen_opening': true

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

private.broadcast.attention_screen_opening is (will be) removed in bug 1170944, so that leaves this bug with only private.broadcast.stop_recording to remove.

Dominic, what it takes to remove it? Do you know? Can we remove it now that we have visibility timing and audio channel timing fixed? Thanks!

Comment 2

[Dominic Kuo [:dkuo]]

The |private.broadcast.stop_recording| hack was first introduced in bug 1051172, it attempted to addressed the cases that the user uses the home button to switch to another app, while the camera is recording. So it's slightly different from the |private.broadcast.attention_screen_opening| hack though they both wanted to address the visiblitychange issues.

If I recalled correctly, no bug and no one complains about the audio channel things on the home button switching cases, also the comment says once bug 1034001 is fixed we can remove the |private.broadcast.stop_recording| hack, so I think we can remove it now.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Dominic Kuo [:dkuo] from comment #2)
> I think we can remove it now.

Can you do it :) ?

Comment 4

[Dominic Kuo [:dkuo]]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #3)
> Can you do it :) ?

Sure :)

Comment 5

[Dominic Kuo [:dkuo]]

Attachment: patch

This patch basically removes the hacks in bug 1051172, so two reviewers(system and camera) will be nice to check this patch, also David wrote the hack and hope he remembered it.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8628172 [details] [review]
patch

Thanks! David can review everything.

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8628172 [details] [review]
patch

The code looks good to me. I'm about to leave for an extended vacation, and am sort of rushing this review:

1) I have not tried to verify that this patch removes all of the code that it should remove

2) I have not tested this patch to verify that the camera stops recording quickly enough so that the incoming call or alarm sounds are not heard.

3) When I wrote the patch the System app did not have the current Services stuff, and I don't know anything about that, so I'm not completely comfortable reviewing that code.  Seems like it is probably okay though.

So this is good to land if we can verify (on a 319mb flame) that with this patch incoming calls and alarms stop recording before the sound begins to play.

Needinfo Andrew so he's aware of this change.

Comment 8

[Dominic Kuo [:dkuo]]

While I am cleaning up the patch I found there are also private.broadcast.* hacks under tv_apps/smart-system. John, since you are familiar with the tv system, do you think we can also remove the hacks in this patch? thanks.

Comment 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to Dominic Kuo [:dkuo] from comment #8)
> While I am cleaning up the patch I found there are also private.broadcast.*
> hacks under tv_apps/smart-system. John, since you are familiar with the tv
> system, do you think we can also remove the hacks in this patch? thanks.

Yes, I think so. Thanks for this.

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

(In reply to David Flanagan[PTO until July 24][:djf] from comment #7)
> Needinfo Andrew so he's aware of this change.

Thanks, noted.

Comment 11

[BMO Automation]

Firefox OS is not being worked on