1396951 - Get rid of GeckoAppShell.getLayerView

Status: RESOLVED FIXED

(GeckoView :: General, defect)

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 1. Add and use HapticFeedbackDelegate;

Instead of using `getLayerView()` to perform haptic feedback, this patch
adds a `HapticFeedbackDelegate`, which `GeckoApplication` implements to
call `performHapticFeedback()` on the active view. Also, use
HapticFeedbackDelegate elsewhere in the Fennec codebase where we want to
perform haptic feedback.

Review commit: https://reviewboard.mozilla.org/r/182224/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182224/

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 2. Don't use getLayerView() in GeckoInputConnection;

In GeckoInputConnection, use the current view available through
`getView()`, instead of using `GeckoAppShell.getLayerView()`.

Review commit: https://reviewboard.mozilla.org/r/182226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182226/

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient;

Let `GeckoLayerClient` directly deal with scroll-to-input on resize,
instead of going through `GeckoAppShell` and relying on
`getLayerView()`. This is a necessary fix to let us remove
`getLayerView()`, and in a follow-up bug we should actually fix
scroll-to-input to work on any GeckoView.

Review commit: https://reviewboard.mozilla.org/r/182228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182228/

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 4. Pass in GeckoView instance when sending a11y event;

Pass in a `GeckoView` instance when sending a11y events so we're not
dependent on `GeckoAppShell.getLayerView()`. However, there's likely
more work to be done to make a11y work for any GeckoView.

Review commit: https://reviewboard.mozilla.org/r/182230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182230/

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 5. Remove getLayerView usage from Robocop code;

Find the Fennec LayerView through `Solo.getView()` and the View id
instead of going through `GeckoAppShell.getLayerView()`.

Review commit: https://reviewboard.mozilla.org/r/182232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182232/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1396951 - 6. Remove setLayerView/getLayerView;

Remove `GeckoAppShell.setLayerView()/getLayerView()` now that it's no
longer used anywhere.

Review commit: https://reviewboard.mozilla.org/r/182234/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182234/

Comment 7

[Geoff Brown [:gbrown]]

Comment on attachment 8910757 [details]
Bug 1396951 - 5. Remove getLayerView usage from Robocop code;

https://reviewboard.mozilla.org/r/182232/#review187560

Comment 8

[Randall Barker [:rbarker]]

Comment on attachment 8910755 [details]
Bug 1396951 - 3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient;

https://reviewboard.mozilla.org/r/182228/#review187578

Comment 9

[Eugen Sawin [:esawin]]

Comment on attachment 8910754 [details]
Bug 1396951 - 2. Don't use getLayerView() in GeckoInputConnection;

https://reviewboard.mozilla.org/r/182226/#review187594

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoInputConnection.java:365
(Diff revision 1)
>              mCursorAnchorInfoBuilder = new CursorAnchorInfo.Builder();
>          }
>          mCursorAnchorInfoBuilder.reset();
>  
>          // Calculate Gecko logical coords to screen coords
> -        final View v = getView();
> +        final GeckoView v = getView();

Use a more descriptive variable name: view.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8910753 [details]
Bug 1396951 - 1. Add and use HapticFeedbackDelegate;

https://reviewboard.mozilla.org/r/182224/#review187698

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8910756 [details]
Bug 1396951 - 4. Pass in GeckoView instance when sending a11y event;

https://reviewboard.mozilla.org/r/182230/#review187700

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8910758 [details]
Bug 1396951 - 6. Remove setLayerView/getLayerView;

https://reviewboard.mozilla.org/r/182234/#review187702

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910753 [details]
Bug 1396951 - 1. Add and use HapticFeedbackDelegate;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182224/diff/1-2/

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910754 [details]
Bug 1396951 - 2. Don't use getLayerView() in GeckoInputConnection;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182226/diff/1-2/

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910755 [details]
Bug 1396951 - 3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182228/diff/1-2/

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910756 [details]
Bug 1396951 - 4. Pass in GeckoView instance when sending a11y event;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182230/diff/1-2/

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910757 [details]
Bug 1396951 - 5. Remove getLayerView usage from Robocop code;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182232/diff/1-2/

Comment 18

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910758 [details]
Bug 1396951 - 6. Remove setLayerView/getLayerView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182234/diff/1-2/

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910753 [details]
Bug 1396951 - 1. Add and use HapticFeedbackDelegate;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182224/diff/2-3/

Comment 20

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910754 [details]
Bug 1396951 - 2. Don't use getLayerView() in GeckoInputConnection;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182226/diff/2-3/

Comment 21

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910755 [details]
Bug 1396951 - 3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182228/diff/2-3/

