Closed Bug 1290280 Opened 8 years ago Closed 8 years ago

[e10s] Tab crashes on startup

Categories

(Core :: DOM: Content Processes, defect, P1)

Product:
Core
Component:
DOM: Content Processes
Version:
50 Branch
Platform:
x86
Windows 10
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
e10s + ---
firefox47 --- unaffected
firefox48 --- unaffected
firefox49 --- unaffected
firefox-esr45 --- unaffected
firefox50 + fixed
firefox51 + verified

People

(Reporter: alice0775, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(4 files)

sessionstore.js
64.73 KB, application/x-javascript
Details
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote.
58 bytes, text/x-review-board-request
mikedeboer
: review+
ritu
: approval-mozilla-aurora+
Details
Bug 1290280 - Add tests for window state restoration remoteness flips.
58 bytes, text/x-review-board-request
mikedeboer
: review+
ritu
: approval-mozilla-aurora+
Details
Bug 1290280 - Make bug_423132.js more resilient to the initial browser being remote by default.
58 bytes, text/x-review-board-request
mikedeboer
: review+
ritu
: approval-mozilla-aurora+
Details 
Build Identifier:
https://hg.mozilla.org/mozilla-central/rev/db3ed1fdbbeaf5ab1e8fe454780146e7499be3db
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 ID:20160728030208

The crash is only reproducible on Windows10 IP x64 build 14393.5 (on VMware Workstation Player12). 
Disabling anti-virus does not fix. 
Disabling e10s fixes the crash.

However, I can not reproduce the crash on [1] and [2].
[1] Windows10 x64 build 10115 actual machine
[2] Windows8.1 x64 (on VMware Workstation Player12)

Reproducible: 100% always

Steps To Reproduce:
1. Set "When Nightly starts : Show my windows and tabs from last time" in about:preferences
2. Open only one tab (e.g. about:home)
3. Exit Browser and restart

Actual Results:
Tab crashes
bp-67ced0f8-2e87-4b72-a677-5770f2160728

Expected Results:
Tab should be restored properly

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=84b5a1027550faf65534d67dc02372ba09508550&tochange=80ab1089a326e4a35d32cd6f52ed0194eba89ecf

Regressed by: Bug 1261842
Flags: needinfo?(mconley)
Via local build,
Last Good : ee193eb98d2f
First Bad : 402b6b26d2eb

Regressed by: 	402b6b26d2eb	Mike Conley — Bug 1261842 - When putting the initial tab into the restored background state, flip it to non-remote. r=mikedeboer
Flags: needinfo?(mdeboer)
[Tracking Requested - why for this release]: tab crashes

Sorry. Comment 3 is wrong. (i disabled e10s.)

The problem is reproduced with e10s.
Blocks: 1261842
Status: RESOLVED → REOPENED
status-firefox47: --- → unaffected
status-firefox48: --- → unaffected
status-firefox49: --- → unaffected
status-firefox50: --- → affected
tracking-e10s: --- → ?
tracking-firefox50: --- → ?
Flags: needinfo?(mdeboer)
Flags: needinfo?(mconley)
Resolution: WORKSFORME → ---
Crash volume for signature 'IPCError-browser | ShutDownKill':
 - nightly(version 50):53160 crashes from 2016-06-06.
 - aurora (version 49):91633 crashes from 2016-06-07.
 - beta   (version 48):1174 crashes from 2016-06-06.
 - release(version 47):984 crashes from 2016-05-31.
 - esr    (version 45):18 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly   10822    5966    8216    6846    5875    5644    5988
 - aurora    12859   12518   13245   13035   14496   12750   10399
 - beta        193     189     225     171     117     145     103
 - release     149     166     170     131     143      99     101
 - esr           2       0       0       5       2       3       1

Affected platforms: Windows, Mac OS X, Linux
status-firefox-esr45: --- → affected
Blocks: shutdownkill
Hey jimm,

It's been a while since I've dealt with shutdown kills... it looks like the parent is killing the content process here for some reason, but I can't for the life of me figure out why. The crash reports from Alice show content process stacks that are all over the place - is there a way we can find out why the parent is killing the child?
Flags: needinfo?(mdeboer)
Flags: needinfo?(mconley)
Flags: needinfo?(jmathies)
(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #8)
> Hey jimm,
> 
> It's been a while since I've dealt with shutdown kills... it looks like the
> parent is killing the content process here for some reason, but I can't for
> the life of me figure out why. The crash reports from Alice show content
> process stacks that are all over the place - is there a way we can find out
> why the parent is killing the child?

Looks like this is a funny side effect of making about:blank remote. Looking over:

https://crash-stats.mozilla.com/report/index/d96fb7db-a35f-4b10-9ac0-eff022160730
http://bsmedberg.github.io/socorro-toolbox/html/multiple-minidumps.html?crashID=d96fb7db-a35f-4b10-9ac0-eff022160730

https://crash-stats.mozilla.com/report/index/74558ab7-5732-403a-84bc-d9f8f2160730
http://bsmedberg.github.io/socorro-toolbox/html/multiple-minidumps.html?crashID=74558ab7-5732-403a-84bc-d9f8f2160730

https://crash-stats.mozilla.com/report/index/87af68aa-f9a1-42c8-984d-3f7992160728
http://bsmedberg.github.io/socorro-toolbox/html/multiple-minidumps.html?crashID=87af68aa-f9a1-42c8-984d-3f7992160728

- very low uptimes implying the browser was opened and then quickly closed
- all crashes submitted from the info bar on a restart
- child stacks in startup areas
- all reports are shutdown kill reports - child didn't shutdown properly in 5 seconds after the browser asked it to.
Flags: needinfo?(jmathies)
Hey Alice0775,

Would it be possible for you to record a screen capture of your actions so that I can better understand this?
Flags: needinfo?(alice0775)
Environment:
Windows10 IP x64 build 14393.10 (on VMware Workstation Player12)
Enable e10s
Set "When Nightly starts : Show my windows and tabs from last time" in about:preferences

Screen capture:  https://youtu.be/nG2ZHcO7i3o
1. Open about:home
2. Close Nightly
3. Wait a few seconds
4. Re-launch Nightly
5. You can see tab crashing
6. After a few seconds, Unsubmitted crash report notification will pops up.
7. Click [Submit]
Flags: needinfo?(alice0775)
Flags: needinfo?(mconley)
I think what's happening here  is that the home tab fails to shutdown properly, resulting in a shutdownkill report. That's why the notification comes up after restart.

I'm guessing there's another report for the about:home crash, or a bug that's preventing it. Can you check about:crashes?
The first tab crashes is not only on the about:home but also any on web page(such as www.google.co.jp). bp-32d893ca-3528-416d-bd9d-d0b112160804
Screencast: https://youtu.be/JpRm5RxonEA
1. Open https://www.google.com in several tabs
2. Select first tab
3. Close Nightly
4. Wait a few seconds
5. Re-launch Nightly
6. You can see tab crashing
7. Repeat Step1 to 6
8. Unsubmitted crash report notification will pops up.
9. Click [Submit]
Looking at Alice's crash reports, they all have the "submitted from infobar" metatag, so I think that supports jimm's theory.

Namely, there are two bugs here:

1) Alice0775's content process is sent a message to quit, but it doesn't exit within 5 seconds, so the parent kills it manually (which creates a crash report)

2) When Alice0775's session starts up, they're presented with about:tabcrashed. Presumably, the newly opened content process for the initial browser is crashing immediately. For this, I don't think we have a crash report (and it doesn't look like one's available, seeing as how about:tabcrashed isn't showing the submission form).

