Closed Bug 1589939 Opened 5 years ago Closed 5 years ago

`mozIStorageAsyncConnection.variableLimit` is missing in an artifact build

Categories

(Toolkit :: Data Sanitization, defect, P1)

Product:
Toolkit
Component:
Data Sanitization
Type:
defect

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: itiel_yn8, Unassigned)

References

(Regression)

Details

(Keywords: regression)

STR:

  1. Place the Forget button on the toolbar
  2. Click the button, select any of the options there and click Forget!

AR:
A popup is displayed saying that the recent history is cleared, but in fact this does not happen and the following is logged in the console:

TypeError: this.document.location is null PurgeSessionHistoryChild.jsm:29:9

NS_ERROR_UNEXPECTED: 

TypeError: "Chunk length must be a positive integer"
    chunkArray resource://gre/modules/PlacesUtils.jsm:1922
    removeVisitsByFilter resource://gre/modules/History.jsm:1289
    executeTransaction resource://gre/modules/Sqlite.jsm:1579
    transactionPromise resource://gre/modules/Sqlite.jsm:654
    promise resource://gre/modules/Sqlite.jsm:702
    promise callback*executeTransaction resource://gre/modules/Sqlite.jsm:611
    executeTransaction resource://gre/modules/Sqlite.jsm:1579
    removeVisitsByFilter resource://gre/modules/History.jsm:1287
    removeVisitsByFilter resource://gre/modules/History.jsm:504
    executeBeforeShutdown resource://gre/modules/Sqlite.jsm:420
    executeBeforeShutdown resource://gre/modules/Sqlite.jsm:1437
    withConnectionWrapper resource://gre/modules/PlacesUtils.jsm:1543
    removeVisitsByFilter resource://gre/modules/History.jsm:502
    deleteByRange resource://gre/modules/ClearDataService.jsm:765
    deleteDataInTimeRange resource://gre/modules/ClearDataService.jsm:1190
    promises resource://gre/modules/ClearDataService.jsm:1227
    _deleteInternal resource://gre/modules/ClearDataService.jsm:1225
    deleteDataInTimeRange resource://gre/modules/ClearDataService.jsm:1186
    clearData resource:///modules/Sanitizer.jsm:1113
    clearData resource:///modules/Sanitizer.jsm:1112
    clear resource:///modules/Sanitizer.jsm:391
    sanitizeInternal resource:///modules/Sanitizer.jsm:727
    sanitize resource:///modules/Sanitizer.jsm:285
    forgetButtonCalled resource:///modules/CustomizableWidgets.jsm:906
    handleEvent resource:///modules/CustomizableWidgets.jsm:926
    onViewShowing resource:///modules/CustomizableWidgets.jsm:976
    aEventName resource:///modules/CustomizableUI.jsm:2864
    dispatchCustomEvent resource:///modules/PanelMultiView.jsm:197
    dispatchCustomEvent resource:///modules/PanelMultiView.jsm:1388
    _blockersPromise resource:///modules/PanelMultiView.jsm:231
    promise callback*dispatchAsyncEvent resource:///modules/PanelMultiView.jsm:229
    _openView resource:///modules/PanelMultiView.jsm:819
    _showMainView resource:///modules/PanelMultiView.jsm:785
    _openPopupPromise resource:///modules/PanelMultiView.jsm:524
    promise callback*openPopup resource:///modules/PanelMultiView.jsm:507
    openPopup resource:///modules/PanelMultiView.jsm:284
    showSubView chrome://browser/content/customizableui/panelUI.js:494
    handleWidgetCommand resource:///modules/CustomizableUI.jsm:1929
    buildWidget resource:///modules/CustomizableUI.jsm:1742
    getWidgetNode resource:///modules/CustomizableUI.jsm:1165
    buildArea resource:///modules/CustomizableUI.jsm:975
    registerToolbarNode resource:///modules/CustomizableUI.jsm:889
    registerToolbarNode resource:///modules/CustomizableUI.jsm:3601
    onDOMContentLoaded chrome://browser/content/browser.js:1776
    EventListener.handleEvent* chrome://browser/content/browser.xhtml:132
