Closed Bug 1343056 Opened 3 years ago Closed 3 years ago

Session Restore is broken due to recent regression in FF53 & FF54

Categories

(Firefox :: Session Restore, defect, critical)

53 Branch
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 54
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 + verified
firefox54 + verified

People

(Reporter: u589863, Assigned: mlongaray)

References

(Blocks 1 open bug)

Details

(Keywords: dataloss, regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.88 Safari/537.36 Vivaldi/1.8.755.3

Steps to reproduce:

STR:
To reproduce the problem Use FF50 & FF54.

Also set setting: restore tabs & windows on restart
e10s on /off doesn't matter

now use this session-store file

https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg

might not be totally sfw.

use FF50 and open the session.

to the right there are 75tabs & 2tabs to the left.

now click open tabs to the right, especially the tabs labeled new-tab.
They have sites stored in them(NO DATA LOSS!)

now open the same session in FF54 and click tabs and they are restored properly that is even new tabs with data/no data loss.


But now the problem can be reproduced

now open the same session in FF54 and then close FF54 and start again

see the total number of tabs they are reduced from , all tabs which sometimes shown as new-tabs even with data are removed
IMPORTANT DATA LOSS

Now this is fairly common with default profile(except restore tabs at start)
and a low speed/**** connection.
Like open multiple tabs quickly on a page and the connection of isp has low response and some pages fail to load and tabs are labelled as newtab(but stil contain data as they should of url and etc),
restarting FF50 does not loose those tabs but FF54 does loose them.

Please Fix this to work like FF50 does so none of the important data is lost due to improper connection or renaming to not properly loaded tabs etc

I am sorry if was not clear in my explanation.
if any more question are there i'll try to answer them.



Actual results:

This is a major regression and users updating will unknowingly  suffer data loss!!

just use the same session in FF50 no loss of data
in FF54 no loss first time with same session(sessionstore file is ok and working properly)


but just use the same session in FF54 & restart FF54 twice and data loss.

And FF54 does not save data which is incomplete/not totally correct
but is important still like 
improperly loaded tabs / not completely loaded tabs/ tabs stopped loading for some other reason 
and named new tab.
FF50 does this properly & so did FF53 just before this bug was filed & session store was modified




Expected results:

https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA

Open in FF50 : 58 tabs open and session is loaded properly.
Click on the tabs named new tab & data is loaded properly=NO DATA LOST

open in FF53/54 : restart again
from 58 tabs it goes don to 19 tabs=IMPORTANT DATA IS LOST

58 tabs to 19 tabs

IDK why but FF50 data restore is VERY RELIABLE & FF54 is just not up-to the mark.
In normal browsing , incomplete tabs and websites which do not load properly or some reason FF crashes etc then FF54 looses data & info unlike FF50 which no matter what keeps all info & restores properly

The bunch of patch (Bug 1323987 and Bug 1306294) should be backed out.
[Tracking Requested - why for this release]:IMPORTANT DATA LOSS
Severity: normal → major
Has STR: --- → yes
Component: Untriaged → Session Restore
Keywords: dataloss, regression
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Depends on: 1342849
Blocks: 1306294
Blocks: 1323987
Summary: The bunch of patch (Bug 1323987 and Bug 1306294) should be backed out. → Session Restore is broken due to recent regression in FF53 & FF54
Whiteboard: The bunch of patch (Bug 1323987 and Bug 1306294) should be backed out.
Error in Browwser console on Nightly54.0a1.

Utils: Failed to deserialize principal_b64 'SmIS26zLEdO3ZQBgsLbOy9A5HoYa10qwu3wU1tmWc2k=' [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISerializationHelper.deserializeObject]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: resource://gre/modules/sessionstore/Utils.jsm :: deserializePrincipal :: line 140"  data: no]
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISHEntry.triggeringPrincipal]  SessionHistory.jsm:432



Utils: Failed to deserialize principal_b64 'ZT4OTT7kRfqycpfCC8AeuAAAAAAAAAAAwAAAAAAAAEYBLyd8AA6vTdu5NkEya6SKrpIHOlRteE8wkTq4cYEyCMYAAAAABWFib3V0AAAABWJsYW5rAODaHXAvexHTjNAAYLD8FKOSBzpUbXhPMJE6uHGBMgjGAAAAAA5tb3otc2FmZS1hYm91dAAAAAVibGFuawAB3pRy0IA0EdOTmQAQS6D9QAAAAAAAAAAAwAAAAAAAAEYAAAAB//////////8BAAAAJGNocm9tZTovL2Jyb3dzZXIvY29udGVudC9icm93c2VyLnh1bAAAAAAAAAAGAAAACQAAAAcAAAAJ/////wAAAAn/////AAAACQAAAAcAAAAQAAAAFAAAABAAAAAUAAAAEAAAAAkAAAAZAAAABwAAACEAAAADAAAAAP////8AAAAQ/////wAAABD/////AQAAAAAAAAAAAAEAAAAAAAA=' [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISerializationHelper.deserializeObject]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/sessionstore/Utils.jsm :: deserializePrincipal :: line 140"  data: no]
https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA

Here is another one was able to reproduce the problem even more and better.

Open in FF50 : 58 tabs open and session is loaded properly.
Click on the tabs named new tab & data is loaded properly=NO DATA LOST

open in FF53/54 : restart again
from 58 tabs it goes don to 19 tabs=IMPORTANT DATA IS LOST

58 tabs to 19 tabs

IDK why but FF50 data restore is VERY RELIABLE & FF54 is just not up-to the mark.
In normal browsing , incomplete tabs and websites which do not load properly or some reason FF crashes etc then FF54 looses data & info unlike FF50 which no matter what keeps all info & restores properly

This is a major regression and users updating will unknowingly  suffer data loss!!
This should be unacceptable for FF devs as this is a major Flaw.

If it helps this problem started when this bug was filed- some other bug changed the way session restore works,
up to that point session restore was very very reliable in FF53 but then went south.
Flags: needinfo?(alice0775)
Flags: needinfo?(Virtual)
I can reproduce the data loss with the provided sessionstore.js on Nightly54.0a1.
(However, I do not know what procedure the sessionstore.js was generated. I could not reproduce yet.)
Flags: needinfo?(alice0775)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Alice0775 White from comment #4)
> I can reproduce the data loss with the provided sessionstore.js on
> Nightly54.0a1.
> (However, I do not know what procedure the sessionstore.js was generated. I
> could not reproduce yet.)

This happens quite often with heavy usage, started happening when those patched landed and after restarts tab data is missing.
Also sometimes while loading tabs 10-15 suddenly while loading tabs stop loading and get renamed to new tab(only in FF53/54 same setup on three different laptops)
Flags: needinfo?(alice0775)
Every thing is fresh

adding more info:


Application Basics
------------------

Name: Firefox
Version: 54.0a1
Build ID: 20170226030205
Update Channel: nightly
User Agent: Mozilla/5.0 (Windows NT 10.0; rv:54.0) Gecko/20100101 Firefox/54.0
OS: Windows_NT 10.0
Multiprocess Windows: 1/1 (Enabled by default)
Google Key: Found
Mozilla Location Service Key: Found
Safe Mode: false

Crash Reports for the Last 3 Days
---------------------------------

All Crash Reports

Extensions
----------

Name: Application Update Service Helper
Version: 2.0
Enabled: true
ID: aushelper@mozilla.org

Name: FlyWeb
Version: 1.0.0
Enabled: true
ID: flyweb@mozilla.org

Name: Form Autofill
Version: 1.0
Enabled: true
ID: formautofill@mozilla.org

Name: Multi-process staged rollout
Version: 1.9
Enabled: true
ID: e10srollout@mozilla.org

Name: Pocket
Version: 1.0.5
Enabled: true
ID: firefox@getpocket.com

Name: Presentation
Version: 1.0.0
Enabled: true
ID: presentation@mozilla.org

Name: Shield Recipe Client
Version: 1.0.0
Enabled: true
ID: shield-recipe-client@mozilla.org

Name: Web Compat
Version: 1.1
Enabled: true
ID: webcompat@mozilla.org

Name: WebCompat Reporter
Version: 1.0.0
Enabled: true
ID: webcompat-reporter@mozilla.org

Graphics
--------

Features
Compositing: Direct3D 11
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled; scrollbar drag enabled
WebGL 1 Driver WSI Info: EGL_VENDOR: Google Inc. (adapter LUID: 000000000000d2d3) EGL_VERSION: 1.4 (ANGLE 2.1.0.2a250c8a0e15) EGL_EXTENSIONS: EGL_EXT_create_context_robustness EGL_ANGLE_d3d_share_handle_client_buffer EGL_ANGLE_d3d_texture_client_buffer EGL_ANGLE_surface_d3d_texture_2d_share_handle EGL_ANGLE_query_surface_pointer EGL_ANGLE_window_fixed_size EGL_ANGLE_keyed_mutex EGL_ANGLE_surface_orientation EGL_ANGLE_direct_composition EGL_NV_post_sub_buffer EGL_KHR_create_context EGL_EXT_device_query EGL_KHR_image EGL_KHR_image_base EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_image EGL_KHR_get_all_proc_addresses EGL_KHR_stream EGL_KHR_stream_consumer_gltexture EGL_NV_stream_consumer_gltexture_yuv EGL_ANGLE_flexible_surface_compatibility EGL_ANGLE_stream_producer_d3d_texture_nv12 EGL_ANGLE_create_context_webgl_compatibility EGL_CHROMIUM_create_context_bind_generates_resource EGL_EXTENSIONS(nullptr): EGL_EXT_client_extensions EGL_EXT_platform_base EGL_EXT_platform_device EGL_ANGLE_platform_angle EGL_ANGLE_platform_angle_d3d EGL_ANGLE_device_creation EGL_ANGLE_device_creation_d3d11 EGL_ANGLE_experimental_present_path EGL_KHR_client_get_all_proc_addresses
WebGL 1 Driver Renderer: Google Inc. -- ANGLE (AMD Radeon HD 6480G Direct3D11 vs_5_0 ps_5_0)
WebGL 1 Driver Version: OpenGL ES 2.0 (ANGLE 2.1.0.2a250c8a0e15)
WebGL 1 Driver Extensions: GL_ANGLE_depth_texture GL_ANGLE_framebuffer_blit GL_ANGLE_framebuffer_multisample GL_ANGLE_instanced_arrays GL_ANGLE_lossy_etc_decode GL_ANGLE_pack_reverse_row_order GL_ANGLE_robust_client_memory GL_ANGLE_texture_compression_dxt3 GL_ANGLE_texture_compression_dxt5 GL_ANGLE_texture_usage GL_ANGLE_translated_shader_source GL_CHROMIUM_bind_generates_resource GL_CHROMIUM_bind_uniform_location GL_CHROMIUM_copy_compressed_texture GL_CHROMIUM_copy_texture GL_CHROMIUM_sync_query GL_EXT_blend_minmax GL_EXT_color_buffer_half_float GL_EXT_debug_marker GL_EXT_discard_framebuffer GL_EXT_disjoint_timer_query GL_EXT_draw_buffers GL_EXT_frag_depth GL_EXT_map_buffer_range GL_EXT_occlusion_query_boolean GL_EXT_read_format_bgra GL_EXT_robustness GL_EXT_sRGB GL_EXT_shader_texture_lod GL_EXT_texture_compression_dxt1 GL_EXT_texture_filter_anisotropic GL_EXT_texture_format_BGRA8888 GL_EXT_texture_rg GL_EXT_texture_storage GL_EXT_unpack_subimage GL_KHR_debug GL_NV_EGL_stream_consumer_external GL_NV_fence GL_NV_pack_subimage GL_NV_pixel_buffer_object GL_OES_EGL_image GL_OES_EGL_image_external GL_OES_compressed_ETC1_RGB8_texture GL_OES_depth32 GL_OES_element_index_uint GL_OES_get_program_binary GL_OES_mapbuffer GL_OES_packed_depth_stencil GL_OES_rgb8_rgba8 GL_OES_standard_derivatives GL_OES_texture_float GL_OES_texture_float_linear GL_OES_texture_half_float GL_OES_texture_half_float_linear GL_OES_texture_npot GL_OES_vertex_array_object
WebGL 1 Extensions: ANGLE_instanced_arrays EXT_blend_minmax EXT_color_buffer_half_float EXT_frag_depth EXT_shader_texture_lod EXT_texture_filter_anisotropic EXT_disjoint_timer_query MOZ_debug_get OES_element_index_uint OES_standard_derivatives OES_texture_float OES_texture_float_linear OES_texture_half_float OES_texture_half_float_linear OES_vertex_array_object WEBGL_color_buffer_float WEBGL_compressed_texture_s3tc WEBGL_debug_renderer_info WEBGL_debug_shaders WEBGL_depth_texture WEBGL_draw_buffers WEBGL_lose_context MOZ_WEBGL_lose_context MOZ_WEBGL_compressed_texture_s3tc MOZ_WEBGL_depth_texture
WebGL 2 Driver WSI Info: EGL_VENDOR: Google Inc. (adapter LUID: 000000000000d2d3) EGL_VERSION: 1.4 (ANGLE 2.1.0.2a250c8a0e15) EGL_EXTENSIONS: EGL_EXT_create_context_robustness EGL_ANGLE_d3d_share_handle_client_buffer EGL_ANGLE_d3d_texture_client_buffer EGL_ANGLE_surface_d3d_texture_2d_share_handle EGL_ANGLE_query_surface_pointer EGL_ANGLE_window_fixed_size EGL_ANGLE_keyed_mutex EGL_ANGLE_surface_orientation EGL_ANGLE_direct_composition EGL_NV_post_sub_buffer EGL_KHR_create_context EGL_EXT_device_query EGL_KHR_image EGL_KHR_image_base EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_image EGL_KHR_get_all_proc_addresses EGL_KHR_stream EGL_KHR_stream_consumer_gltexture EGL_NV_stream_consumer_gltexture_yuv EGL_ANGLE_flexible_surface_compatibility EGL_ANGLE_stream_producer_d3d_texture_nv12 EGL_ANGLE_create_context_webgl_compatibility EGL_CHROMIUM_create_context_bind_generates_resource EGL_EXTENSIONS(nullptr): EGL_EXT_client_extensions EGL_EXT_platform_base EGL_EXT_platform_device EGL_ANGLE_platform_angle EGL_ANGLE_platform_angle_d3d EGL_ANGLE_device_creation EGL_ANGLE_device_creation_d3d11 EGL_ANGLE_experimental_present_path EGL_KHR_client_get_all_proc_addresses
WebGL 2 Driver Renderer: Google Inc. -- ANGLE (AMD Radeon HD 6480G Direct3D11 vs_5_0 ps_5_0)
WebGL 2 Driver Version: OpenGL ES 3.0 (ANGLE 2.1.0.2a250c8a0e15)
WebGL 2 Driver Extensions: GL_ANGLE_depth_texture GL_ANGLE_framebuffer_blit GL_ANGLE_framebuffer_multisample GL_ANGLE_instanced_arrays GL_ANGLE_lossy_etc_decode GL_ANGLE_pack_reverse_row_order GL_ANGLE_robust_client_memory GL_ANGLE_texture_compression_dxt3 GL_ANGLE_texture_compression_dxt5 GL_ANGLE_texture_usage GL_ANGLE_translated_shader_source GL_CHROMIUM_bind_generates_resource GL_CHROMIUM_bind_uniform_location GL_CHROMIUM_copy_compressed_texture GL_CHROMIUM_copy_texture GL_CHROMIUM_sync_query GL_EXT_blend_minmax GL_EXT_color_buffer_float GL_EXT_color_buffer_half_float GL_EXT_debug_marker GL_EXT_discard_framebuffer GL_EXT_disjoint_timer_query GL_EXT_draw_buffers GL_EXT_frag_depth GL_EXT_map_buffer_range GL_EXT_occlusion_query_boolean GL_EXT_read_format_bgra GL_EXT_robustness GL_EXT_sRGB GL_EXT_shader_texture_lod GL_EXT_texture_compression_dxt1 GL_EXT_texture_filter_anisotropic GL_EXT_texture_format_BGRA8888 GL_EXT_texture_norm16 GL_EXT_texture_rg GL_EXT_texture_storage GL_EXT_unpack_subimage GL_KHR_debug GL_NV_EGL_stream_consumer_external GL_NV_fence GL_NV_pack_subimage GL_NV_pixel_buffer_object GL_OES_EGL_image GL_OES_EGL_image_external GL_OES_EGL_image_external_essl3 GL_OES_compressed_ETC1_RGB8_texture GL_OES_depth32 GL_OES_element_index_uint GL_OES_get_program_binary GL_OES_mapbuffer GL_OES_packed_depth_stencil GL_OES_rgb8_rgba8 GL_OES_standard_derivatives GL_OES_texture_float GL_OES_texture_float_linear GL_OES_texture_half_float GL_OES_texture_half_float_linear GL_OES_texture_npot GL_OES_vertex_array_object
WebGL 2 Extensions: EXT_color_buffer_float EXT_texture_filter_anisotropic EXT_disjoint_timer_query MOZ_debug_get OES_texture_float_linear WEBGL_compressed_texture_s3tc WEBGL_debug_renderer_info WEBGL_debug_shaders WEBGL_lose_context MOZ_WEBGL_lose_context MOZ_WEBGL_compressed_texture_s3tc
Audio Backend: wasapi
Direct2D: true
DirectWrite: true (10.0.15042.0)
GPU #1
Active: Yes
Description: AMD Radeon HD 6480G
Vendor ID: 0x1002
Device ID: 0x9648
Driver Version: 15.201.1151.0
Driver Date: 8-22-2015
Drivers: aticfx32 aticfx32 aticfx32 atiumdag atidxx32 atiumdva
Subsys ID: 3564103c
RAM: 512

Diagnostics
AzureCanvasAccelerated: 0
AzureCanvasBackend: Direct2D 1.1
AzureCanvasBackend (UI Process): skia
AzureContentBackend: Direct2D 1.1
AzureContentBackend (UI Process): skia
AzureFallbackCanvasBackend (UI Process): cairo
GPUProcessPid: 1336
GPUProcess: Terminate GPU Process
Decision Log
D3D9_COMPOSITING:
disabled by default: Disabled by default
WEBRENDER:
unavailable by runtime: Build doesn't include WebRender




Important Modified Preferences
------------------------------

browser.cache.disk.capacity: 358400
browser.cache.disk.filesystem_reported: 1
browser.cache.disk.smart_size.first_run: false
browser.cache.frecency_experiment: 1
browser.download.importedFromSqlite: true
browser.places.smartBookmarksVersion: 8
browser.startup.homepage_override.buildID: 20170226030205
browser.startup.homepage_override.mstone: 54.0a1
extensions.lastAppVersion: 54.0a1
media.gmp.storage.version.observed: 1
media.hardware-video-decoding.failed: false
network.cookie.prefsMigrated: true
places.history.expiration.transient_current_max_pages: 105795
plugin.disable_full_page_plugin_for_types: application/pdf
security.sandbox.content.tempDirSuffix: {bbcc2a9f-0881-426e-8016-32401e6a9c4a}

Important Locked Preferences
----------------------------

Places Database
---------------

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.13.1
Version in use: 4.13.1

NSS
Expected minimum version: 3.30 Beta
Version in use: 3.30 Beta

NSSSMIME
Expected minimum version: 3.30 Beta
Version in use: 3.30 Beta

NSSSSL
Expected minimum version: 3.30 Beta
Version in use: 3.30 Beta

NSSUTIL
Expected minimum version: 3.30 Beta
Version in use: 3.30 Beta

Experimental Features
---------------------

Sandbox
-------

Content Process Sandbox Level: 2
Flags: needinfo?(alice0775)
Severity: major → critical
Flags: needinfo?(Virtual)
Priority: P1 → --
Version: 53 Branch → 54 Branch
Has Regression Range: --- → yes
Flags: needinfo?(mlongaray)
Version: 54 Branch → 53 Branch
That sessionstore.js linked above have some entries like

{"entries":[],"lastAccessed":1488152376406,"hidden":false,"attributes":{},"userContextId":0,"userTypedValue":"https://c2.staticflickr.com/4/3932/15311141280_61a6fca4ea_o.jpg","userTypedClear":1,"image":"","iconLoadingPrincipal": ...

ContentRestore.restoreTabContent is handling it, so the page is loaded normally when the tab was clicked.
It was saved before bug 1306294 but it is discarded in SessionStore._shouldSaveTab for now.
(In reply to atlanto from comment #7)
> That sessionstore.js linked above have some entries like
> 
> {"entries":[],"lastAccessed":1488152376406,"hidden":false,"attributes":{},
> "userContextId":0,"userTypedValue":"https://c2.staticflickr.com/4/3932/
> 15311141280_61a6fca4ea_o.jpg","userTypedClear":1,"image":"",
> "iconLoadingPrincipal": ...
> 
> ContentRestore.restoreTabContent is handling it, so the page is loaded
> normally when the tab was clicked.
> It was saved before bug 1306294 but it is discarded in
> SessionStore._shouldSaveTab for now.

"but it is discarded in
> SessionStore._shouldSaveTab for now."
But that should not be acceptable as it is causing data loss regardless intended or not,either it should be changed to save all and not discard anything or the bug should be backed out.
Flags: needinfo?(euthanasia_waltz)
(In reply to atlanto from comment #7)
> That sessionstore.js linked above have some entries like
> 
> {"entries":[],"lastAccessed":1488152376406,"hidden":false,"attributes":{},
> "userContextId":0,"userTypedValue":"https://c2.staticflickr.com/4/3932/
> 15311141280_61a6fca4ea_o.jpg","userTypedClear":1,"image":"",
> "iconLoadingPrincipal": ...
> 
> ContentRestore.restoreTabContent is handling it, so the page is loaded
> normally when the tab was clicked.
> It was saved before bug 1306294 but it is discarded in
> SessionStore._shouldSaveTab for now.

You got it, atlanto. I believe that's a case we did not cover when re-writing the patch on bug 1323987.

I think it's better for us to submit yet another small patch fixing the issue rather than backing out patch sets from bug 1306294 and bug 1323987.

What do you think, mikedeboer?
Flags: needinfo?(mlongaray) → needinfo?(mdeboer)
No longer depends on: 1342849
I'm for amending what we currently have than a backout in this case. Backouts are very costly in general, especially in an early stage like this. If you see a clear route to fixing this issue, please submit a patch and I'll review it within 24h. Or mconley, depending on who's more appropriate ;-)
Flags: needinfo?(mdeboer)
Assignee: nobody → mlongaray
Browser chrome tests look good - all green except for one intermittent
Attached file sessionstore.js
guys sorry for disturbig again

But just tried it in Linux/windows 10/windows 7
just to see how problematic the situation and can confirm that data loss happens on all.


like altanto said
>"It was saved before bug 1306294 but it is discarded in SessionStore._shouldSaveTab for now"

>"Bug 1343056 - Take userTypedValue into account when saving tabs to disk.

>This patch adds userTypedValue data validation when saving tabs to disk"

Can you please test this patch against this new session restore File?

In firefox54 just use the sessiorestore file.
STR:1
First time when opened over 100 tabs, all can be loaded when clicked.

Second restart most of tabs are lost =dataloss

STR2:
Use the sessionrestore File
Start Firefox54 , over hundred tabs, close firefox.
Restart firefox 54 & Tabs are lost =data loss occurs.
Okay, did some testing using all of aforementioned session store files. Following are the results:

File 1: https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg
76 tabs on session store -> _76_ tabs properly restored after restarting session.

File 2: https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA
59 tabs on session store -> _59_ tabs properly restored after restarting session.

File 3: sessionstore.js attachment
STR1: 194 tabs on session store, tabs loaded when clicked (except for 6 - see note) -> _188_ tabs restored after restarting session.
STR2: 194 tabs on session store -> _188_ tabs restored after restarting session.


I think we're okay now restoring session - no loss of important data, see why below. mikedeboer, please let me know otherwise so we can update the patch somehow.


**Note:** 
6 tabs from _file 3_ have the following state. No entry url (not even "about:blank" or "about:newtab") nor user data (no userTypedValue).
{
 "entries":[],
 "lastAccessed":1488502876863,
 "hidden":false,
 "attributes":{},
 "userContextId":0,
 "userTypedValue":"",
 "userTypedClear":0,
 "image":"",
 "iconLoadingPrincipal": ...,
 "index":null
}
Flags: needinfo?(mdeboer)
Matheus, thanks for analyzing that data. Those entries are indeed not supposed to be restored and can/ will not be considered data loss. Just a good cleanup.
Flags: needinfo?(mdeboer)
Comment on attachment 8843017 [details]
Bug 1343056 - Take userTypedValue into account when saving tabs to disk.

https://reviewboard.mozilla.org/r/116764/#review118682

LGTM!
Attachment #8843017 - Flags: review?(mdeboer) → review+
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32604a6dd45e
Take userTypedValue into account when saving tabs to disk. r=mikedeboer
Blocks: 962433
(In reply to Matheus Longaray (:mlongaray) from comment #16)
> Okay, did some testing using all of aforementioned session store files.
> Following are the results:
> 
> File 1: https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg
> 76 tabs on session store -> _76_ tabs properly restored after restarting
> session.
> 
> File 2: https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA
> 59 tabs on session store -> _59_ tabs properly restored after restarting
> session.
> 
> File 3: sessionstore.js attachment
> STR1: 194 tabs on session store, tabs loaded when clicked (except for 6 -
> see note) -> _188_ tabs restored after restarting session.
> STR2: 194 tabs on session store -> _188_ tabs restored after restarting
> session.
> 
> 
> I think we're okay now restoring session - no loss of important data, see
> why below. mikedeboer, please let me know otherwise so we can update the
> patch somehow.
> 
> 
> **Note:** 
> 6 tabs from _file 3_ have the following state. No entry url (not even
> "about:blank" or "about:newtab") nor user data (no userTypedValue).
> {
>  "entries":[],
>  "lastAccessed":1488502876863,
>  "hidden":false,
>  "attributes":{},
>  "userContextId":0,
>  "userTypedValue":"",
>  "userTypedClear":0,
>  "image":"",
>  "iconLoadingPrincipal": ...,
>  "index":null
> }

Good work, Glad to hear that you fixed it, now just needs testings to see if it's indeed fixed.
when can we test this?
also will this be back-ported to FF52 stable & FF53 beta? Those seem affected.
(In reply to Sunil Kumar from comment #20) 
> Good work, Glad to hear that you fixed it, now just needs testings to see if
> it's indeed fixed.
> when can we test this?
> also will this be back-ported to FF52 stable & FF53 beta? Those seem
> affected.

FF52 is not affected. Bug 1306294 started affecting FF53 only.

Bug 1323987 got merged for FF54 - as we hope for bug 1343056 (_this_).

As soon as _this_ bug lands on mozilla-central and you're able to verify the fix, we'll figure out an uplift strategy so these two bug fixes can clear things up for FF53.
(In reply to Matheus Longaray (:mlongaray) from comment #21)
> (In reply to Sunil Kumar from comment #20) 
> > Good work, Glad to hear that you fixed it, now just needs testings to see if
> > it's indeed fixed.
> > when can we test this?
> > also will this be back-ported to FF52 stable & FF53 beta? Those seem
> > affected.
> 
> FF52 is not affected. Bug 1306294 started affecting FF53 only.
> 
> Bug 1323987 got merged for FF54 - as we hope for bug 1343056 (_this_).
> 
> As soon as _this_ bug lands on mozilla-central and you're able to verify the
> fix, we'll figure out an uplift strategy so these two bug fixes can clear
> things up for FF53.

Ah thanks for the Fix and reply :) was thinking of any link for test-builds to test but like you said it's fixed.

ps: reddit images sometimes keep loading then convert to newtab on weak 2g/3g connections instead of showing problem loading pages,
what criteria should be put in whiteboard and keywords while filing the bug.
(In reply to Sunil Kumar from comment #22)
> (In reply to Matheus Longaray (:mlongaray) from comment #21)
> > (In reply to Sunil Kumar from comment #20) 
> > > Good work, Glad to hear that you fixed it, now just needs testings to see if
> > > it's indeed fixed.
> > > when can we test this?
> > > also will this be back-ported to FF52 stable & FF53 beta? Those seem
> > > affected.
> > 
> > FF52 is not affected. Bug 1306294 started affecting FF53 only.
> > 
> > Bug 1323987 got merged for FF54 - as we hope for bug 1343056 (_this_).
> > 
> > As soon as _this_ bug lands on mozilla-central and you're able to verify the
> > fix, we'll figure out an uplift strategy so these two bug fixes can clear
> > things up for FF53.
> 
> Ah thanks for the Fix and reply :) was thinking of any link for test-builds
> to test but like you said it's fixed.

Oh no - sorry the confusion, I meant for you to get a Nightly fresh build to test it as soon as our patch lands (and it gets into the build of course). You can get a new build every day at https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.

Once we get a green light from you, we'll proceed with the uplift strategy.
(In reply to Sunil Kumar from comment #22)
> ps: reddit images sometimes keep loading then convert to newtab on weak
> 2g/3g connections instead of showing problem loading pages,
> what criteria should be put in whiteboard and keywords while filing the bug.

This is being tracked in bug #610357 and it's especially visible on slow ISPs and when closing not fully loaded website pages and restoring them again.
(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #24)
> (In reply to Sunil Kumar from comment #22)
> > ps: reddit images sometimes keep loading then convert to newtab on weak
> > 2g/3g connections instead of showing problem loading pages,
> > what criteria should be put in whiteboard and keywords while filing the bug.
> 
> This is being tracked in bug #610357 and it's especially visible on slow
> ISPs and when closing not fully loaded website pages and restoring them
> again.

Oh thanks , was going to file a bug and now more people have the same problem.
2010-11-08 was the initial date of the bug and no one seems to be working on it,will it be fixed as it is critical imo.(In reply to Matheus Longaray (:mlongaray) from comment #23)
> (In reply to Sunil Kumar from comment #22)
> > (In reply to Matheus Longaray (:mlongaray) from comment #21)
> > > (In reply to Sunil Kumar from comment #20) 
> > > > Good work, Glad to hear that you fixed it, now just needs testings to see if
> > > > it's indeed fixed.
> > > > when can we test this?
> > > > also will this be back-ported to FF52 stable & FF53 beta? Those seem
> > > > affected.
> > > 
> > > FF52 is not affected. Bug 1306294 started affecting FF53 only.
> > > 
> > > Bug 1323987 got merged for FF54 - as we hope for bug 1343056 (_this_).
> > > 
> > > As soon as _this_ bug lands on mozilla-central and you're able to verify the
> > > fix, we'll figure out an uplift strategy so these two bug fixes can clear
> > > things up for FF53.
> > 
> > Ah thanks for the Fix and reply :) was thinking of any link for test-builds
> > to test but like you said it's fixed.
> 
> Oh no - sorry the confusion, I meant for you to get a Nightly fresh build to
> test it as soon as our patch lands (and it gets into the build of course).
> You can get a new build every day at
> https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.
> 
> Once we get a green light from you, we'll proceed with the uplift strategy.

ah thanks

> You can get a new build every day at
> https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.
> 
> Once we get a green light from you, we'll proceed with the uplift strategy.

will this patch be in before FF55? Still not landed, just tested.
(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #24)
> (In reply to Sunil Kumar from comment #22)
> > ps: reddit images sometimes keep loading then convert to newtab on weak
> > 2g/3g connections instead of showing problem loading pages,
> > what criteria should be put in whiteboard and keywords while filing the bug.
> 
> This is being tracked in bug #610357 and it's especially visible on slow
> ISPs and when closing not fully loaded website pages and restoring them
> again.

Is that the right bug? seems off, maybe my english is not good so did not understand correctly.
https://hg.mozilla.org/mozilla-central/rev/32604a6dd45e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
@ Matheus Longaray (:mlongaray) - I'm requesting an uplift request.
Flags: needinfo?(mlongaray)
Tracking 53/54+ for this session restore regression issue.
Whiteboard: The bunch of patch (Bug 1323987 and Bug 1306294) should be backed out.
We may want to verify that this bug is indeed fixed prior to requesting an uplift of these patches. Can one of you guys download the latest version of Nightly and test it against the aforementioned test files?

You can download latest version at https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.

I downloaded _firefox-54.0a1.en-US.linux-i686.tar.bz2_ and could confirm that our patch is working. But let's see if you guys can get the same behavior.

File 1: https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg
76 tabs on session store -> _76_ tabs properly restored after restarting session.

File 2: https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA
59 tabs on session store -> _59_ tabs properly restored after restarting session.

File 3: sessionstore.js attachment
STR2: 194 tabs on session store -> _188_ tabs restored after restarting session. (see comment #16 & comment #17 for better understanding)
Flags: needinfo?(sunilbablusingh)
Flags: needinfo?(mlongaray)
Flags: needinfo?(Virtual)
(In reply to Matheus Longaray (:mlongaray) from comment #31)
> We may want to verify that this bug is indeed fixed prior to requesting an
> uplift of these patches. Can one of you guys download the latest version of
> Nightly and test it against the aforementioned test files?
> 
> You can download latest version at
> https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.
> 
> I downloaded _firefox-54.0a1.en-US.linux-i686.tar.bz2_ and could confirm
> that our patch is working. But let's see if you guys can get the same
> behavior.
> 
> File 1: https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg
> 76 tabs on session store -> _76_ tabs properly restored after restarting
> session.
> 
> File 2: https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA
> 59 tabs on session store -> _59_ tabs properly restored after restarting
> session.
> 
> File 3: sessionstore.js attachment
> STR2: 194 tabs on session store -> _188_ tabs restored after restarting
> session. (see comment #16 & comment #17 for better understanding)

Yes this bug seems to be fixed now and working properly with no loss of data but seems to be hitting this https://bugzilla.mozilla.org/show_bug.cgi?id=610357 more now(now happening more often)
Flags: needinfo?(euthanasia_waltz)
(In reply to Matheus Longaray (:mlongaray) from comment #31)
> We may want to verify that this bug is indeed fixed prior to requesting an
> uplift of these patches. Can one of you guys download the latest version of
> Nightly and test it against the aforementioned test files?
> 
> You can download latest version at
> https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/.
> 
> I downloaded _firefox-54.0a1.en-US.linux-i686.tar.bz2_ and could confirm
> that our patch is working. But let's see if you guys can get the same
> behavior.
> 
> File 1: https://1drv.ms/u/s!AqDeaDGj6aTxgb4MpSkzyOsD5grHNg
> 76 tabs on session store -> _76_ tabs properly restored after restarting
> session.
> 
> File 2: https://1drv.ms/u/s!AqDeaDGj6aTxgb4NQiu_HtoNHAK0YA
> 59 tabs on session store -> _59_ tabs properly restored after restarting
> session.
> 
> File 3: sessionstore.js attachment
> STR2: 194 tabs on session store -> _188_ tabs restored after restarting
> session. (see comment #16 & comment #17 for better understanding)

Yes this bug seems to be fixed now and working properly with no loss of data but seems to be hitting this https://bugzilla.mozilla.org/show_bug.cgi?id=610357 more now(now happening more often)
Flags: needinfo?(sunilbablusingh) → needinfo?(euthanasia_waltz)
Also I can confirm too that the issue is indeed fixed.

(In reply to Sunil Kumar from comment #33)
> Yes this bug seems to be fixed now and working properly with no loss of data
> but seems to be hitting this
> https://bugzilla.mozilla.org/show_bug.cgi?id=610357 more now(now happening
> more often)
The only workaround for now is opening less windows and tabs at once, especially on slow ISP and don't close not fully loaded (in URL term) website. Let's hope that bug #610357 will be also fixed "Soon™".
Status: RESOLVED → VERIFIED
Flags: needinfo?(euthanasia_waltz)
Flags: needinfo?(Virtual)
@ Matheus Longaray (:mlongaray) - I'm requesting an uplift request, as this bug was verified already by 3 persons.
Flags: needinfo?(mlongaray)
(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me) from comment #34)
> Also I can confirm too that the issue is indeed fixed.
> 
> (In reply to Sunil Kumar from comment #33)
> > Yes this bug seems to be fixed now and working properly with no loss of data
> > but seems to be hitting this
> > https://bugzilla.mozilla.org/show_bug.cgi?id=610357 more now(now happening
> > more often)
> The only workaround for now is opening less windows and tabs at once,
> especially on slow ISP and don't close not fully loaded (in URL term)
> website. Let's hope that bug #610357 will be also fixed "Soon™".

The way things are being handled in that bug IDK if going to fixed.
Large tabs is the reason to use FF, and that problem hits especially when suddenly while loading(progress to load eg page name on tab) they turn into new tab.

>fully loaded (in URL term) website

Like said above suddenly while loading turn into newtab and don't see how that can be prevented using less tabs/windows.
(In reply to Matheus Longaray (:mlongaray) from comment #21)
> (In reply to Sunil Kumar from comment #20) 
> > Good work, Glad to hear that you fixed it, now just needs testings to see if
> > it's indeed fixed.
> > when can we test this?
> > also will this be back-ported to FF52 stable & FF53 beta? Those seem
> > affected.
> 
> FF52 is not affected. Bug 1306294 started affecting FF53 only.
> 
> Bug 1323987 got merged for FF54 - as we hope for bug 1343056 (_this_).
> 
> As soon as _this_ bug lands on mozilla-central and you're able to verify the
> fix, we'll figure out an uplift strategy so these two bug fixes can clear
> things up for FF53.

Big thankyou for the quick fix, really appreciate it.

(In reply to atlanto from comment #7)
> That sessionstore.js linked above have some entries like
> 
> {"entries":[],"lastAccessed":1488152376406,"hidden":false,"attributes":{},
> "userContextId":0,"userTypedValue":"https://c2.staticflickr.com/4/3932/
> 15311141280_61a6fca4ea_o.jpg","userTypedClear":1,"image":"",
> "iconLoadingPrincipal": ...
> 
> ContentRestore.restoreTabContent is handling it, so the page is loaded
> normally when the tab was clicked.
> It was saved before bug 1306294 but it is discarded in
> SessionStore._shouldSaveTab for now.

Thanks for that.

(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me) from comment #34)


Thanks for your help too.
Comment on attachment 8843017 [details]
Bug 1343056 - Take userTypedValue into account when saving tabs to disk.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1306294

[User impact if declined]: Data loss when restoring session

[Is this code covered by automated tests?]: SessionStore itself has a substantial automated test suite, but unfortunately, that suite did not catch the regression that these patches fix

[Has the fix been verified in Nightly?]: Yes, verified in m-i (already merged in Aurora)

[Needs manual test from QE? If yes, steps to reproduce]: Yes, see comment #16 (although was manually tested by Virtual_ManPL and Sunil Kumar)

[List of other uplifts needed for the feature/fix]: Bug 1323987 is a _must_ (patch dependencies)

[Is the change risky?]: No

[Why is the change risky/not risky?]: One-liner patch that increments patches from bug 1323987

[String changes made/needed]: None
Flags: needinfo?(mlongaray)
Comment on attachment 8843017 [details]
Bug 1343056 - Take userTypedValue into account when saving tabs to disk.

See comment 38. Note that this needs to land _after_ the patches from bug 1323987, as it's a follow-up.
Attachment #8843017 - Flags: approval-mozilla-beta?
Comment on attachment 8843017 [details]
Bug 1343056 - Take userTypedValue into account when saving tabs to disk.

This is needed to fix bug 1323987, Beta53+
Attachment #8843017 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I can also confirm the session is properly restored using the STR in comment 16 on Fx 53b2, Win 10 x64.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.