1189875 - Move NativePanZoomController JNI methods out of AndroidJNI.cpp

Status: RESOLVED DUPLICATE of bug 1238761

(Firefox for Android Graveyard :: Toolbar, defect)

Description

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

Similar to bug 1182641 and bug 1186467, we want to move NativePanZoomController JNI methods out of AndroidJNI.cpp and into an appropriate class. The result will be when the JNI methods get called, the calls will automatically translate to calls on members of that C++ class.

Comment 1

[Mark Capella [:capella]]

Looked at this briefly ... wondering about |sNativePanZoomController| being stored in AndroidContentController ... 

   Does this need to be kept as an actual instance (vs a simple bool) as it seems you're only using it to flag for "previously init-ed"?
   Does it need to be kept in AndroidContentController, or can we now move it to NativePanZoomController.h/.cpp ?

Also:

   Do this need a NativePanZoomController::Init() call in AndroidBridge.cpp?
   Wouldn't this require a rename of the java init() method in NativePanZoomController.java to avoid conflict?


Also, I realize you're active in this area ... (it's interesting to watch) ...

Let me know if you intended to just knock this out yourself  :-)

Comment 2

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

Thanks for looking at this :)

WRT AndroidContentController, I think :kats would be the best person to answer your questions. I'm not even sure what the plan is going forward since a lot of the NativePanZoomController methods look to be unimplemented.

Yes, NativePanZoomController::Init would probably go in AndroidBridge.cpp. We could rename init to nativeInit() or something. There are C++ tricks to avoid renaming too.

I'll probably not work on a lot of these bugs. Feel free to tinker with it! :)

Comment 3

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

(In reply to Mark Capella [:capella] from comment #1)
> Looked at this briefly ... wondering about |sNativePanZoomController| being
> stored in AndroidContentController ... 
> 
>    Does this need to be kept as an actual instance (vs a simple bool) as it
> seems you're only using it to flag for "previously init-ed"?

It can probably be gotten rid of entirely (along with the init/destroy methods). We used to need it, but don't any more. If we need it in the future we can add it back in a better place.

(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> WRT AndroidContentController, I think :kats would be the best person to
> answer your questions. I'm not even sure what the plan is going forward
> since a lot of the NativePanZoomController methods look to be unimplemented.

For now NativePanZoomController will continue to exist. There's only a handful of methods which are unimplemented, and those will probably stay unimplemented for now. They're largely hooks that the Java PZC code exposes to GeckoLayerClient but shouldn't be needed after the switch to APZ.

Comment 4

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

Technically this doesn't block bug 776030.