Closed
Bug 852787
Opened 10 years ago
Closed 10 years ago
StrictMode policy violation from LocalBrowserDB.getUrlForKeyword()
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect, P4)
Tracking
(firefox21 fixed, firefox22 fixed, firefox23 fixed)
RESOLVED
FIXED
Firefox 23
People
(Reporter: cpeterson, Assigned: mfinkle)
References
Details
Attachments
(2 files)
1.50 KB,
patch
|
bnicholson
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.77 KB,
patch
|
bnicholson
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #734205 -
Flags: review?(bnicholson) → review+
Assignee | ||
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
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+
Assignee | ||
Comment 4•10 years ago
|
||
Made the change to skip the replace if keywordUrl is null https://hg.mozilla.org/integration/mozilla-inbound/rev/deef421f9169 https://hg.mozilla.org/integration/mozilla-inbound/rev/ba48781b74e4
Assignee | ||
Comment 5•10 years ago
|
||
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?
Assignee | ||
Comment 6•10 years ago
|
||
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?
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/deef421f9169 https://hg.mozilla.org/mozilla-central/rev/ba48781b74e4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Comment 8•10 years ago
|
||
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 9•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #734316 -
Flags: approval-mozilla-beta?
Attachment #734316 -
Flags: approval-mozilla-beta+
Attachment #734316 -
Flags: approval-mozilla-aurora?
Attachment #734316 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/cd024a6af563 https://hg.mozilla.org/releases/mozilla-aurora/rev/2fe1b366eae9 https://hg.mozilla.org/releases/mozilla-beta/rev/e9a9918f8c76 https://hg.mozilla.org/releases/mozilla-beta/rev/f2e3a0d44669
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•