Sqlite.jsm:714:15
    _transactionQueue resource://gre/modules/Sqlite.jsm:714
    executeTransaction resource://gre/modules/Sqlite.jsm:713
    executeTransaction resource://gre/modules/Sqlite.jsm:1579
    removeVisitsByFilter resource://gre/modules/History.jsm:1287
    removeVisitsByFilter resource://gre/modules/History.jsm:504
    executeBeforeShutdown resource://gre/modules/Sqlite.jsm:420
    executeBeforeShutdown resource://gre/modules/Sqlite.jsm:1437
    withConnectionWrapper resource://gre/modules/PlacesUtils.jsm:1543
    removeVisitsByFilter resource://gre/modules/History.jsm:502
    deleteByRange resource://gre/modules/ClearDataService.jsm:765
    deleteDataInTimeRange resource://gre/modules/ClearDataService.jsm:1190
    promises resource://gre/modules/ClearDataService.jsm:1227
    _deleteInternal resource://gre/modules/ClearDataService.jsm:1225
    deleteDataInTimeRange resource://gre/modules/ClearDataService.jsm:1186
    clearData resource:///modules/Sanitizer.jsm:1113
    clearData resource:///modules/Sanitizer.jsm:1112
    clear resource:///modules/Sanitizer.jsm:391
    sanitizeInternal resource:///modules/Sanitizer.jsm:727
    sanitize resource:///modules/Sanitizer.jsm:285
    forgetButtonCalled resource:///modules/CustomizableWidgets.jsm:906
    handleEvent resource:///modules/CustomizableWidgets.jsm:926
    onViewShowing resource:///modules/CustomizableWidgets.jsm:976
    aEventName resource:///modules/CustomizableUI.jsm:2864
    dispatchCustomEvent resource:///modules/PanelMultiView.jsm:197
    dispatchCustomEvent resource:///modules/PanelMultiView.jsm:1388
    _blockersPromise resource:///modules/PanelMultiView.jsm:231
    dispatchAsyncEvent resource:///modules/PanelMultiView.jsm:229
    _openView resource:///modules/PanelMultiView.jsm:819
    _showMainView resource:///modules/PanelMultiView.jsm:785
    _openPopupPromise resource:///modules/PanelMultiView.jsm:524
    openPopup resource:///modules/PanelMultiView.jsm:507
    openPopup resource:///modules/PanelMultiView.jsm:284
    showSubView chrome://browser/content/customizableui/panelUI.js:494
    handleWidgetCommand resource:///modules/CustomizableUI.jsm:1929
    buildWidget resource:///modules/CustomizableUI.jsm:1742
    getWidgetNode resource:///modules/CustomizableUI.jsm:1165
    buildArea resource:///modules/CustomizableUI.jsm:975
    registerToolbarNode resource:///modules/CustomizableUI.jsm:889
    registerToolbarNode resource:///modules/CustomizableUI.jsm:3601
    onDOMContentLoaded chrome://browser/content/browser.js:1776
    <anonymous> chrome://browser/content/browser.xhtml:132

This doesn't happen on a new profile, but this reproduces on a fairly new profile with no addons at all, also reproduces when restarting with addons disabled.

[Tracking Requested - why for this release]: Possibly broken "Forget!" feature.

Hmm, even though I can't reproduce this on a fresh profile, I managed to use mozregression to find when the TypeError: this.document.location is null PurgeSessionHistoryChild.jsm:29:9 error first appeared, and that's bug 1576908. So.. that's the regressor?

Has Regression Range: --- → yes
status-firefox71: --- → affected
tracking-firefox71: --- → ?
Keywords: regression
Regressed by: 1576908

That seems likely. Tyler, Gijs, can you take a look, please?

