942790 - Re-enable keyboard oop

Status: RESOLVED FIXED

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

Description

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

+++ This bug was initially created as a clone of Bug #941885 +++

We ran into platform issue bug 942788 when switching keyboard oop on. This is somehow unfortunate because the regression happened around the same time. After that bug is resolved we need to re-enable keyboard oop on master branch to unlock development.

Comment 1

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#14037

Comment 2

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

Comment on attachment 8338202 [details] [review]
mozilla-b2g:master PR#14037

This must land AFTER revert of bug 917416 reaches m-c.

Comment 3

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8338202 [details] [review]
mozilla-b2g:master PR#14037

Thanks.

Comment 4

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

master: https://github.com/mozilla-b2g/gaia/commit/afa10908940f8d84f9e0477deeeba6569bf780a1

Comment 5

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

Jason, this has nothing to do with bug 930402.

Comment 6

[[:fabrice] Fabrice Desré]

Tim, does that mean that we now run all keyboards OOP? If so, I first want performance timings without OOP to make sure we don't regress what is already a pain point performance-wise.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #5)
> Jason, this has nothing to do with bug 930402.

No. This was marked on this bug because it was dependent on having 3rd party keyboard enabled. Re-adding the dependency.

Comment 8

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

(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #5)
> > Jason, this has nothing to do with bug 930402.
> 
> No. This was marked on this bug because it was dependent on having 3rd party
> keyboard enabled. Re-adding the dependency.

Jason, you really need to read the context of both bugs.

This bug enables 3rd-party keyboard along with oop. Enabling this feature (literally, "allowing 3rd-party keyboard app to be installed") has nothing to do with the behavior of the default keyboard app (bug 930402), and the two fixes have no dependency at all. I am not sure what the dependency you are taking about since I am taking care of the both bugs and I am suppose the person to understand everything.

I appreciate your time and effort to touch every bug and make sure they are directed the proper persons, but it would be better if you give more rationale when tagging bugs.

If you are looking at tagging bug with "hey, this is related to overall 3rd-party keyboard support work", or "hey, this behavior change is related to 3rd-party keyboard support work", there is a whiteboard flag and bug 816869 for that.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #8)
> (In reply to Jason Smith [:jsmith] from comment #7)
> > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> > comment #5)
> > > Jason, this has nothing to do with bug 930402.
> > 
> > No. This was marked on this bug because it was dependent on having 3rd party
> > keyboard enabled. Re-adding the dependency.
> 
> Jason, you really need to read the context of both bugs.

Everyone in triage read bug 930402 to be applicable to third-party keyboards - the bug title in the other bug said "[e.me] Support for 3rd party keyboards." That immediately reads off as a requirement as part of third-party keyboard work.

> 
> This bug enables 3rd-party keyboard along with oop. Enabling this feature
> (literally, "allowing 3rd-party keyboard app to be installed") has nothing
> to do with the behavior of the default keyboard app (bug 930402), and the
> two fixes have no dependency at all. I am not sure what the dependency you
> are taking about since I am taking care of the both bugs and I am suppose
> the person to understand everything.

Then next time I would suggest responding to a inquiry when someone asks your for team's input to triage something (https://bugzilla.mozilla.org/show_bug.cgi?id=930402#c5). We had to make a decision here with low context because repeated inquiries did not get us information we needed from your team. As a result, we had make a decision based on what people knew in that triage session.

I also do not see how the original definition of bug 930420 has anything to do with default keyboard alone - the discussion in the bug is talking heavily about differences with introduction of third-party keyboards. The bug has been morphed however at this point, so it's literally confusing to anyone what's being tracked there at this point.

> 
> I appreciate your time and effort to touch every bug and make sure they are
> directed the proper persons, but it would be better if you give more
> rationale when tagging bugs.

That was a triage decision. The people in triage had to make a decision with the context we knew. We did ask for input, but you didn't respond.

Comment 10

[Andreas Gal :gal]

Could OOP keyboard help performance by allowing parallel painting on dual core devices?

Comment 11

[[:fabrice] Fabrice Desré]

(In reply to Andreas Gal :gal from comment #10)
> Could OOP keyboard help performance by allowing parallel painting on dual
> core devices?

Maybe, but we'll also have slightly slower IPC (and we do two much of that - 8 async js messages per touch tap) so I'd like to get Eiderticker measurements.

Comment 12

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

(In reply to Jason Smith [:jsmith] from comment #9)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> > This bug enables 3rd-party keyboard along with oop. Enabling this feature
> > (literally, "allowing 3rd-party keyboard app to be installed") has nothing
> > to do with the behavior of the default keyboard app (bug 930402), and the
> > two fixes have no dependency at all. I am not sure what the dependency you
> > are taking about since I am taking care of the both bugs and I am suppose
> > the person to understand everything.
> 
> Then next time I would suggest responding to a inquiry when someone asks
> your for team's input to triage something
> (https://bugzilla.mozilla.org/show_bug.cgi?id=930402#c5). We had to make a
> decision here with low context because repeated inquiries did not get us
> information we needed from your team. As a result, we had make a decision
> based on what people knew in that triage session.

For the record, I did provide an explanation on bug 930402 comment 9 asking e.me to move away from relying on keyboard.current mozSetting value. Given the fact Ran decline to do that, I morph the bug in bug 930402 comment 18 so that built-in keyboard could continue provide the information to e.me legacy code.

It's reasonable to say I didn't provide enough information between bug 930402 comment 5 to  bug 930402 comment 8, but it's not reasonable say the cause of confusion on flipping flags in comment 7 is because I did not provide enough information (note the date).

Now, can we get back to work?

Comment 13

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

Public update per comment 6, comment 10, and comment 11: an e-mail has been sent to loop Jed and William on possible performance issues. Bugs to be file if they are.

Comment 14

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12)
> For the record, I did provide an explanation on bug 930402 comment 9 asking
> e.me to move away from relying on keyboard.current mozSetting value. Given
> the fact Ran decline to do that, I morph the bug in bug 930402 comment 18 so
> that built-in keyboard could continue provide the information to e.me legacy
> code.
> 
> It's reasonable to say I didn't provide enough information between bug
> 930402 comment 5 to  bug 930402 comment 8, but it's not reasonable say the
> cause of confusion on flipping flags in comment 7 is because I did not
> provide enough information (note the date).

Re-read bug 930402 I realized I had given the same information to Ran ~1 month ago in bug 930402 comment 2. I don't understand why triage session failed to read that.

Comment 15

[Jason Smith [:jsmith]]

This patch caused a serious performance hit to all of our keyboard UI automation - the keyboard is now taking way too long too load across the entire phone. We need to back this out.

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

[master 03e6e87] Revert "Merge pull request #14037 from timdream/keyboard-oop-re

Comment 17

[Jason Smith [:jsmith]]

Given the fact that we're putting forth a heavy effort to prevent crash landings in the last two weeks of 1.3 feature dev, I think this needs to wait until 1.4. We've already seen some pretty bad fallout per comment 15 from the followup attempt of enabling oop for 3rd party keyboards. This also is not a committed feature for 1.3.

Comment 18

[Andreas Gal :gal]

Jason, OOP keyboard/keyboard framework was already committed for 1.2. We can discuss slipping this again, but thats of course really not great (but we will if there is no other way). Could you and Tim talk directly and discuss the risks and a test plan? I would like the two of you to make a joint recommendation whether we can salvage the OOP work for 1.3 and what the concerns and risks are. Thanks!

Comment 19

[Jason Smith [:jsmith]]

(In reply to Andreas Gal :gal from comment #18)
> Jason, OOP keyboard/keyboard framework was already committed for 1.2. We can
> discuss slipping this again, but thats of course really not great (but we
> will if there is no other way). Could you and Tim talk directly and discuss
> the risks and a test plan? I would like the two of you to make a joint
> recommendation whether we can salvage the OOP work for 1.3 and what the
> concerns and risks are. Thanks!

Sure. I'll followup with Tim offline.

Comment 20

[[:fabrice] Fabrice Desré]

Also, just trust the Sheriff...

Comment 21

[Jason Smith [:jsmith]]

Moving to 1.4? per offline discussion - we'll be keeping this preffed off for 1.3.

Comment 22

[Ivan Tsay (:ITsay)]

Plus it for v1.4 as the committed feature.

Comment 23

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Log some test result here, after testing a local build with the patch in bug 944397 applied, we still could reproduce the case that sometimes the built-in would be shown after you click on an input field.

BTW, according to William, the automation test could pass.
--

Repro steps for the above issue
==============================
1. Try to launch several apps, like contacts, browser, gallery, etc.
2. Use b2g-ps to see when keyboard app is OOMed.
3. Go to an input field and click on it => sometimes the keyboard would not show up.

Comment 24

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Stealing this bug from Tim.

Comment 25

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Rudy Lu [:rudyl] from comment #23)
> Log some test result here, after testing a local build with the patch in bug
> 944397 applied, we still could reproduce the case that sometimes the
> built-in would be shown after you click on an input field.
> 
> BTW, according to William, the automation test could pass.
> --
> 
> Repro steps for the above issue
> ==============================
> 1. Try to launch several apps, like contacts, browser, gallery, etc.
> 2. Use b2g-ps to see when keyboard app is OOMed.
> 3. Go to an input field and click on it => sometimes the keyboard would not
> show up.

Thanks Rudy,

This is intermittent issue, it does not always occur but keyboard would not show up in low percentage after OOM.

I am trying to investigate the root cause.

Comment 26

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

After further testing, Bug 950573 is the root cause for both symptoms:
 1. As comment 24 stated, the keyboard would not show up. 
 2. As Bug 950573's original description, the keyboard would show up but the input function does not work.

So, I guess we are blocked by Bug 944397 only to re-enable keyboard OOP.

Comment 27

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

William, do you support the above estimation? Should we attempt to re-enable keyboard oop this week? If QA is confident on this I would like to have this resolved before end-of-year. Thanks!

Comment 28

[William Hsu [:whsu]]

Hi, Tim,

Sure. If we have solved all block issues of keyboard OOP, I suggest to re-enable it on V1.4 since we can stabilize it in early stage.

Many thanks!

Comment 29

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

I will re-land this bug today so it could hit TBPL tonight and smoketest tomorrow. Hopefully we could get this working this time.

Comment 30

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

master: ef87cccd69aba806b4fc5bc79991bf9a7132714e

Comment 31

[Ivan Tsay (:ITsay)]

Move 3rd party keyboard feature meta to bug 964670

Comment 32

[Ivan Tsay (:ITsay)]

Move depending to bug to correct use story bug.