bug fix for test breakage by 594644

RESOLVED FIXED

Status

Firefox Graveyard
Panorama
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: iangilman, Assigned: iangilman)

Tracking

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 496060 [details] [diff] [review]
patch v1

Bug 594644 breaks some session store tests; this patch fixes  that by taking advantage of the new session store events from bug 615394.
Attachment #496060 - Flags: review?(dietrich)
(Assignee)

Comment 1

7 years ago
Requesting blocking as it blocks a blocker.
blocking2.0: --- → ?
What blocker does this block? I'm going to hold off on this review until bug 615394 is finished up.
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> What blocker does this block? I'm going to hold off on this review until bug
> 615394 is finished up.

Bug 594644 can't land without this fix or we'll get oranges, so this bug is holding that bug up (though it's not marked as blocking that bug here in bugzilla, as that would create a circular reference).
Comment on attachment 496060 [details] [diff] [review]
patch v1

r+ with one fix:

since .sessionstoreReady and .sessionstoreBusy are also manually used for private browsing state changes, i think you should rename the methods (and the counter) to something abstract from the storage implementation, and more descriptive of the action taken, like tabviewStoragePause/tabviewStorageReady.
Attachment #496060 - Flags: review?(dietrich) → review+
blocking2.0: ? → final+
(Assignee)

Updated

7 years ago
Assignee: nobody → ian
Status: NEW → ASSIGNED
Doesn't block, but approved for landing when appropriate. a=me
blocking2.0: final+ → -
(Assignee)

Comment 6

7 years ago
(In reply to comment #4)
> Comment on attachment 496060 [details] [diff] [review]
> patch v1
> 
> r+ with one fix:
> 
> since .sessionstoreReady and .sessionstoreBusy are also manually used for
> private browsing state changes, i think you should rename the methods (and the
> counter) to something abstract from the storage implementation, and more
> descriptive of the action taken, like tabviewStoragePause/tabviewStorageReady.

I've updated the names and given the routines more descriptive comments:

  // ----------
  // Function: storageBusy
  // Pauses the storage activity that conflicts with sessionstore updates and 
  // private browsing mode switches. Calls can be nested. 
  storageBusy: function UI_storageBusy() {
    if (!this._storageBusyCount)
      TabItems.pauseReconnecting();
    
    this._storageBusyCount++;
  },
  
  // ----------
  // Function: storageReady
  // Resumes the activity paused by storageBusy, and updates for any new group
  // information in sessionstore. Calls can be nested. 
  storageReady: function UI_storageReady() {
    this._storageBusyCount--;
    if (!this._storageBusyCount) {
      let hasGroupItemsData = GroupItems.load();
      if (!hasGroupItemsData)
        this.reset(false);
  
      TabItems.resumeReconnecting();
    }
  },

I didn't go with .storagePause, as we're not actually pausing all storage usage.
(Assignee)

Comment 7

7 years ago
http://hg.mozilla.org/mozilla-central/rev/f453924d5fe1
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Depends on: 623792

Updated

7 years ago
Depends on: 586198

Updated

7 years ago
Depends on: 624102
Depends on: 624265

Updated

7 years ago
Whiteboard: [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.