Closed Bug 852787 Opened 9 years ago Closed 9 years ago

StrictMode policy violation from LocalBrowserDB.getUrlForKeyword()

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect, P4)

All
Android
defect

Tracking

(firefox21 fixed, firefox22 fixed, firefox23 fixed)

RESOLVED FIXED
Firefox 23
Tracking Status
firefox21 --- fixed
firefox22 --- fixed
firefox23 --- fixed

People

(Reporter: cpeterson, Assigned: mfinkle)

References

Details

Attachments

(2 files)

While running the Robocop test "TEST_PATH=testLoad make mochitest-robotium", I saw the following StrictMode violation when the test was entering a URL into the address bar:

D/StrictMode(29467): StrictMode policy violation; ~duration=16 ms: android.os.StrictMode$StrictModeDiskReadViolation: policy=31 violation=2
D/StrictMode(29467): 	at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1107)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteConnection.applyBlockGuardPolicy(SQLiteConnection.java:1034)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteConnection.executeForCursorWindow(SQLiteConnection.java:835)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteSession.executeForCursorWindow(SQLiteSession.java:836)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteQuery.fillWindow(SQLiteQuery.java:62)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteCursor.fillWindow(SQLiteCursor.java:143)
D/StrictMode(29467): 	at android.database.sqlite.SQLiteCursor.getCount(SQLiteCursor.java:133)
D/StrictMode(29467): 	at android.content.ContentResolver.query(ContentResolver.java:390)
D/StrictMode(29467): 	at android.content.ContentResolver.query(ContentResolver.java:315)
D/StrictMode(29467): 	at org.mozilla.gecko.db.LocalBrowserDB.getUrlForKeyword(LocalBrowserDB.java:520)
D/StrictMode(29467): 	at org.mozilla.gecko.db.BrowserDB.getUrlForKeyword(BrowserDB.java:189)
D/StrictMode(29467): 	at org.mozilla.gecko.AwesomeBar.openUserEnteredAndFinish(AwesomeBar.java:394)
D/StrictMode(29467): 	at org.mozilla.gecko.AwesomeBar.access$300(AwesomeBar.java:50)
D/StrictMode(29467): 	at org.mozilla.gecko.AwesomeBar$3.onKeyPreIme(AwesomeBar.java:159)
D/StrictMode(29467): 	at org.mozilla.gecko.CustomEditText.onKeyPreIme(CustomEditText.java:34)
D/StrictMode(29467): 	at android.view.View.dispatchKeyEventPreIme(View.java:7179)
D/StrictMode(29467): 	at android.view.ViewGroup.dispatchKeyEventPreIme(ViewGroup.java:1338)
D/StrictMode(29467): 	at android.view.ViewGroup.dispatchKeyEventPreIme(ViewGroup.java:1338)
D/StrictMode(29467): 	at android.view.ViewGroup.dispatchKeyEventPreIme(ViewGroup.java:1338)
D/StrictMode(29467): 	at android.view.ViewGroup.dispatchKeyEventPreIme(ViewGroup.java:1338)
D/StrictMode(29467): 	at android.view.ViewGroup.dispatchKeyEventPreIme(ViewGroup.java:1338)
D/StrictMode(29467): 	at android.view.ViewRootImpl.deliverKeyEvent(ViewRootImpl.java:3606)
D/StrictMode(29467): 	at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:3161)
D/StrictMode(29467): 	at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:4292)
D/StrictMode(29467): 	at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:4271)
D/StrictMode(29467): 	at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:4363)
D/StrictMode(29467): 	at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:179)
D/StrictMode(29467): 	at android.os.MessageQueue.nativePollOnce(Native Method)
D/StrictMode(29467): 	at android.os.MessageQueue.next(MessageQueue.java:125)
D/StrictMode(29467): 	at android.os.Looper.loop(Looper.java:124)
D/StrictMode(29467): 	at android.app.ActivityThread.main(ActivityThread.java:5226)
D/StrictMode(29467): 	at java.lang.reflect.Method.invokeNative(Native Method)
D/StrictMode(29467): 	at java.lang.reflect.Method.invoke(Method.java:511)
D/StrictMode(29467): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:795)
D/StrictMode(29467): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:562)
D/StrictMode(29467): 	at dalvik.system.NativeStart.main(Native Method)
First patch is just a small optimization. We should never even attempt to do a DB query for a keyword match if the text (url entered by the user) does not look like a search query. 
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/util/StringUtils.java#28

Text that looks like a URL is not a keyword. The new domain/hostname auto-completion code will make this situation (text is a URL) happen even more.
Assignee: nobody → mark.finkle
Attachment #734205 - Flags: review?(bnicholson)
Attachment #734205 - Flags: review?(bnicholson) → review+
This patch builds on the first. It moves the keyword search to the background thread. Since "url" becomes final, I had to tweak the logic a tiny bit.

Testing shows no more strict violation. keyword searches do work and we fallback to google searches correctly too, if a keyword is not defined.
Attachment #734316 - Flags: review?(bnicholson)
Comment on attachment 734316 [details] [diff] [review]
patch 2: move keyword searches to background thread

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

This will always do a replace on the string for %s, but we should only do a replace if we have a keyword URL. r+ with that fixed.
Attachment #734316 - Flags: review?(bnicholson) → review+
Comment on attachment 734205 [details] [diff] [review]
patch1: limit keyword searches to text that looks like a search query

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Accessing the DB on the UI thread can cause hangs and locks, like the test failures linked in bug 856621
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): some risk if I made a stupid mistake in the patch. Seems to work well and reduces the DB work too.
String or IDL/UUID changes made by this patch: none
Attachment #734205 - Flags: approval-mozilla-beta?
Attachment #734205 - Flags: approval-mozilla-aurora?
Comment on attachment 734316 [details] [diff] [review]
patch 2: move keyword searches to background thread

[Approval Request Comment]
Same as above
Attachment #734316 - Flags: approval-mozilla-beta?
Attachment #734316 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/deef421f9169
https://hg.mozilla.org/mozilla-central/rev/ba48781b74e4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Approving in favor of fixing intermittent failures given in bug 856621 and may help avoid hangs on UI thread DB access.

Early enough in the beta cycle where the risk is manageable . Please watch for any new regressions this may cause due to less baketime on m-c aurora.
Comment on attachment 734205 [details] [diff] [review]
patch1: limit keyword searches to text that looks like a search query

Please land asap to get this into for Beta 2 going to build shortly.
Attachment #734205 - Flags: approval-mozilla-beta?
Attachment #734205 - Flags: approval-mozilla-beta+
Attachment #734205 - Flags: approval-mozilla-aurora?
Attachment #734205 - Flags: approval-mozilla-aurora+
Attachment #734316 - Flags: approval-mozilla-beta?
Attachment #734316 - Flags: approval-mozilla-beta+
Attachment #734316 - Flags: approval-mozilla-aurora?
Attachment #734316 - Flags: approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.