#1 is known-ish. There's definitely work to be done to make content process shutdown faster, and for us to handle it better than just killing the content process after 5 seconds.

#2 is more concerning to me, and I think is what I want to focus on in this bug.

Alice, would you mind repeating steps 1-3, and then attaching the sessionstore.js from your profile directory? I want to make sure that we're not somehow accidentally putting about:tabcrashed into the first browser tab slot accidentally via session restore.
Flags: needinfo?(mconley) → needinfo?(alice0775)
Attached file sessionstore.js — 
Flags: needinfo?(alice0775)

    
    

    
  
Mike, I feel that our test failures in fx-ui-tests as logged as bug 1290186 seem to have the same cause. Might that be the case? In that case it would not only be Windows but all platforms.

    
    

    
  
I don't see why esr45 should be affected. I assume the release management bot made a mistake here. But instead 51.0a1 should still show it.

          status-firefox51:
          --- → affected

          status-firefox-esr45:
          affected → unaffected

          tracking-firefox51:
          --- → ?

    
    

    
  
(In reply to Alice0775 White from comment #15)
> Created attachment 8777949 [details]
> sessionstore.js

I couldn't reproduce with this. It just opened four google tabs. I have processCount set to the default as well. Tested on Win7.

Note Alice mentioned that this was only reproducible on Windows10 IP x64 build 14393.5 (vm), and not on Windows10 x64 build 10115 (physical).

    
    

    
  
Probably, slow machine(CPU 4@2.0GHz, RAM 2.5GB is assigned to the VM) is required.
No longer blocks: 1290186

    
    

    
  
I saw this once yesterday on my OS X machine, so I don't think it's limited to VM's.

I also think the shutdown kill crash report is from the previous shutdown, and is unrelated to the initial tab showing about:tabcrashed. I want to focus on that latter problem here in this bug.
Summary: [e10s] Crash in IPCError-browser | ShutDownKill , Tab crashes on startup on Windows10IP Virtual Machine → [e10s] Tab crashes on startup

    
    

    
      
  


  

    
    

    
  
I was able to reproduce this pretty reliably today. It's hard for me to know for sure what's going on, but I have a theory. First of all, the code I added in bug 1291860 wasn't enough - the logic in that I added checks the following:

1) The browser is remote
2) The browser is not pinned
3) restoreImmediately is false OR forceOnDemand is true

