Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] total_uri_count not incremented for existing tabs #6126

Closed
severinrudie opened this issue Oct 18, 2019 · 5 comments
Closed

[Bug] total_uri_count not incremented for existing tabs #6126

severinrudie opened this issue Oct 18, 2019 · 5 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Telemetry P2 Upcoming release

Comments

@severinrudie
Copy link
Contributor

severinrudie commented Oct 18, 2019

Steps to reproduce

  • Navigate to any website in Fenix
  • Leaving the tab open, swipe close Fenix
  • Reopen Fenix
  • Select your previous tab
  • Select the nav bar
  • Navigate to a new site

Expected behavior

  • total_uri_count event should be sent when the site finishes loading

Actual behavior

  • total_uri_count event is never sent
    (NOTE that the event works from tabs created during the current session)

Device information

  • Android device: Pixel 2 on Android 10. Reproduced on Pixel 2 emulator, API 28
  • Fenix version: Current head of master (ba086a8)

Verification notes

Verified by logging in UriOpenedObserver#onLoadingStateChanged, which is currently the only place total_uri_count events are sent

Investigation notes (engineering)

  • When navigating to an existing tab then clicking links without searching, UriOpenedObserver is never instantiated
  • When navigating to an existing tab then searching, UriOpenedObserver is instantiated, but it does not appear that UriOpenedObserver.singleSessionObserver is ever registered

┆Issue is synchronized with this Jira Task

@codrut-topliceanu
Copy link
Contributor

Might be fixed by #6577 pending QA verification.

@BranescuMihai
Copy link
Contributor

Even though the actual total_uri_count count might be fixed by #6577, the underlying issue with the UriOpenedObserver remains, and I detailed the problem here: #9816

@vesta0 vesta0 removed this from General in Workflow Apr 20, 2020
@vesta0 vesta0 added this to Prioritized Bug Backlog in Fenix Sprint Kanban Apr 20, 2020
@vesta0 vesta0 added P2 Upcoming release and removed P1 Current sprint labels Apr 28, 2020
@AndiAJ
Copy link
Collaborator

AndiAJ commented Jun 12, 2020

Hi @baron-severin I've just re-checked this matter on Nightly Build 200611 from 6/11 using a Google Pixel 3a (Android 10) and it's still reproducible

Since you've reported this matter the "events.total_uri_count" is now displayed only in the metrics ping

As per your STR, I've:

  1. Navigated to theverge.com
  2. Swipe closed Fenix having the tab opened
  3. Resumed Fenix and navigated to cnet.com

The Metrics Ping 2b9472e4-7e2f-45fd-9c75-cbbbc54d3060

"counter": {
          "events.total_uri_count": 2

Logcat
Glean dashboard

@ekager & @BranescuMihai could you please review and share your thoughts ? ☺️

@BranescuMihai
Copy link
Contributor

@AndiAJ I think it should have been 3: 2 because you navigated twice to theverge.com and 1 for cnet.com.

From engineering perspective, the init of the sessionObserver is not called from the TabTrayDialogFragment, I'll add a fix for that

@BranescuMihai BranescuMihai self-assigned this Jun 12, 2020
@BranescuMihai BranescuMihai added this to In Progress in Hershey's 🍫 Jun 12, 2020
BranescuMihai added a commit to BranescuMihai/fenix that referenced this issue Jun 12, 2020
This helps because we will always need the observer to be initiated, not only when the `openToBrowser` method gets called. Example: Opening a tab from the tab tray had it's own method for opening the browser, causing this to not be called.
BranescuMihai added a commit that referenced this issue Jun 15, 2020
This helps because we will always need the observer to be initiated, not only when the `openToBrowser` method gets called. Example: Opening a tab from the tab tray had it's own method for opening the browser, causing this to not be called.
@BranescuMihai BranescuMihai moved this from In Progress to QA Review in Hershey's 🍫 Jun 15, 2020
@BranescuMihai BranescuMihai moved this from Prioritized Bug Backlog to Ready for QA in Fenix Sprint Kanban Jun 15, 2020
@BranescuMihai BranescuMihai added the eng:qa:needed QA Needed label Jun 25, 2020
@AndiAJ
Copy link
Collaborator

AndiAJ commented Jun 25, 2020

Hi, verified as fixed on the latest Nightly Build 200625 from 6/25 using a Google Pixel 3a (Android 10)

✔️ 1st scenario

  1. Navigated to theverge.com
  2. Swipe close Fenix
  3. Resume Fenix select the previous tab (theverge.com)
  4. Tap the Nav bar navigate to cnet.com

Ping - 21c39440-c85f-4ed9-93c3-d40fe704ecbb

"counter": {
          "events.total_uri_count": 3

Logcat
Glean dashboard

✔️ 2nd scenario

  1. Navigated to theverge.com
  2. Swipe close Fenix
  3. Resume Fenix select the previous tab (theverge.com)
  4. Swipe close Fenix
  5. Resume Fenix select the previous tab (theverge.com)
  6. Tap the Nav bar navigate to cnet.com
  7. Swipe close Fenix
  8. Resume Fenix select the previous tab (cnet.com)
  9. Swipe close Fenix
    10 Resume Fenix select the previous tab (cnet.com)

Ping - e56b5aed-78da-415a-bdf1-17ede7b67870

"counter": {
          "events.total_uri_count": 6

Logcat
Glean dashboard

@AndiAJ AndiAJ closed this as completed Jun 25, 2020
Fenix Sprint Kanban automation moved this from Ready for QA to Sprint 20.11 Done Jun 25, 2020
@AndiAJ AndiAJ removed the eng:qa:needed QA Needed label Jun 25, 2020
@AndiAJ AndiAJ added the eng:qa:verified QA Verified label Jun 25, 2020
@mcarare mcarare moved this from QA Review to Done in Hershey's 🍫 Jul 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Telemetry P2 Upcoming release
Projects
Fenix Sprint Kanban
  
Sprint 20.11 Done
Development

No branches or pull requests

7 participants