Comment 22

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910756 [details]
Bug 1396951 - 4. Pass in GeckoView instance when sending a11y event;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182230/diff/2-3/

Comment 23

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910757 [details]
Bug 1396951 - 5. Remove getLayerView usage from Robocop code;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182232/diff/2-3/

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910758 [details]
Bug 1396951 - 6. Remove setLayerView/getLayerView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182234/diff/2-3/

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e5ce5633017
1. Add and use HapticFeedbackDelegate; r=snorp
https://hg.mozilla.org/integration/autoland/rev/71134284dc36
2. Don't use getLayerView() in GeckoInputConnection; r=esawin
https://hg.mozilla.org/integration/autoland/rev/155c1005c388
3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient; r=rbarker
https://hg.mozilla.org/integration/autoland/rev/e9bc6e08091b
4. Pass in GeckoView instance when sending a11y event; r=snorp
https://hg.mozilla.org/integration/autoland/rev/b5a88815b3a6
5. Remove getLayerView usage from Robocop code; r=gbrown
https://hg.mozilla.org/integration/autoland/rev/0a367a71ca43
6. Remove setLayerView/getLayerView; r=snorp

Comment 26

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5e5ce5633017
https://hg.mozilla.org/mozilla-central/rev/71134284dc36
https://hg.mozilla.org/mozilla-central/rev/155c1005c388
https://hg.mozilla.org/mozilla-central/rev/e9bc6e08091b
https://hg.mozilla.org/mozilla-central/rev/b5a88815b3a6
https://hg.mozilla.org/mozilla-central/rev/0a367a71ca43

Comment 27

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequently failing robocop's testSettingsPages:

https://hg.mozilla.org/mozilla-central/rev/2cd3752963fc8f24f7c202687eab55e83222f608
https://hg.mozilla.org/mozilla-central/rev/ee035fc58bfa813da94d31d22a701c63cc283390
https://hg.mozilla.org/mozilla-central/rev/c24ad4171417a4d7bb10381152432794b5e03acd
https://hg.mozilla.org/mozilla-central/rev/866ee4c0900f538827535cf97070530e499878c8
https://hg.mozilla.org/mozilla-central/rev/ee9bdc65d7720dedf6e284ab36d76817c6c88211
https://hg.mozilla.org/mozilla-central/rev/d464f77195576407e0e77b9fe48981af154abbd7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0a367a71ca4381367346f9d600472d80205e72e3
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=132656726&repo=autoland
> TEST-UNEXPECTED-FAIL | testSettingsPages | Exception caught - junit.framework.AssertionFailedError: View with id: '2131296519', resource name: 'layer_view' is not found!

Comment 28

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910753 [details]
Bug 1396951 - 1. Add and use HapticFeedbackDelegate;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182224/diff/3-4/

Comment 29

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910754 [details]
Bug 1396951 - 2. Don't use getLayerView() in GeckoInputConnection;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182226/diff/3-4/

Comment 30

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910755 [details]
Bug 1396951 - 3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182228/diff/3-4/

Comment 31

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910756 [details]
Bug 1396951 - 4. Pass in GeckoView instance when sending a11y event;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182230/diff/3-4/

Comment 32

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910757 [details]
Bug 1396951 - 5. Remove getLayerView usage from Robocop code;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182232/diff/3-4/

Comment 33

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8910758 [details]
Bug 1396951 - 6. Remove setLayerView/getLayerView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182234/diff/3-4/

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa3c8eefecef
1. Add and use HapticFeedbackDelegate; r=snorp
https://hg.mozilla.org/integration/autoland/rev/a39d4bbc8306
2. Don't use getLayerView() in GeckoInputConnection; r=esawin
https://hg.mozilla.org/integration/autoland/rev/4791f0377e29
3. Move GeckoAppShell.viewSizeChanged() to GeckoLayerClient; r=rbarker
https://hg.mozilla.org/integration/autoland/rev/1dd2fccf441f
4. Pass in GeckoView instance when sending a11y event; r=snorp
https://hg.mozilla.org/integration/autoland/rev/4a111afcd561
5. Remove getLayerView usage from Robocop code; r=gbrown
https://hg.mozilla.org/integration/autoland/rev/4324d70a4fc5
6. Remove setLayerView/getLayerView; r=snorp

Comment 35

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fa3c8eefecef
https://hg.mozilla.org/mozilla-central/rev/a39d4bbc8306
https://hg.mozilla.org/mozilla-central/rev/4791f0377e29
https://hg.mozilla.org/mozilla-central/rev/1dd2fccf441f
https://hg.mozilla.org/mozilla-central/rev/4a111afcd561
https://hg.mozilla.org/mozilla-central/rev/4324d70a4fc5