The problem is that often times restoreImmediately is _undefined_, which coerces to false, meaning we end up flipping the remoteness of that initial browser just before we attempt to restore it.

For some reason, flipping the remoteness that early on in the browser start-up seems to be a Bad Idea - the IPC layer ends up saying that an abnormal shutdown has occurred, and so the remote browser crash observer notification gets fired and the event that triggers about:tabcrashed to load appears. I don't fully understand why this is occurring, but it only seems to only occur very early in the browser lifetime. If I, for example, have only a non-e10s tab open, and restore an e10s window which has the first tab selected, I cannot get the abnormal shutdown to occur.

So the patch I've attached here works around this (and should more correctly improve the telemetry regression in bug 1291860) because we'll ignore the restoreImmediately option, and we'll also skip flipping remoteness if the selected browser is the browser in question. Belt and suspenders.

    
    

    
  
Alice - I have a try build with my patch built here: http://archive.mozilla.org/pub/firefox/try-builds/mconley@mozilla.com-a2f24f6975518509451e27b7efc350d443884c29/try-win32/firefox-51.0a1.en-US.win32.zip

This fixes the issue for me. Does it fix it for you?
Flags: needinfo?(alice0775)

    
  
Depends on: 1291860

    
    

    
  
(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #23)
> Alice - I have a try build with my patch built here:
> http://archive.mozilla.org/pub/firefox/try-builds/mconley@mozilla.com-
> a2f24f6975518509451e27b7efc350d443884c29/try-win32/firefox-51.0a1.en-US.
> win32.zip
> 
> This fixes the issue for me. Does it fix it for you?

Yes, I can confirm that the try build fixed the first tab crash :)
https://hg.mozilla.org/try/rev/a2f24f6975518509451e27b7efc350d443884c29
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 ID:20160809120149
Flags: needinfo?(alice0775)

    
  

        Attachment #8779465 -
        Flags: review?(mdeboer)

    
    

    
  
Comment on attachment 8779465 [details]
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote.

This appears to have made browser/base/content/test/urlbar/browser_urlbarAboutHomeLoading.js go perma-orange. *sigh*

Investigating.

        Attachment #8779465 -
        Flags: review?(mdeboer)

    
    

    
  