Flags: needinfo?(staatsty)
Flags: needinfo?(gijskruitbosch+bugs)
Priority: -- → P1

Tracking, we'll most likely want to uplift a fix in beta 71. Thanks for catching!

tracking-firefox71: ? → +

What history isn't cleared? Places history?

The changes in bug 1576908 do not affect places history (only per-docshell session history, ie whether if you click back/fwd, there's stuff there), so I suspect the error is unrelated to the actual issue. The this.document.location error is also coming from the child actor, so probably the child process, so I don't think it's related to the chunk length / places issue.

It'd really help if we had steps to reproduce that worked on a clean profile.

Flags: needinfo?(staatsty)
Flags: needinfo?(itiel_yn8)
Flags: needinfo?(gijskruitbosch+bugs)

Mark, can you deduce anything from the stack in comment #0 that might give us a clue how to repro this and/or what broke it? I strongly suspect it wasn't bug 1576908.

Flags: needinfo?(standard8)

Hm, in fact maybe this is bug 1588329 which is much more recent and is specifically about this chunking. Lina?

Flags: needinfo?(standard8) → needinfo?(lina)

(In reply to :Gijs (he/him) from comment #5)

What history isn't cleared? Places history?

Yes.

I strongly suspect it wasn't bug 1576908.

You're probably right, because forgetting a specific site from the Library window outputs only the TypeError: "Chunk length must be a positive integer" error.

It'd really help if we had steps to reproduce that worked on a clean profile.

Sorry, none atm :(
Could it be that this reproduces only on a locally built Firefox? I managed to reproduce this on a profile using my installed Nightly, whereas I couldn't when using the locally built Firefox, on the same profile.
I'm also unable to delete there a page from history, no matter in which way (Shift+Del / just Del from the URL bar / Library / sidebar). I see no error in the console when this happens.

Flags: needinfo?(itiel_yn8)
Keywords: regression → regressionwindow-wanted
No longer regressed by: 1576908

Could it be that this reproduces only on a locally built Firefox?

Ohhh, interesting, it definitely could be, if it's linking to the system SQLite and that has a different bound parameter limit! Could you please run the following snippet in Tools > Web Developer > Browser Console (you may need to enable chrome and add-on debugging in Dev Tools settings so that you can type into the console):

PlacesUtils.promiseUnsafeWritableDBConnection().then(db => console.log("Limit", db.variableLimit)).catch(console.error)

What does that line print for you?

Flags: needinfo?(lina) → needinfo?(itiel_yn8)

(In reply to :Lina Cambridge from comment #9)

Could it be that this reproduces only on a locally built Firefox?

Ohhh, interesting, it definitely could be, if it's linking to the system SQLite and that has a different bound parameter limit! Could you please run the following snippet in Tools > Web Developer > Browser Console (you may need to enable chrome and add-on debugging in Dev Tools settings so that you can type into the console):

PlacesUtils.promiseUnsafeWritableDBConnection().then(db => console.log("Limit", db.variableLimit)).catch(console.error)

What does that line print for you?

Running it after Forget'ing (in the same console instance):

ReferenceError: PlacesUtils is not defineddebugger eval code:1:1
    <anonymous> debugger eval code:1

Running it after restarting the browser console:

Promise { "pending" }
​
<state>: "pending"
​
<prototype>: PromiseProto { … }

Limit undefined debugger eval code:1:68
Flags: needinfo?(itiel_yn8)
Flags: needinfo?(lina)

Thanks! But hmmm, that's unexpected! 😮 Are you using an artifact build locally?

What about PlacesUtils.history.DBConnection.variableLimit?

Flags: needinfo?(lina) → needinfo?(itiel_yn8)

(In reply to :Lina Cambridge from comment #11)

Thanks! But hmmm, that's unexpected! 😮 Are you using an artifact build locally?

Yes.

What about PlacesUtils.history.DBConnection.variableLimit?

undefined

Flags: needinfo?(itiel_yn8) → needinfo?(lina)

Can you please run ./mach xpcshell-test toolkit/components/places/tests/history/test_removeByFilter.js locally, and see if it throws the same "Chunk length must be a positive integer" error?

I just pulled the latest central, clobbered, and rebuilt, and got it to run successfully. With an older artifact build, the test failed, since the variableLimit attribute requires native code changes and updated artifacts.

Flags: needinfo?(lina)

(In reply to :Lina Cambridge from comment #13)

Can you please run ./mach xpcshell-test toolkit/components/places/tests/history/test_removeByFilter.js locally, and see if it throws the same "Chunk length must be a positive integer" error?

I just pulled the latest central, clobbered, and rebuilt, and got it to run successfully. With an older artifact build, the test failed, since the variableLimit attribute requires native code changes and updated artifacts.

------------------
toolkit/components/places/tests/history/test_removeByFilter.js
  FAIL toolkit/components/places/tests/history/test_removeByFilter.js - xpcshell return code: 0
ERROR Unexpected exception TypeError: Chunk length must be a positive integer at resource://gre/modules/PlacesUtils.jsm:1922
chunkArray@resource://gre/modules/PlacesUtils.jsm:1922:13
removeByFilter/<@resource://gre/modules/History.jsm:1430:7
executeTransaction/<@resource://gre/modules/Sqlite.jsm:1579:62
executeTransaction/promise</transactionPromise<@resource://gre/modules/Sqlite.jsm:654:28
async*executeTransaction/promise<@resource://gre/modules/Sqlite.jsm:702:9
_do_main@c:\mozilla-central\testing\xpcshell\head.js:246:6
_execute_test@c:\mozilla-central\testing\xpcshell\head.js:573:5
@-e:1:1
Flags: needinfo?(lina)

I'm...not sure what the right next steps are. Looks like outdated artifacts? Gijs, any ideas?

Does this only happen with local artifacts builds, or can you repro in an official Nightly or Beta, or a full local build?

Flags: needinfo?(lina)
Keywords: regressionwindow-wanted
Regressed by: 1588329
Summary: The 'Forget' option fails silently - TypeError: "Chunk length must be a positive integer" → `mozIStorageAsyncConnection.variableLimit` is missing in an artifact build
Keywords: regression

(In reply to :Lina Cambridge from comment #15)

Does this only happen with local artifacts builds, or can you repro in an official Nightly or Beta, or a full local build?

On the latest Nightly I created a new profile and tested there, this does not reproduce. And the output of the commands above is 999.
I'm not sure how to change the state I'm in right now to full local build (from artifact) to test.

I did hg pull a few hours ago if that matters btw.

(In reply to :Lina Cambridge from comment #15)

I'm...not sure what the right next steps are. Looks like outdated artifacts? Gijs, any ideas?

Re-running ./mach build should update the artifacts if the builds weren't ready the first time the artifact build ran, I think? Itiel, what revision are you building off of?

When you run ./mach build on the artifact build there should be output along the lines of:

 0:29.12 Installing from remote pushhead 4b332005f92551e4c96e74a150a601764f76aa02 on mozilla-central

which you can compare with hg wip or hg parent to check if you're installing artifacts that match the revision that you're actually on, or if you're running front-end code on revision X on binary blobs from revision Y, which might lead to problems like this one.

Flags: needinfo?(itiel_yn8)

Hmm, so I ran the following set of commands:

hg pull
hg update
hg up central
./mach build

and I'm now unable to reproduce.

So.. RESOLVED INVALID?

Sorry for the noise!

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(itiel_yn8)
Resolution: --- → INVALID

Thanks for getting back to us!

I guess it's safe to untrack at this point.

status-firefox71: affected → ---
Flags: needinfo?(pascalc)
tracking-firefox71: + → ---
Flags: needinfo?(pascalc)
You need to log in before you can comment on or make changes to this bug.