900089 - SessionStore.setTabState on pending tab doesn't call _resetTabRestoringState

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[tabmix.onemen]

SessionRetore.setTabState on pending tab doesn't call _resetTabRestoringState

_resetTabRestoringState comment is:
"Reset the restoring state for a particular tab. This will be called when removing a tab or when a tab needs to be reset (it's being overwritten)."

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Oops, didn't mean to assign that to me.

Comment 2

[Abhishek Potnis [:avp]]

Hi Tim,
I would like to work on this bug. Can you please help me get started with it?

Thanks.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Hey Abhishek, that's great to hear! This is what we need to do:

Right above the TabStateCache.delete() call [1] we want to reset the tab's restoring state but only if it currently is restoring. To do that we can just insert the following code:

  if (aTab.linkedBrowser.__SS_restoreState) {
    this._resetTabRestoringState(aTab);
  }

Should you have any more questions don't hesitate to ask here or on IRC.

[1] http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/SessionStore.jsm#1463

Comment 4

[Abhishek Potnis [:avp]]

Attachment: bug-900089-fix.patch

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 811226 [details] [diff] [review]
bug-900089-fix.patch

Review of attachment 811226 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this looks great! Can you please prepare the patch for checkin-needed?

::: browser/components/sessionstore/src/SessionStore.jsm
@@ +1459,5 @@
>        debug("Default view of ownerDocument must have a unique identifier");
>        throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
>      }
> +  
> +    if (aTab.linkedBrowser.__SS_restoreState) {

Nit: the first line above the if statement has some white space in it, can you please remove that?

Comment 6

[Abhishek Potnis [:avp]]

Attachment: Added call to _resetTabRestoringState in SessionRestore.setTabState

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Thanks!

https://hg.mozilla.org/integration/fx-team/rev/8d198483ae3b

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 811701 [details] [diff] [review]
Added call to _resetTabRestoringState in SessionRestore.setTabState

Small hint: Please use the 'checkin-needed' keyword the next time. You can set is using the 'Keywords' field at the top. The checkin flag is only used for bugs with multiple patches that need to be landed.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8d198483ae3b

Comment 10

[Abhishek Potnis [:avp]]

Thanks Tim, will take care about that from next time ! :)