Skip to content

Commit 8da9283

Browse files
ADFA-5176: Do not build the shared interceptor during Activity construction
HelpActivity and IDETooltipWebViewFragment forced DocumentationRequestInterceptor.shared from a property initializer, so the whole lazy ran during construction, before onCreate, on the main thread. Two consequences. Environment.DOC_DB is a plain static File with no initializer, assigned only by Environment.init(). DeviceProtectedApplicationLoader wraps that call in runCatching and the credential-protected loader returns before it when storage is not ready, so null is a state the app can really be in -- and the non-null parameter turned it into an NPE that killed the activity before it existed. shared is nullable now and declines instead, which puts the request back on the local web server: the same thing a null from intercept() already means everywhere else. The lazy also stats external storage for the nointercept sentinel. On the main thread that is a disk read under a StrictMode policy built with detectAll(), and on a contended FUSE mount it stalls the frame that opens the screen. Touched from shouldInterceptRequest instead, on a WebView thread, the way FAQActivity already did it. Not covered here: intercept() still has no throw guard, so an Error (an OOM decoding a large row) escapes onto a Chromium thread rather than falling through to the server the way the class documents. That is a separate finding from the same review. Found in review of PR #1726.
1 parent 3be53d3 commit 8da9283

4 files changed

Lines changed: 40 additions & 13 deletions

File tree

app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class FAQActivity : EdgeToEdgeIDEActivity() {
7171
view: WebView,
7272
request: WebResourceRequest,
7373
): WebResourceResponse? =
74-
DocumentationRequestInterceptor.shared.intercept(request)
74+
DocumentationRequestInterceptor.shared?.intercept(request)
7575
?: super.shouldInterceptRequest(view, request)
7676
}
7777

app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ import com.itsaky.androidide.documentation.DocumentationRequestInterceptor
3838
class IDETooltipWebviewFragment : Fragment() {
3939
private lateinit var webView: WebView
4040
private lateinit var website : String
41-
private val documentation = DocumentationRequestInterceptor.shared
41+
// by lazy: an initializer here runs during Fragment construction on the main thread, where the
42+
// interceptor's sentinel check is a disk read StrictMode reports and a missing database is a
43+
// crash before the view exists. See HelpActivity for the same note.
44+
private val documentation by lazy { DocumentationRequestInterceptor.shared }
4245

4346
//This warning is unnecessary because we control the content
4447
@SuppressLint("SetJavaScriptEnabled")
@@ -83,7 +86,7 @@ class IDETooltipWebviewFragment : Fragment() {
8386
override fun shouldInterceptRequest(
8487
view: WebView,
8588
request: WebResourceRequest,
86-
): WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request)
89+
): WebResourceResponse? = documentation?.intercept(request) ?: super.shouldInterceptRequest(view, request)
8790

8891
override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean {
8992
// Allow loading of local assets files

common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ class HelpActivity : BaseIDEActivity() {
7070

7171
// ADFA-5176: answers documentation requests from the database in-process, so loading a page
7272
// no longer opens a TCP connection per asset to the local web server.
73-
private val documentation = DocumentationRequestInterceptor.shared
73+
//
74+
// by lazy, not an initializer: forcing it here runs during Activity construction, before
75+
// onCreate, on the main thread -- which both stats external storage for the sentinel under a
76+
// StrictMode policy that reports it, and dies before this screen exists if the shared
77+
// interceptor cannot be built. Touched from shouldInterceptRequest instead, on a WebView
78+
// thread, the way FAQActivity does it.
79+
private val documentation by lazy { DocumentationRequestInterceptor.shared }
7480

7581
// Wall-clock start of the page currently loading, for the ADFA-5176 measurement.
7682
// elapsedRealtime, not currentTimeMillis: an NTP correction or a user clock change between
@@ -124,7 +130,10 @@ class HelpActivity : BaseIDEActivity() {
124130
override fun shouldInterceptRequest(
125131
view: android.webkit.WebView,
126132
request: android.webkit.WebResourceRequest,
127-
): android.webkit.WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request)
133+
): android.webkit.WebResourceResponse? {
134+
val intercepted = documentation?.intercept(request)
135+
return intercepted ?: super.shouldInterceptRequest(view, request)
136+
}
128137

129138
override fun onPageStarted(
130139
view: android.webkit.WebView?,
@@ -150,7 +159,7 @@ class HelpActivity : BaseIDEActivity() {
150159
"Loaded '{}' in {} ms; in-process totals so far: {}.",
151160
url,
152161
SystemClock.elapsedRealtime() - pageLoadStartMillis,
153-
documentation.servedSummary(),
162+
documentation?.servedSummary(),
154163
)
155164
pageLoadStartMillis = 0L
156165
}

common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,28 @@ class DocumentationRequestInterceptor(
123123
* a WebView can outlive that, and neither should be able to close the other's handle. The
124124
* cost of the second handle is SQLite's page cache plus the dictionary, a few MB.
125125
*/
126-
val shared: DocumentationRequestInterceptor by lazy {
127-
DocumentationRequestInterceptor(
128-
DocumentationContentSource(
129-
Environment.DOC_DB,
130-
File(getExternalStorageDirectory(), "Download/documentation.db"),
131-
),
132-
)
126+
val shared: DocumentationRequestInterceptor? by lazy {
127+
// Null when Environment.init() has not run, which is a real state, not a defensive
128+
// nicety: DeviceProtectedApplicationLoader wraps that call in runCatching, and the
129+
// credential-protected loader returns before it when storage is not ready. DOC_DB is a
130+
// plain static File with no initializer, so it is null in both cases, and the non-null
131+
// parameter below turns that into an NPE at the first touch of this property. Declining
132+
// instead puts the request on the local web server, which is exactly what a null return
133+
// from intercept() already means everywhere else.
134+
val database = Environment.DOC_DB
135+
if (database == null) {
136+
LoggerFactory
137+
.getLogger(DocumentationRequestInterceptor::class.java)
138+
.warn("Environment.DOC_DB is not set; documentation requests stay on the web server.")
139+
null
140+
} else {
141+
DocumentationRequestInterceptor(
142+
DocumentationContentSource(
143+
database,
144+
File(getExternalStorageDirectory(), "Download/documentation.db"),
145+
),
146+
)
147+
}
133148
}
134149
}
135150
}

0 commit comments

Comments
 (0)