Closed Bug 1044744 Opened 9 years ago Closed 9 years ago

Move IFrame management in keyboard_manager.js to another script


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

Gonk (Firefox OS)
Not set


(Not tracked)

2.1 S2 (15aug)


(Reporter: mnjul, Assigned: mnjul)



(Whiteboard: [p=3])


(1 file)

Let's move functionalities related to the management of keyboard iframes (one keyboard "app" has an iframe) in from system/js/keyboard_manager.js to another script.
Assignee: nobody → jlu
Whiteboard: [p=3]
Attached file Patch (PR @ GitHub)
Let's have TBPL test the patch I've been working on.
Comment on attachment 8464612 [details] [review]
Patch (PR @ GitHub)

Hi Tim,

Could you review this patch? It's passed all TBPL tests except for the Linux x64 Debug Gip that's failing for other PRs too.

In this patch, any IFrame-related chores (creation, destruction, book-keeping, visibility, ...etc) have been moved to KeyboardFrameManager. The rationale is that KeyboardManager should not know of any iframe; KFM receives references of "layout" (that has manifestURL, id, path, ... as kept in runningLayouts), and it book-keeps the mapping from [layout.manifestURL,] to iFrame in its runningLayouts.

As KM no longer needs to know frames, its 'runningLayouts' structure has been modified to {manifestURL1: Set([id, id, id, ...]), manifestURL2: Set([id, id, id, ...]), ...}. Any logics in KM that used frames to decide "which was the old layout during layout switch" and alike would now use layouts to carry out the decision. To satisfy the need to know the showing 'layout'[1], KM's 'showingLayout' would now carry information to 'layout'[1] instead of 'frame'. Note that in before, 'showingLayout' isn't actually a layout[1], which could cause confusion now that KM and KFM relies on layouts more heavily to operate. To avoid the confusion, 'showingLayout' in KM has been renamed to 'showingLayoutInfo'. 

KFM exposes six functions (aside from constructor and start/stop) to KM:
1. launchFrame & destroyFrame: launchFrame is used in KM's setKeyboardToShow() and it either [constructs a frame for a layout that hasn't been constructed], or [reuses a frame for a layout that has the same keyboard's frame already constructed]; destroyFrame is used in KM's removeKeyboard().
2. setupFrame & resetFrame: used when a constructed frame is to be switched-in/shown or to be switched-out/hidden.
3. deleteRunningKeyboardRef & deleteRunningFrameRef: also used in KM's removeKeyboard().

KM has three functions for KFM to call:
1. resizeKeyboard, called when a frame receives resize event
2. setHasActiveKeyboard, which KFM's setFrameActive() relays the activeness of any keyboard
3. deleteRunningLayout, which updates KM's runningLayouts book-keeping when a frame is being reused as decided by KFM.
Some codes in KM have been abstracted into functions too.
KM also has some options and frameContainer to KFM to use.

[1] I mean the data structure with manifestURL, id, path, ...

I think after this patch I will still poke into the flow of "switching keyboards vs. frame generation & showing/hiding" between KM and KFM and see if it can be more streamlined and the crossings between KM/KFM can be reduced and/or streamlined.
Attachment #8464612 - Flags: review?(timdream)
Attachment #8464612 - Attachment description: WIP Patch (PR @ GitHub) → Patch (PR @ GitHub)
Target Milestone: --- → 2.1 S2 (15aug)
Comment on attachment 8464612 [details] [review]
Patch (PR @ GitHub)

Very sorry for the delay. Please read Github comments and amend the patch accordingly. I think we can land this and address anything quirky as follow-ups.
Attachment #8464612 - Flags: review?(timdream) → review+
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 1052272
You need to log in before you can comment on or make changes to this bug.