Open Bug 1274958 Opened 4 years ago Updated 2 years ago

Usage of BrowserTestUtils.removeTab()/switchTab() needs to be made clearer

Categories

(Testing :: Mochitest, defect)

Version 3
defect
Not set

Tracking

(Not tracked)

People

(Reporter: Unfocused, Unassigned)

Details

In a review of a patch that included a new test that made use of BrowserTestUtils, Dao pointed me at bug 1253584 comment 3. Which proposes that BrowserTestUtils.removeTab()/switchTab() should generally not be used unless needed; instead favouring using tabbrowser's methods directly - unless you explicitly need to wait for some async operation.

However, there's a lot of missing context and subtly in that. Unless you're very familiar with e10s details, you're not going to know when it's appropriate to use one or the other.

One solution to this is to write a bunch of documentation, and ensure it's kept up to date. I think that's the minimum needed, but I still think it's a bit of a footgun given the module & naming, and wonder if something else can be done to make it more obvious/easier.
I think it fundamentally boils down to our expectations of how tabbrowser works. If I switch from one tab to another and then back, synchronously, should *all* our internal state be up-to-date in a synchronous fashion? Or just some? What about the child process' state? What about session restore? This doesn't ever happen when a human user uses the browser, and doing everything synchronously on the main thread is slow, and the e10s stuff means we just can't, anyway, because of the inter-process communication. So we do things asynchronously and then shit hits the fan when we try to test stuff. AFAIK switching tabs in e10s is always async, and I think that in 99% of cases we should not rely on anything in particular about that process being synchronous, unless that is explicitly what we're testing.
(In reply to :Gijs Kruitbosch (Gone July 28 - Aug 11) from comment #1)
> I think it fundamentally boils down to our expectations of how tabbrowser
> works. If I switch from one tab to another and then back, synchronously,
> should *all* our internal state be up-to-date in a synchronous fashion? Or
> just some? What about the child process' state? What about session restore?
> This doesn't ever happen when a human user uses the browser, and doing
> everything synchronously on the main thread is slow, and the e10s stuff
> means we just can't, anyway, because of the inter-process communication. So
> we do things asynchronously and then shit hits the fan when we try to test
> stuff. AFAIK switching tabs in e10s is always async, and I think that in 99%
> of cases we should not rely on anything in particular about that process
> being synchronous, unless that is explicitly what we're testing.

Both TabClose and TabSelect happen synchronously. That's what the vast majority of our consumers listen to. They don't care about rendering in the content process or whatever BrowserTestUtils currently waits for.
Component: BrowserTest → Mochitest
You need to log in before you can comment on or make changes to this bug.