Closed Bug 985817 Opened 8 years ago Closed 8 years ago

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

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla31

People

(Reporter: tatiana, Assigned: tatiana)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attachment #8393935 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #8393935 - Flags: feedback?(bugmail.mozilla) → feedback+
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
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
Attachment #8393935 - Attachment is obsolete: true
Attachment #8395208 - Flags: review?(bugmail.mozilla)
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.
Attachment #8395208 - Flags: review?(bugmail.mozilla) → review+
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
https://hg.mozilla.org/mozilla-central/rev/daada068c02f
Assignee: nobody → tanya.meshkova
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.