985817 - Move some TabChild functionality into base class to be reusable in Embedlite implementation

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Tatiana Meshkova (:tatiana)]

Embedlite currently duplicate lot's of TabChild functionality https://github.com/tmeshkova/gecko-dev/blob/embedlite_26/embedding/embedlite/utils/TabChildHelper.cpp

We want to share as much functionality as possible and reuse some parts especially "viewport stuff" between b2g/WinMetro and Embedlite.

Comment 1

[Tatiana Meshkova (:tatiana)]

Attachment: Split TabChild functionality into reusable class

Comment 2

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

Comment on attachment 8393935 [details] [diff] [review]
Split TabChild functionality into reusable class

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

This seems reasonable to me, but it needs more documentation about what belongs in TabChild vs TabChildBase. For instance, if I were to add some more stuff how do I know whether it belongs in TabChild or TabChildBase? I don't want to have to go look at the Embedlite code every time.

Comment 3

[Tatiana Meshkova (:tatiana)]

yep this is very reasonable question, which is hard to answer as is. straight answer is TabChildBase should keep functionality which would be shareable between IPC/ITC(OMTC) implementations. For know we have found hot space is Viewport related functionality and relevant APZC changes. so I want to move it in order to prevent hard time of merges Embedlite branch with upstream.

It would be nice to move UpdateTapState relevant functionality to TabChildBase too, but it is in my TODO list still.
I will try to formulate description of TabChildBase class purpose and move all TabChildBase in single place of TabChild.cpp file. if you want I can event try to move TabChildBase class into separate files

Comment 4

[Tatiana Meshkova (:tatiana)]

Attachment: Split TabChild functionality into reusable class. v2

This is the same version with functionality moved up. plus comment added.
let me know if you want TabChildBase to be moved into separate file

Comment 5

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

Comment on attachment 8395208 [details] [diff] [review]
Split TabChild functionality into reusable class. v2

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

This looks ok to me, but I didn't go through each function to ensure the code wasn't modified. Did you make any changes to the code when shuffling it around? I noticed you added a UpdateFrameHandler helper function which should be fine. Also please push this to try with "-p all -u all -t none" to ensure it doesn't break anything before landing. r=me with the comment updated and question answered.

::: dom/ipc/TabChild.h
@@ +145,5 @@
>  
> +// This is base clase which helps to share Viewport and touch related functionality
> +// between b2g/android FF/embedlite clients implementation.
> +// puth if this class all helper functions, and functionality which could be shared between
> +// Cross process/Cross thread implmentations.

This comment needs to be expanded a bit. Also "puth if this class" doesn't make much sense.

Comment 6

[Tatiana Meshkova (:tatiana)]

See previous version of the patch, it does not have any functional changes, except return values for nsEventStatus DispatchWidgetEvent and HandlePossibleViewportChange.
Filled account reactivation bug 987488, in the meantime
https://tbpl.mozilla.org/?tree=Try&rev=c1fc368b5371

Comment 7

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

It looks like you landed this on inbound also:

http://hg.mozilla.org/integration/mozilla-inbound/rev/daada068c02f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/daada068c02f