1445226 - Port Bug 1435446 part 3 "Add a default transaction type for storage connections" to Calendar - Many Xpcshell and Mozmill failures

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testAnnualRecurrence.js | testAnnualRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testBiweeklyRecurrence.js | testBiweeklyRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testDailyRecurrence.js | testDailyRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testLastDayOfMonthRecurrence.js | testLastDayOfMonthRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testWeeklyNRecurrence.js | testWeeklyNRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testWeeklyUntilRecurrence.js | testWeeklyUntilRecurrence.js::setupModule
TEST-UNEXPECTED-FAIL | /Users/cltbld/tasks/task_1520941181/build/tests/mozmill/cal-recurrence/testWeeklyWithExceptionRecurrence.js | testWeeklyWithExceptionRecurrence.js::setupModule

Those logs say:
https://taskcluster-artifacts.net/Zd9jKn_wQ-ueKPWrTRliIw/0/public/logs/live_backing.log
JavaScript error: resource://calendar/modules/calUtils.jsm -> file:///Users/cltbld/tasks/task_1520941181/build/application/Thunderbird%20Daily.app/Contents/Resources/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calCalendarManager.js, line 326: TypeError: db.beginTransactionAs is not a function

This was removed here:
https://hg.mozilla.org/mozilla-central/rev/f8a3b9547dbc#l3.24

We use it a few times:
https://dxr.mozilla.org/comm-central/search?q=beginTransactionAs&redirect=false

There are also failures in
mozmill/testBasicFunctionality.js | testBasicFunctionality.js::testSmokeTest
mozmill/testLocalICS.js | testLocalICS.js::testLocalICS
mozmill/testTodayPane.js | testTodayPane.js::setupModule
which may be the same thing.

In the logs I see 
https://taskcluster-artifacts.net/Q-BC7bsOSUaWrJmB08O2MQ/0/public/logs/live_backing.log
INFO -  TEST-START | /Users/cltbld/tasks/task_1520941342/build/tests/mozmill/testBasicFunctionality.js | testSmokeTest
INFO -  TypeError: view is null
and also
INFO -  JavaScript error: chrome://calendar/content/calendar-management.js, line 121: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "right-hand side of 'in' should be an object, got undefined" {file: "resource://calendar/modules/calUtils.jsm -> file:///Users/cltbld/tasks/task_1520941342/build/application/Thunderbird%20DailyDebug.app/Contents/Resources/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calCalendarManager.js" line: 563}]'[JavaScript Error: "right-hand side of 'in' should be an object, got undefined" {file: "resource://calendar/modules/calUtils.jsm -> file:///Users/cltbld/tasks/task_1520941342/build/application/Thunderbird%20DailyDebug.app/Contents/Resources/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calCalendarManager.js" line: 563}]' when calling method: [calICalendarManager::registerCalendar]

but that may be follow-on errors.

Marco, what's the replacement? Maybe:
db.defaultTransaction = xx;
db.beginTransaction();

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1445226-replace-use-of-beginTransactionAs.patch (v1)

So here is the replacement. Still untested, my local build isn't finished yet.

Comment 2

[Jorg K (CEST = GMT+2)]

Hmm, running one of the failing Xpcshell tests
  mozilla/mach xpcshell-test calendar/test/unit/test_alarmutils.js
I get
JavaScript error: resource://calendar/modules/calUtils.jsm -> file:///c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calCalendarManager.js, line 326: NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN: Cannot modify properties of a WrappedNative

so
db.defaultTransaction = Components.interfaces.mozIStorageConnection.TRANSACTION_EXCLUSIVE;
isn't the right thing to do.

Oops, that should be defaultTransactionType as can be seen here:
https://hg.mozilla.org/mozilla-central/rev/f8a3b9547dbc#l7.33

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1445226-replace-use-of-beginTransactionAs.patch (v2)

This appears to be working.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c3a61fde3260
Port Bug 1435446: Replace use of beginTransactionAs(). rs=bustage-fix DONTBUILD

Comment 5

[Jorg K (CEST = GMT+2)]

C-C doesn't look so good right now, but this worked:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c376290a7122083d5dfbe11ca12d9bb15504d484

Comment 6

[Jorg K (CEST = GMT+2)]

Philipp, please set up target milestone 6.3.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8958473 [details] [diff] [review]
1445226-replace-use-of-beginTransactionAs.patch (v2)

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

r+ with these changes:

::: calendar/providers/storage/calStorageCalendar.js
@@ +230,5 @@
>                      }
>                  }
>                  try {
>                      // hold lock on storage.sdb until we've migrated data from storage.sdb:
> +                    this.mDB.defaultTransactionType = Components.interfaces.mozIStorageConnection.TRANSACTION_EXCLUSIVE;

Given this sets a default, I'd prefer to move this line directly after the respective openDatabase call.

@@ +272,5 @@
>  
>              // WARNING: This is a somewhat fragile process. Great care should be
>              // taken during future schema upgrades to make sure this still
>              // works.
> +            this.mDB.defaultTransactionType = Components.interfaces.mozIStorageConnection.TRANSACTION_EXCLUSIVE;

Same here.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1445226-follow-up.patch

Note the |this.mDB = localDB;|.

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1445226-follow-up.patch

Oops.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1445226-follow-up.patch

3rd time lucky, sorry about the noise.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8958748 [details] [diff] [review]
1445226-follow-up.patch

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

Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7e4813dc7c60
Follow-up: Move setting defaultTransactionType to after openDatabase(). r=philipp