Tracking 50/51+ for this regression, especially since it causes a crash.

          tracking-firefox50:
          ? → +

          tracking-firefox51:
          ? → +

    
  
Component: Untriaged → DOM: Content Processes

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Just a note that I'm going to be adding some tests in another commit on top of the one that I've just requested review on.

    
    

    
      
  


  

    
    

    
  
Comment on attachment 8779465 [details]
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote.

https://reviewboard.mozilla.org/r/70452/#review68368

Only nits. Thanks!

::: browser/components/sessionstore/SessionStore.jsm:3219
(Diff revision 3)
>      let browser = tab.linkedBrowser;
>      let window = tab.ownerGlobal;
>      let tabbrowser = window.gBrowser;
>      let forceOnDemand = options.forceOnDemand;
>  
> -    this._maybeUpdateBrowserRemoteness(Object.assign({
> +    let shouldRestoreImmediately = restoreImmediately ||

Wouldn't 'willRestoreImmediately' be more aptly named?

::: browser/components/sessionstore/SessionStore.jsm:4441
(Diff revision 3)
>        visible.splice(index, 1);
>        hidden.push(tab);
>      }
> +  },
> +
> +  // Returns true if the passed tab is in one of the sets that we're

Can you turn this into a docblock comment?

::: browser/components/sessionstore/SessionStore.jsm:4445
(Diff revision 3)
> +
> +  // Returns true if the passed tab is in one of the sets that we're
> +  // restoring automatically.
> +  willRestoreSoon: function (tab) {
> +    let {priority, hidden, visible} = this.tabs;
> +    let {restoreOnDemand, restorePinnedTabsOnDemand} = this.prefs;

What do you think of letting the variables inside a destructuring assignment breathe a bit?
```js
let { priority, hidden, visible } = this.tabs;
```

::: browser/components/sessionstore/SessionStore.jsm:4449
(Diff revision 3)
> +    let {priority, hidden, visible} = this.tabs;
> +    let {restoreOnDemand, restorePinnedTabsOnDemand} = this.prefs;
> +    let restorePinned = !(restoreOnDemand && restorePinnedTabsOnDemand);
> +    let candidateSet = [];
> +
> +    if (restorePinned && priority.length) {

I'm ok with omitting curlies for one-line statements, since we have those a lot in JS.

::: browser/components/sessionstore/SessionStore.jsm:4450
(Diff revision 3)
> +    let {restoreOnDemand, restorePinnedTabsOnDemand} = this.prefs;
> +    let restorePinned = !(restoreOnDemand && restorePinnedTabsOnDemand);
> +    let candidateSet = [];
> +
> +    if (restorePinned && priority.length) {
> +      candidateSet = candidateSet.concat(priority);

Please use `candidateSet.push(...priority);`, as well as below in this function.

::: browser/components/sessionstore/SessionStore.jsm:4457
(Diff revision 3)
> +
> +    if (!restoreOnDemand) {
> +      if (visible.length) {
> +        candidateSet = candidateSet.concat(visible);
> +      }
> +      if (this.prefs.restoreHiddenTabs && hidden.length) {

Destructure this one too above.

        Attachment #8779465 -
        Flags: review?(mdeboer) → review+

    
  

          tracking-e10s:
          ? → +
Priority: -- → P1

    
    

    
  
Comment on attachment 8779836 [details]
Bug 1290280 - Add tests for window state restoration remoteness flips.

https://reviewboard.mozilla.org/r/70750/#review68386

::: browser/components/sessionstore/test/browser_remoteness_flip_on_restore.js:207
(Diff revision 1)
> +
> +  const TEST_SCENARIOS = [
> +  // Only one tab in the new window, and it's remote. This
> +  // is the common case, since this is how restoration occurs
> +  // when the restored window is being opened.
> +  {

nit: indentation.

::: browser/components/sessionstore/test/browser_remoteness_flip_on_restore.js:315
(Diff revision 1)
> +    // so it will start non-remote, and then flip remoteness.
> +    expectedFlips: [true, false, true],
> +    // Both pinned tabs and the selected tabs should all
> +    // end up being remote.
> +    expectedRemoteness: [true, true, true],
> +  },

nit: trailing comma.

    
    

    
  
Comment on attachment 8779836 [details]
Bug 1290280 - Add tests for window state restoration remoteness flips.

https://reviewboard.mozilla.org/r/70750/#review68388

        Attachment #8779836 -
        Flags: review?(mdeboer) → review+

    
    

    
  
Wouldn't it be fantastic if we'd be able to not have to deal with remoteness flips anymore?
Thanks for grinding through this - important stuff!

    
    

    
  
Comment on attachment 8779836 [details]
Bug 1290280 - Add tests for window state restoration remoteness flips.

https://reviewboard.mozilla.org/r/70750/#review68386

> nit: trailing comma.

We actually seem to have a tendancy to have trailing commas in long (and likely to expand) lists like this, and that's to avoid adding additional lines of blame when new entries need to be added. Are you okay with me leaving it?

    
    

    
  
(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #35)
> Comment on attachment 8779836 [details]
> Bug 1290280 - Add tests for window state restoration remoteness flips.
> 
> https://reviewboard.mozilla.org/r/70750/#review68386
> 
> > nit: trailing comma.
> 
> We actually seem to have a tendancy to have trailing commas in long (and
> likely to expand) lists like this, and that's to avoid adding additional
> lines of blame when new entries need to be added. Are you okay with me
> leaving it?

ni'ing for comment 35 - though, if it's alright with you, I'm going to drop the issue and land as-is. If you feel strongly about the trailing comma, I can land a fix in a follow-up DONTBUILD style. :)
Flags: needinfo?(mdeboer)

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8a2e829640a
Improve the logic for flipping the remoteness of the initial browser back to non-remote. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/1c7f1b07be44
Add tests for window state restoration remoteness flips. r=mikedeboer

    
    

    
  
(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #36)
> > We actually seem to have a tendancy to have trailing commas in long (and
> > likely to expand) lists like this, and that's to avoid adding additional
> > lines of blame when new entries need to be added. Are you okay with me
> > leaving it?

Well, the argument is not solid as a rock and dangling commas always look off to me, just like a painting that's hanging crooked on a wall. However, on a world scale of things, this is but a speck on - not a Van Gogh painting - but a Bob Ross counterfeit. So please, do not push a follow-up merely for the sake of remedying my OCD.
Flags: needinfo?(mdeboer)

    
    

    
  
(In reply to Mike de Boer [:mikedeboer] from comment #40)
> Well, the argument is not solid as a rock and dangling commas always look
> off to me, just like a painting that's hanging crooked on a wall. However,
> on a world scale of things, this is but a speck on - not a Van Gogh painting
> - but a Bob Ross counterfeit. So please, do not push a follow-up merely for
> the sake of remedying my OCD.

<3

    
    

    
      
  

    
    

    
  
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e57110913fc
Improve the logic for flipping the remoteness of the initial browser back to non-remote. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/2b50b05f8929
Add tests for window state restoration remoteness flips. r=mikedeboer

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/d8a2e829640a
Status: NEW → RESOLVED
Closed: 8 years ago → 8 years ago

          status-firefox51:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

    
    

    
  
Backed out 

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2b50b05f8929f3e72f3dc881610f1bb77d1e1118

Failure log for cookie failure: https://treeherder.mozilla.org/logviewer.html#?job_id=1831663&repo=autoland

10:28:01     INFO -  24 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_423132.js | expected one cookie - Got 0, expected 1
10:28:01     INFO -  Stack trace:
10:28:01     INFO -  chrome://mochikit/content/browser-test.js:test_is:967
10:28:01     INFO -  chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_423132.js:test/</</</ready:45
10:28:01     INFO -  resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:Handler.prototype.process:937
10:28:01     INFO -  resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:this.PromiseWalker.walkerLoop:816
10:28:01     INFO -  *************************
10:28:01     INFO -  A coding exception was thrown in a Promise resolution callback.
10:28:01     INFO -  See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
10:28:01     INFO -  Full message: TypeError: cookie is undefined
10:28:01     INFO -  Full stack: test/</</</ready@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_423132.js:61:11
10:28:01     INFO -  Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
10:28:01     INFO -  this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
10:28:01     INFO -  Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
10:28:01     INFO -  this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
10:28:01     INFO -  this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
10:28:01     INFO -  resolve@resource://app/modules/sessionstore/TabStateFlusher.jsm:146:5
10:28:01     INFO -  resolve@resource://app/modules/sessionstore/TabStateFlusher.jsm:52:5
10:28:01     INFO -  receiveMessage@resource:///modules/sessionstore/SessionStore.jsm:727:11

Failure log for timeout of new test: https://treeherder.mozilla.org/logviewer.html#?job_id=1894473&repo=autoland

browser_remoteness_flip_on_restore.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/24b4836d23c6
Backed out changeset 2b50b05f8929 
https://hg.mozilla.org/integration/autoland/rev/23ca0cdcf3b0
Backed out changeset 1e57110913fc for timeouts of added test and remoteness issues e.g. in cookie tests like browser_423132.js. r=backout

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 8780871 [details]
Bug 1290280 - Make bug_423132.js more resilient to the initial browser being remote by default.

https://reviewboard.mozilla.org/r/71448/#review69066

`BrowserTestUtils` make you win each time. Thanks!

::: browser/components/sessionstore/test/browser_423132.js:1
(Diff revision 2)
>  /* This Source Code Form is subject to the terms of the Mozilla Public

While you're here, can you remove the license header and add `"use strict";` here?

        Attachment #8780871 -
        Flags: review?(mdeboer) → review+

    
    

    
      
  

    
    

    
  
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3bce29db2329
Improve the logic for flipping the remoteness of the initial browser back to non-remote. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/e5ab45eab1be
Add tests for window state restoration remoteness flips. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/6ad896bd72cb
Make bug_423132.js more resilient to the initial browser being remote by default. r=mikedeboer

    
  
Flags: needinfo?(mconley)

    
    

    
  
Comment on attachment 8779465 [details]
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote.

Approval Request Comment
[Feature/regressing bug #]:

Bug 1261842

[User impact if declined]:

In some cases, during a session restore with e10s enabled, if the initial browser is selected, the tab will crash immediately.

[Describe test coverage new/current, TreeHerder]:

I've added automated tests in this patch series to ensure that we're flipping remoteness correctly.

[Risks and why]: 

I'd say low risk, especially because the tests show us doing the right thing now.

[String/UUID change made/needed]:

None.

        Attachment #8779465 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8779836 [details]
Bug 1290280 - Add tests for window state restoration remoteness flips.

See comment 57

        Attachment #8779836 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8780871 [details]
Bug 1290280 - Make bug_423132.js more resilient to the initial browser being remote by default.

See comment 57

        Attachment #8780871 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Hello Alice0775 White, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(alice0775)

    
    

    
  
Reproduced Nightly50.0a1 (2016-Jul-28).
And I verified to fix on Nightly51.0a1 (2016-Aug-16).
Flags: needinfo?(alice0775)

    
    

    
  
(In reply to Alice0775 White from comment #61)
> Reproduced Nightly50.0a1 (2016-Jul-28).
> And I verified to fix on Nightly51.0a1 (2016-Aug-16).

Fantastic! Thank you for a prompt reply. :)
Status: RESOLVED → VERIFIED

          status-firefox51:
          fixed → verified

    
    

    
  
Comment on attachment 8779465 [details]
Bug 1290280 - Improve the logic for flipping the remoteness of the initial browser back to non-remote.

Crash was verified on Nightly, this seems like a medium risk change to me, therefore makes sense to uplift sooner in Aurora cycle so it gets stabilized sooner. Aurora50+

        Attachment #8779465 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

        Attachment #8779836 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

        Attachment #8780871 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
  
Needs rebasing for Aurora.
Assignee: nobody → mconley
Flags: needinfo?(mconley)

    
    

    
  
Bah, forgot that bug 1291860 is a requirement for it. Requested approval on bug 1291860.
Flags: needinfo?(mconley)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: