880024 - Create a local version of GeckoContentController

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Currently we have RemoteContentController which talks to TabChild. TabChild contains a bunch of logic that needs to be shared with LocalContentController.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

If we want the dynamic toolbar to work as described at https://bugzilla.mozilla.org/show_bug.cgi?id=860812#c7 then we will need to have an APZC built for the browser chrome layer, which exists in the root process. See https://bugzilla.mozilla.org/show_bug.cgi?id=898478#c3 for some more backstory/design.

Whether we turn on APZCs for all the layers or just the one layer in the root process, we will need an implementation of GeckoContentController to hand to that APZC, and that lives in the root process (hence it's a "LocalContentController"). I believe this should live in the widget code (probably have a base implementation in widget/xpwidgets and then have a specific subclass in widget/gonk for the dynamic toolbar).

This implementation will need to do all the usual things a GeckoContentController does - that is, pass on detected gestures to content, request content repaints, schedule runnables for later, and so on.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: WIP

I decided that the cleanest way to do this was to just extract a class with static helper methods to hold the reusable bits of the code. That way the specific implementations of GeckoContentController aren't bound to a particular class hierarchy.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Extract some code from TabChild

Updated with some documentation and tweaks.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Extract some code from TabChild

Rebased

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 805590 [details] [diff] [review]
Extract some code from TabChild

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

::: dom/base/nsDOMWindowUtils.cpp
@@ +1523,5 @@
> +  } else {
> +    if (aRetVal) {
> +      *aRetVal = false;
> +    }
> +  }

Suggestion: I would probably write outside the if:

if (aRetVal) {
  *RetVal = sf != nullptr;
}

::: widget/xpwidgets/APZCCallbackHelper.cpp
@@ +67,5 @@
> +    nsCOMPtr<nsIDOMElement> element;
> +    if (content) {
> +      element = do_QueryInterface(content);
> +    }
> +    if (element) {

Suggestion: element is only set in one place so we could just move everything inside the 'if (content)' block. However I'd probably use gating:

nsCOMPtr<nsIContent> content = nsLayoutUtils::FindContentFor(aMetrics.mScrollId);
if (!content) {
  return;
}

nsCOMPtr<nsIDOMElement> element = do_QueryInterface(content);
if (!element) {
  return;
}

...

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Landed with the suggestions implemented, thanks!

https://hg.mozilla.org/integration/b2g-inbound/rev/2b836100c277

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2b836100c277