Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Multi-window Telemetry #1443

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

keianhzo
Copy link
Collaborator

Added telemetry events from #1329

@MortimerGoro
Copy link
Contributor

/opt/FirefoxReality/app/src/main/res/layout/context_menu_item.xml:15: AAPT: error: resource drawable/context_menu_item_background_color (aka org.mozilla.vrbrowser:drawable/context_menu_item_background_color) not found.
error: failed linking file resources.

Copy link
Contributor

@daoshengmu daoshengmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you want to queue your multiple window telemetry at onDestory()?

@keianhzo keianhzo force-pushed the experimental/mw_telemetry branch 2 times, most recently from 40017bc to 8bd10c8 Compare July 23, 2019 13:56
@keianhzo keianhzo force-pushed the experimental/mw_telemetry branch 2 times, most recently from af4db93 to 9a95e61 Compare July 24, 2019 14:57
@keianhzo
Copy link
Collaborator Author

@daoshengmu I've added some logs so all the multi-window related telemetry can be seen at the end of the session. Let me know if you catch anything.

Copy link
Contributor

@daoshengmu daoshengmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure and I might be wrong. If we queue histogram is onDestroy, we will too late to do scheduleUpload our metric. We need to confirm it. Other parts are LGTM.

@daoshengmu
Copy link
Contributor

We probably will start to replace our existing telemetry service to Glean in Q3, because the telemetry service will not be maintained soon. We need to keep an eye on it for the coming changes.

@keianhzo keianhzo force-pushed the experimental/mw_telemetry branch 2 times, most recently from 766557a to 3f23f46 Compare July 31, 2019 12:29
@keianhzo
Copy link
Collaborator Author

@daoshengmu Just FYI I've removed the the Telemetry queueing from start as it’s always empty at that point (we always flush in on stop) so it doesn’t make sense to queue it. I've just removed the data queuing NOT the upload scheduling, we still need that to solve the boot permission issues.

@daoshengmu
Copy link
Contributor

@daoshengmu Just FYI I've removed the the Telemetry queueing from start as it’s always empty at that point (we always flush in on stop) so it doesn’t make sense to queue it. I've just removed the data queuing NOT the upload scheduling, we still need that to solve the boot permission issues.

I just took a look at queueHistogram(), it will write file stream into a storage. Every bucket is 0 value from start, so it will not affect the final result. But, they are still redundant data.

According to #1353 (comment), our BOOT permission will only cause the regression of the job scheduler. So, I agree we can remove the queueHistogram() at start. Thanks.

@keianhzo keianhzo merged commit 5ed0f24 into experimental/multiwindow Aug 1, 2019
MortimerGoro pushed a commit that referenced this pull request Aug 1, 2019
@bluemarvin bluemarvin deleted the experimental/mw_telemetry branch August 1, 2019 17:47
MortimerGoro pushed a commit that referenced this pull request Aug 2, 2019
keianhzo added a commit that referenced this pull request Aug 2, 2019
bluemarvin pushed a commit that referenced this pull request Aug 2, 2019
* Remove unused code

* Sessions Multiwindow implementation

Refactor of the current sessions code and added support for multi window.

* Implement window placement logic

* Refactored session settings to SessionManager

Always update all session when a session settings changes

* Keyboard multiwindow placement

* Added support for browser context menus

* Context Menu integration

* Fix parenting bug with three window moves

* Fix relayout on window resize. Set up better anchor points for left/right window resizes.

* Fixed Bookmarks navigation

Also added support for bookmarks in all the windows as stated in the spec: #1331

* Close sessions when a window is closed

Now when a window it's closed it's underlying sessions are closed.

* Add new tray icons

* Session restore


wip

* Fix resize gesture flickering and conflicts.

* Added support for restoring private sessions

* Save and restore window sizes

* Set up max reside sizes for multiwindow

* Exit fullscreen when window is closed

Also fixed a regression that made session not to close when a window is closed

* Fix fullscreen mode: hide other windows and make the fullscreened window the front one

* Fixed ADB issue and double loadUri when restoring windows.

* Make max windows dialog match  spec

* Curved mode fixes

* Fix rebase issues

* Fix cylindrical mode math for multiwindow.

* Fixed performance monitor from rebase and added missing tooltips

* Rebase fixes

* New Settings spec rebase fixes

* Added User-Agent multi-window support and fixed session restore issues

The home page was being loaded after a session restore. Now the home page is only loaded if the session is not restored. Also fixed some session restore issues to also restore session settings.

* Fix Tasckluster NoApi compilation issue

* Fixed context menu issues

* More ContextMenu fixes

* Fixed move window buttons get stuck in hover state

Fixes #1482

* Disable left/right move buttons when in private mode single window

* Multi-window Telemetry (#1443)

* Remove unneeded switchBookmarks call which causes a surface resize each time windows are updated

* Dime out world when closing a window

* Rebase fixes

* SessionStore rename

* Fix for session restore in WaveVR

* Fix movable keyboard for multiwindow

* Rebase fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants