1133569 - [e10s] Back/forward gesture causes "unsafe CPOW usage" warning

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

STR:

1) In a remote browser, do some browsing so that you have some session history
2) Use the "back" gesture to go back a page. On my MacBook, that's 3-finger swipe left, but it might be different for other folk.
3) Use the "forward" gesture to go forward to where you came from.

This causes some "unsafe CPOW usage" warnings in the Browser Console at the following locations:

in browser/base/content/browser.js:

  /**
   * Starts the swipe animation and handles fast swiping (i.e. a swipe animation
   * is already in progress when a new one is initiated).
   *
   * @param aIsVerticalSwipe
   *        Whether we're dealing with a vertical swipe or not.
   */
  startAnimation: function HSA_startAnimation(aIsVerticalSwipe) {
    this._direction = aIsVerticalSwipe ? "vertical" : "horizontal";

    if (this.isAnimationRunning()) {
      // If this is a horizontal scroll, or if this is a vertical scroll that
      // was started while a horizontal scroll was still running, handle it as
      // as a fast swipe. In the case of the latter scenario, this allows us to
      // start the vertical animation without first loading the final page, or
      // taking another snapshot. If vertical scrolls are initiated repeatedly
      // without prior horizontal scroll we skip this and restart the animation
      // from 0.
      if (this._direction == "horizontal" || this._lastSwipeDir != "") {
        gBrowser.stop();
        this._lastSwipeDir = "RELOAD"; // just ensure that != ""
        this._canGoBack = this.canGoBack();
        this._canGoForward = this.canGoForward();
        this._handleFastSwiping();
      }
    }
    else {
      this._startingIndex = gBrowser.webNavigation.sessionHistory.index; <-- Causes CPOW warning
      this._historyIndex = this._startingIndex;
      this._canGoBack = this.canGoBack();
      this._canGoForward = this.canGoForward();
      if (this.active) {
        this._addBoxes();
        this._takeSnapshot();
        this._installPrevAndNextSnapshots();
        this._lastSwipeDir = "";
      }
    }
    this.updateAnimation(0);
  },

and later in the same file:

  /**
   * Stops the swipe animation.
   */
  stopAnimation: function HSA_stopAnimation() {
    gHistorySwipeAnimation._removeBoxes();
    this._historyIndex = gBrowser.webNavigation.sessionHistory.index; <-- Causes CPOW warning
  },

Comment 1

[Florian Bender]

Currently, the swipe gesture works anywhere that is not scrollable, e.g. the toolbar, findbar, (outer) window, or the top of the sidebar (but not sidebar content or tabbar since they could scroll). However it *should* only work within the content. 

Reworking this to only work in content may fix this issue since swipe detection can happen in the content process and only the navigation event needs to be reported to the main thread. This may or may not help with Bug 927702, too.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

I am not seeing this unsafe CPOW usage warning now, but I want to dig into _why_ this got fixed...

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Nope, I'm wrong. This is still around.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Re-nomming - this is definitely still around.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Stealing because gw280 is on PTO.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hey Neil - as you're already in progress fixing bug 1148505, do you think you'd also like to take this bug as well?

Comment 7

[Neil Deakin]

Attachment: patch?

This is a patch that just uses the session history instead. This won't be 100% accurate is some cases.

I can't get the gesture to work very reliably at all anyway, so it's hard to tell if this patch works or not. I've spent too much time on this already.

Comment 8

[Neil Deakin]

Attachment: Updated patch

This patch gets the updated history index from session store. It seems to work ok for me. The animation mode, disabled by default, doesn't work anyway (bug 1170032) so I didn't particularly test it.

Comment 9

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/696662fec7bf036aa62d259d08bcaceb055c4ac2
Bug 1133569, remove cpow usage from back/forward gesture, r=felipe

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/696662fec7bf