1077715 - [System] Browser bookmarks not migrated to homescreen icons on OTA from 2.0 to 2.2

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Yeojin Chung [:YeojinC]]

Attachment: logcat_20141003_BookmarkNotMigrating.txt

Description:
When user has existing browser bookmarks on 2.0 then performs an OTA update to 2.2, the bookmarks are not migrated to homescreen icons after update. 
   
Repro Steps:
1) Update a Flame device to 2.0 BuildID: 20140917000200
2) Change update channel and url on device to appropriate 2.2 build (20141003040207)
3) Create new bookmarks on Browser app.
4) Perform OTA update.
  
Actual:
Existing browser bookmarks are not migrated to homescreen icons after 2.0 to 2.2 OTA. 
  
Expected: 
All browser bookmarks become homescreen icons after 2.0 to 2.2 OTA.

[Before OTA - Flame 2.0]
 
Environmental Variables:
Device: Flame 2.0 (319mb)(Full Flash)
BuildID: 20141003000204
Gaia: fa797854bfe708129ed54a158ad4336df1015c39
Gecko: 9b7fd1f78a15
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
  
[After OTA - Flame 2.2]

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141003040207
Gaia: d711d1e469eeeecf25a02b2407a542a598918b2c
Gecko: b85c260821ab
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Repro Frequency: 100%
See attached: logcat, screenshot

Comment 1

[Yeojin Chung [:YeojinC]]

This issue does NOT reproduce when upgrading from 2.0 to 2.2:

[Before OTA - Flame 2.0]
 
Environmental Variables:
Device: Flame 2.0 (319mb)(Full Flash)
BuildID: 20141003000204
Gaia: fa797854bfe708129ed54a158ad4336df1015c39
Gecko: 9b7fd1f78a15
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
  
[After OTA - Flame 2.1]

Environmental Variables:
Device: Flame 2.1
BuildID: 20141003000203
Gaia: 9861c61ec302fb0316c753a2e1c0f592180515fd
Gecko: da68900d1c66
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The bookmarks appear in the homescreen after OTA upgrade.

Comment 2

[Yeojin Chung [:YeojinC]]

Attachment: 2.2OTA_BookmarksNotMigrated

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

This is a regression from 2.1 and a loss of data so nominating this 2.2?

Comment 4

[Brogan Zumwalt [Inactive]]

This bug fails verification of bug 1062984

Comment 5

[Candice Serran (:cserran)]

changing as this is an update bug not a new feature

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

From the logcat:

10-03 14:54:29.691  1232  1270 W GeckoConsole: [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:5510"]
10-03 14:54:29.711  1232  1232 W GeckoConsole: [JavaScript Error: "UnknownError" {file: "app://system.gaiamobile.org/js/migrators/browser_migrator.js" line: 105}]
10-03 14:54:29.711  1232  1232 W GeckoConsole: [JavaScript Error: "TransactionInactiveError: A request was placed against a transaction which is currently not active, or which is finished." {file: "app://system.gaiamobile.org/js/migrators/browser_migrator.js" line: 105}]
1

Line 105 is: placesStore.get(bookmark.uri).onsuccess = makeBookmarkProcessor(bookmark);

While the error in ActorsParent line 5510 (https://hg.mozilla.org/mozilla-central/file/b85c260821ab/dom/indexedDB/ActorsParent.cpp#l5510) is:

PBlobParent* actor =
  BackgroundParent::GetOrCreateActorForBlobImpl(aBackgroundActor, impl);
if (!actor) {
  // This can only fail if the child has crashed.
  IDB_REPORT_INTERNAL_ERR();
  return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
}

So that's a little weird. ni?'ing bent though he's probably just gonna say "the comment says it all". 

This is in the system app, which doesn't appear to have crashed in the logcat, so I'm not quite sure how we could have the actor failing.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Bent, see https://bugzilla.mozilla.org/show_bug.cgi?id=1077715#c6

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

Kyle, can you handle this?

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Oh, yeah, already was, just forgot to assign it to myself.

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Can QA run this STR again and see if it still happens? It's been over 3 months, there's been enough changes to 2.2 that this might've worked itself out.

Comment 11

[Brogan Zumwalt [Inactive]]

Issue still occurs in latest Flame 2.2 when OTA-ing from Flame 2.0 build 20150107000201

Actual Results: Existing browser bookmarks are not migrated to homescreen icons after 2.0 to 2.2 OTA. 

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20150108010221
Gaia: d4dac29613076bdba3cb8adc217deadb08a2ac20
Gecko: 70de2960aa87
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 12

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Thanks for the recheck.

It looks like we create our accessor for the places db, then try to use the accessor in the success callback to openCursor, which is where our failure happens. I thought this was a valid way to continue the lifetime of the transaction that created the places db accessor, but it possibly isn't? Will follow up with bent and others on this.

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, reran test myself. Database file move actually still works fine somehow, so this is still something with IDB. Errors are still the same as in the logcat attached to the bug.

Comment 14

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I think a DEBUG build would help us a lot to see what was failing in the Blob path

Comment 15

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Have so far traced this back to an issue with getting the file size for a blob. 

W/GeckoConsole(11477): [JavaScript Error: "GET FILE SIZE FAILED"]
W/GeckoConsole(11477): [JavaScript Error: "/data/local/storage/default/1006+f+app+++system.gaiamobile.org/idb/2959517650brreosw.files/8"]
W/GeckoConsole(11477): [JavaScript Error: "CANNOT GET FILE SIZE"]
W/GeckoConsole(11477): [JavaScript Error: "BLOB NOT MUTABLE"]

/data/local/storage/default/1006+f+app+++system.gaiamobile.org/idb/2959517650brreosw.files/ exists, but there's nothing in the directory below it, so I'm not sure what the file '8' is supposed to be.

ni'ing bent again so he can translate.

Comment 16

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Was forgetting to pass attach to run-gdb.sh. Oops.

So, here's our backtrace:

#0  0xb51f3396 in mozilla::dom::FileImplFile::GetSize (this=0xaf102520, aRv=...) at ../../../dom/base/File.cpp:965
#1  0xb51f3276 in mozilla::dom::FileImplBase::SetMutable (this=0xaf102520, aMutable=<optimized out>) at ../../../dom/base/File.cpp:921
#2  0xb570180e in mozilla::dom::BlobParent::GetOrCreateFromImpl<mozilla::ipc::PBackgroundParent> (aManager=aManager@entry=0xb30aaf10, aBlobImpl=0xaf102520) at ../../../dom/ipc/Blob.cpp:3801
#3  0xb5701a48 in mozilla::dom::BlobParent::GetOrCreate (aManager=aManager@entry=0xb30aaf10, aBlobImpl=<optimized out>) at ../../../dom/ipc/Blob.cpp:3735
#4  0xb4f30a56 in mozilla::ipc::BackgroundParent::GetOrCreateActorForBlobImpl (aBackgroundActor=aBackgroundActor@entry=0xb30aaf10, aBlobImpl=<optimized out>) at ../../../ipc/glue/BackgroundImpl.cpp:865
#5  0xb56d5c4e in ConvertBlobsToActors (aFileInfos=..., aActors=..., aFiles=..., aFileManager=<optimized out>, aBackgroundActor=0xb30aaf10) at ../../../dom/indexedDB/ActorsParent.cpp:5594
#6  mozilla::dom::indexedDB::(anonymous namespace)::ConvertBlobsToActors (aBackgroundActor=0xb30aaf10, aFileManager=<optimized out>, aFiles=..., aActors=..., aFileInfos=...)
    at ../../../dom/indexedDB/ActorsParent.cpp:5527
#7  0xb56d8d20 in mozilla::dom::indexedDB::(anonymous namespace)::ObjectStoreGetRequestOp::ConvertResponse (this=this@entry=0xaed60ef0, aIndex=aIndex@entry=0, aSerializedInfo=...)
    at ../../../dom/indexedDB/ActorsParent.cpp:15126
#8  0xb56d99ce in mozilla::dom::indexedDB::(anonymous namespace)::ObjectStoreGetRequestOp::GetResponse (this=0xaed60ef0, aResponse=...) at ../../../dom/indexedDB/ActorsParent.cpp:15260
#9  0xb56ccf62 in SendSuccessResult (this=0xaed60ef0) at ../../../dom/indexedDB/ActorsParent.cpp:14453
#10 mozilla::dom::indexedDB::(anonymous namespace)::NormalTransactionOp::SendSuccessResult (this=0xaed60ef0) at ../../../dom/indexedDB/ActorsParent.cpp:14447
#11 0xb56d6d30 in RunOnOwningThread (this=0xaed60ef0) at ../../../dom/indexedDB/ActorsParent.cpp:13134
#12 mozilla::dom::indexedDB::(anonymous namespace)::TransactionDatabaseOperationBase::Run (this=0xaed60ef0) at ../../../dom/indexedDB/ActorsParent.cpp:13175
#13 0xb4de6cd8 in nsThread::ProcessNextEvent (this=0xb2ac4120, aMayWait=<optimized out>, aResult=0xb1d3fd27) at ../../../xpcom/threads/nsThread.cpp:855
#14 0xb4df2b70 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=true) at /share/code/mozbuild/B2G/gecko/xpcom/glue/nsThreadUtils.cpp:265
#15 0xb4f3279a in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0xb2ae8610, aDelegate=0xb21f8e80) at ../../../ipc/glue/MessagePump.cpp:368
#16 0xb4f26f24 in MessageLoop::RunInternal (this=this@entry=0xb21f8e80) at ../../../ipc/chromium/src/base/message_loop.cc:233
#17 0xb4f26fd8 in RunHandler (this=0xb21f8e80) at ../../../ipc/chromium/src/base/message_loop.cc:226
#18 MessageLoop::Run (this=this@entry=0xb21f8e80) at ../../../ipc/chromium/src/base/message_loop.cc:200
#19 0xb4de6f26 in nsThread::ThreadFunc (aArg=0xb2ac4120) at ../../../xpcom/threads/nsThread.cpp:356
#20 0xb69bb1da in _pt_root (arg=0xb2108a80) at ../../../../../nsprpub/pr/src/pthreads/ptthread.c:212
#21 0xb6ea822c in __thread_entry (func=0xb69bb141 <_pt_root>, arg=0xb2108a80, tls=0xb1d3fdd0) at bionic/libc/bionic/pthread_create.cpp:105
#22 0xb6ea83c4 in pthread_create (thread_out=0xbe9787fc, attr=<optimized out>, start_routine=0xb69bb141 <_pt_root>, arg=0x78) at bionic/libc/bionic/pthread_create.cpp:224
#23 0x00000000 in ?? ()

Comment 17

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I'd like to look at the database and files directory before and after the migration. Maybe we're goofing in an upgrade path?

Comment 18

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, while zipping up the db directory to post here, I just realized there's actually files in the 16+f+app+++browser.gaiamobile.org/idb/2959517650brreosw/ directory. When I landed the original bookmark migration, I never saw files in that directory and just copied the .sqlite file, ignoring the directory. I guess I need to copy both the file and the directory over?

Still not sure why 2.0 to 2.1 upgrade works, but 2.0 to 2.2 doesn't due to this?

Comment 19

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Browser DB directory before migration

Comment 20

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ah yes. You need to copy those over. I think our migration code will automatically rename the "2959517650brreosw" directory to "2959517650brreosw.files", but if not you should copy over and rename it like that.

Comment 21

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Copy sqlite directory when doing b2g browser migration

Just tested by doing an upgrade by hand from 2.0 to 2.2. Copying the directory as well as the file seems to fix this. r?'ing fabrice since he reviews bug 938171, which is where this came in.

Comment 22

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8547702 [details] [diff] [review]
Patch 1 (v1) - Copy sqlite directory when doing b2g browser migration

If you want this kind of migration to work in the future you'll need to look for the "2959517650brreosw.files" dir too. But this should work for now I think.

Comment 23

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #22)
> Comment on attachment 8547702 [details] [diff] [review]
> Patch 1 (v1) - Copy sqlite directory when doing b2g browser migration
> 
> If you want this kind of migration to work in the future you'll need to look
> for the "2959517650brreosw.files" dir too. But this should work for now I
> think.

Right now there isn't really a "future" for this. This upgrade will only happen from v2.0 to whatever, so it should never change. If we need to generalize the database move procedure in the future I suppose we can, but what sad, sad times those would be.

Comment 24

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

https://bugzilla.mozilla.org/attachment.cgi?id=8547702

Comment 25

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8547702 [details] [diff] [review]
Patch 1 (v1) - Copy sqlite directory when doing b2g browser migration

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 846200
User impact if declined: Bookmarks will not migrate when updating from v2.0 to v2.2
Testing completed: Hand tested on phone
Risk to taking this patch (and alternatives if risky): Only runs during upgrade, so small risk.
String or UUID changes made by this patch: None

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0630fff937bb

Comment 27

[bhavana bajaj [:bajaj]]

Can QA help confirm this is fixed on current 3.0 nightlies before we uplift ?

Comment 28

[Parul Mathur [:pragmatic] - out of office - please ni? [:bajaj] instead]

:ktucker, can you please help verify that this is fixed on latest 3.0 nightly build?

Comment 29

[Brogan Zumwalt [Inactive]]

Issue still occurs when OTAing from Flame 2.0 (20150116000204) to Flame 3.0 (20150116010205)

Changed OTA channel and url to lead to 3.0. Added 4 bookmarks in browser, then downloaded and installed update. No icons migrated to homescreen.

Device: Flame 3.0 Master
BuildID: 20150116010205
Gaia: 401e981f51cf047292d101c785be8ec48bf75e8c
Gecko: cac6192956ab
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 30

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

The version of 3.0 listed in comment 29 does not contain the patch for this bug. It was built 5 hours before the patch made it into mozilla-central. Remarking for verification with a 3.0 version that contains the patch.

Comment 31

[Brogan Zumwalt [Inactive]]

I will need to recheck again on Tuesday as the build information listed in comment 29 is the latest nightly listed on the pvt server.

Comment 32

[Brogan Zumwalt [Inactive]]

Issue is verified fixed on latest Flame 3.0. 

Issue no longer occurs when OTAing from Flame 2.0 (20150116000204) to Flame 3.0 (20150120010227)
All browser bookmarks are migrated to homescreen icons after OTA update from 2.0 to 3.0.

Device: Flame 3.0 Master
BuildID: 20150120010227
Gaia: a5c5ac093814a80b0627514c3bd5f9e96c096a4b
Gecko: c1c6840d9255
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9439cb4863f3