From 34bd629f3cf69f3a29124289eb1f5e77fc0638e4 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 13:19:21 -0700 Subject: [PATCH 01/10] style: spotless reformat, no functional change Enroll SwipeRevealLayout.kt in the file-level Spotless ratchet ahead of the ADFA-5487 functional change, so the whole-file reindent to tabs is not reviewer noise in a behavioral commit. ktlint changes only: import ordering, parameter list wrapping, and `return x` to expression-body conversions. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../itsaky/androidide/ui/SwipeRevealLayout.kt | 802 ++++++++++-------- 1 file changed, 428 insertions(+), 374 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt index c3888d672c..5790a55dce 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt @@ -26,388 +26,442 @@ import android.view.ViewGroup import androidx.annotation.CallSuper import androidx.annotation.FloatRange import androidx.annotation.IdRes +import androidx.core.content.withStyledAttributes import androidx.customview.widget.ViewDragHelper import com.google.android.material.shape.MaterialShapeDrawable import com.itsaky.androidide.R import kotlin.math.max import kotlin.math.min -import androidx.core.content.withStyledAttributes /** * A layout which can be dragged vertically to reveal a hidden content. * * @author Akash Yadav */ -open class SwipeRevealLayout @JvmOverloads constructor( - context: Context, - attrs: AttributeSet? = null, - defStyleAttr: Int = 0, - defStyleRes: Int = 0, -) : ViewGroup(context, attrs, defStyleAttr, defStyleRes) { - - /** - * Interface for listening to drag events. - */ - interface OnDragListener { - - /** - * Called when the drag state changes. - */ - fun onDragStateChanged(swipeRevealLayout: SwipeRevealLayout, state: Int) - - /** - * Called when the drag progress changes. - */ - fun onDragProgress(swipeRevealLayout: SwipeRevealLayout, progress: Float) - } - - private val leftDragHelper: ViewDragHelper - private val rightDragHelper: ViewDragHelper - - private var leftDragProgress = 0f - private var rightDragProgress = 0f - private var isVerticalDragEnabled = true - - private var isDownInDragHandle = false - /** - * Whether the most recent touch-down landed within the configured drag handle. The vertical - * drag-to-reveal gesture is only captured when this is `true`, so that scroll gestures starting - * in the middle of the overlapping content (e.g. the editor) are not stolen. - */ - private val dragHandleLocation = IntArray(2) - - init { - leftDragHelper = ViewDragHelper.create(this, 1f, LeftDragCallback()) - rightDragHelper = ViewDragHelper.create(this, 1f, RightDragCallback()) - } - - private val dragHelperCallback = object : ViewDragHelper.Callback() { - override fun tryCaptureView(child: View, pointerId: Int): Boolean { - return isVerticalDragEnabled && isDownInDragHandle && child === overlappingContent - } - - override fun onViewPositionChanged(changedView: View, left: Int, top: Int, dx: Int, dy: Int) { - draggingViewTop = top - onDragProgress(min(1f, top.toFloat() / dragHeightMax.toFloat())) - } - - override fun getViewVerticalDragRange(child: View): Int { - return if (isVerticalDragEnabled) dragHeightMax else 0 - } - - override fun getOrderedChildIndex(index: Int): Int { - return OVERLAPPING_CONTENT_INDEX - } - - override fun clampViewPositionVertical(child: View, top: Int, dy: Int): Int { - return min(max(top, paddingTop), dragHeightMax) - } - - override fun onViewDragStateChanged(state: Int) { - if (state == draggingState) { - return - } - - if (isDragging && state == ViewDragHelper.STATE_IDLE) { - isOpen = draggingViewTop >= dragHeightMax - } - - onDragStateChanged(state) - } - - override fun onViewReleased(releasedChild: View, xvel: Float, yvel: Float) { - if (draggingViewTop == 0) { - isOpen = false - return - } - - if (draggingViewTop >= dragHeightMax) { - isOpen = true - return - } - - // whether the view should settle to open or close - val settleDestY = if (yvel > AUTO_OPEN_VELOCITY_LIM || draggingViewTop > dragHeightMax / 2) { - dragHeightMax - } else { - paddingTop - } - - if (dragHelper.settleCapturedViewAt(0, settleDestY)) { - this@SwipeRevealLayout.postInvalidateOnAnimation() - } - } - } - - private val hiddenContent: View - get() = getChildAt(HIDDEN_CONTENT_INDEX)!! - - private val overlappingContent: View - get() = getChildAt(OVERLAPPING_CONTENT_INDEX)!! - - private var draggingState = -1 - private var draggingViewTop = 0 - private val dragHeightMax - get() = hiddenContent.height - - private lateinit var dragHelper: ViewDragHelper - - /** - * Whether the view is currently in 'dragging' state. - */ - val isDragging: Boolean - get() = draggingState == ViewDragHelper.STATE_DRAGGING || - draggingState == ViewDragHelper.STATE_SETTLING - - /** - * The ID of the view which will be dragged to reveal the content. - */ - @IdRes - var dragHandleViewId = 0 - - /** - * The current drag progress. - */ - @FloatRange(from = 0.0, to = 1.0) - var dragProgress = 0.0f - private set - - /** - * Listener for drag events. - */ - var dragListener: OnDragListener? = null - - /** - * Whether the view is open. - */ - var isOpen = false - protected set - - companion object { - - private const val HIDDEN_CONTENT_INDEX = 0 - private const val OVERLAPPING_CONTENT_INDEX = 1 - - @Suppress("UNUSED") - const val STATE_IDLE = ViewDragHelper.STATE_IDLE - - @Suppress("UNUSED") - const val STATE_DRAGGING = ViewDragHelper.STATE_DRAGGING - - @Suppress("UNUSED") - const val STATE_SETTLING = ViewDragHelper.STATE_SETTLING - - const val AUTO_OPEN_VELOCITY_LIM = 800.0 - } - - init { - if (attrs != null) { - context.withStyledAttributes( - attrs, R.styleable.SwipeRevealLayout, - defStyleAttr, defStyleRes - ) { - dragHandleViewId = getResourceId( - R.styleable.SwipeRevealLayout_dragHandle, - dragHandleViewId - ) - } - } - } - - override fun onFinishInflate() { - super.onFinishInflate() - this.dragHelper = ViewDragHelper.create(this, dragHelperCallback) - this.isOpen = false - - check(childCount == 2) { - "SwipeRevealLayout must have exactly two children; the hidden content and the overlapping content" - } - } - - override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { - measureChildren(widthMeasureSpec, heightMeasureSpec) - - val maxWidth = MeasureSpec.getSize(widthMeasureSpec) - val maxHeight = MeasureSpec.getSize(heightMeasureSpec) - - setMeasuredDimension(resolveSizeAndState(maxWidth, widthMeasureSpec, 0), - resolveSizeAndState(maxHeight, heightMeasureSpec, 0)) - } - - override fun onLayout(changed: Boolean, l: Int, t: Int, r: Int, b: Int) { - hiddenContent.layout(0, paddingTop, r, paddingTop + hiddenContent.measuredHeight) - - val olapTop = paddingTop + (hiddenContent.height * dragProgress).toInt() - // Ensure overlappingContent extends to bottom to fill available space - overlappingContent.layout(0, olapTop, r, b) - } - - override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { - val action = ev.actionMasked - if (action == MotionEvent.ACTION_CANCEL || action == MotionEvent.ACTION_UP) { - leftDragHelper.cancel() - rightDragHelper.cancel() - dragHelper.cancel() - return false - } - if (action == MotionEvent.ACTION_DOWN) { - isDownInDragHandle = isTouchInDragHandle(ev) - } - val isLeft = leftDragHelper.shouldInterceptTouchEvent(ev) - val isRight = rightDragHelper.shouldInterceptTouchEvent(ev) - val isVertical = dragHelper.shouldInterceptTouchEvent(ev) - return isLeft || isRight || isVertical - } - - @SuppressLint("ClickableViewAccessibility") - override fun onTouchEvent(event: MotionEvent): Boolean { - leftDragHelper.processTouchEvent(event) - rightDragHelper.processTouchEvent(event) - dragHelper.processTouchEvent(event) - return true - } - - /** - * Returns whether the given touch event falls within the bounds of the configured drag handle - * (see [dragHandleViewId] / the `app:dragHandle` attribute). When no drag handle is configured, - * the whole overlapping content acts as the handle (legacy behavior). - */ - private fun isTouchInDragHandle(ev: MotionEvent): Boolean { - if (dragHandleViewId == 0) { - return true - } - - val handle = findViewById(dragHandleViewId) - if (handle == null || handle.visibility != VISIBLE) { - return false - } - - handle.getLocationOnScreen(dragHandleLocation) - val left = dragHandleLocation[0] - val top = dragHandleLocation[1] - val right = left + handle.width - val bottom = top + handle.height - val x = ev.rawX - val y = ev.rawY - return x >= left && x <= right && y >= top && y <= bottom - } - - override fun computeScroll() { - if (leftDragHelper.continueSettling(true) or rightDragHelper.continueSettling(true) or dragHelper.continueSettling(true)) { - postInvalidateOnAnimation() - } - } - - /** - * Internal callback. Invoked when the drag state changes. - */ - @CallSuper - protected open fun onDragStateChanged(state: Int) { - draggingState = state - dragListener?.onDragStateChanged(this, state) - } - - /** - * Internal callback. Invoked when the drag progress changes. - */ - @CallSuper - protected open fun onDragProgress(progress: Float) { - if (dragProgress == progress) { - return - } - - dragProgress = progress - applyDragProgress(progress) - dragListener?.onDragProgress(this, progress) - } - - /** - * Applies the drag progress to the content. - */ - protected open fun applyDragProgress(progress: Float) { - val min = 0.97f - val max = 1f - val scale = min + (max - min) * (1 - progress) - overlappingContent.scaleX = scale - overlappingContent.scaleY = scale - (overlappingContent.background as? MaterialShapeDrawable?)?.interpolation = progress - } - - /** - * Toggles the state of the view. - */ - fun toggleState(isOpen: Boolean) { - if (isOpen) { - open() - } else { - close() - } - } - - /** - * Opens the view. - */ - fun open() { - if (isOpen) { - return - } - smoothSlideTo(1f) - } - - /** - * Closes the view. - */ - fun close() { - if (!isOpen) { - return - } - - smoothSlideTo(0f) - } - - fun setVerticalDragEnabled(enabled: Boolean) { - this.isVerticalDragEnabled = enabled - if (!enabled && isOpen) { - close() - } - } - - private fun smoothSlideTo(offset: Float) { - val y = paddingTop + offset * dragHeightMax - if (dragHelper.smoothSlideViewTo(overlappingContent, overlappingContent.left, y.toInt())) { - postInvalidateOnAnimation() - } - } - - private inner class LeftDragCallback : ViewDragHelper.Callback() { - override fun tryCaptureView(child: View, pointerId: Int): Boolean { - return child.id == R.id.drawer_sidebar // Your left drawer ID - } - - override fun onViewPositionChanged(changedView: View, left: Int, top: Int, dx: Int, dy: Int) { - leftDragProgress = left.toFloat() / changedView.width - dragListener?.onDragProgress(this@SwipeRevealLayout, leftDragProgress) - invalidate() - } - - override fun clampViewPositionHorizontal(child: View, left: Int, dx: Int): Int { - return max(0, min(left, width - child.width)) - } - } - - private inner class RightDragCallback : ViewDragHelper.Callback() { - override fun tryCaptureView(child: View, pointerId: Int): Boolean { - return true//child.id == R.id.right_drawer_sidebar - } - - override fun onViewPositionChanged(changedView: View, left: Int, top: Int, dx: Int, dy: Int) { - rightDragProgress = (width - left).toFloat() / changedView.width - dragListener?.onDragProgress(this@SwipeRevealLayout, rightDragProgress) - invalidate() - } - - override fun clampViewPositionHorizontal(child: View, left: Int, dx: Int): Int { - return max(width - child.width, min(left, width)) - } - } -} \ No newline at end of file +open class SwipeRevealLayout + @JvmOverloads + constructor( + context: Context, + attrs: AttributeSet? = null, + defStyleAttr: Int = 0, + defStyleRes: Int = 0, + ) : ViewGroup(context, attrs, defStyleAttr, defStyleRes) { + /** + * Interface for listening to drag events. + */ + interface OnDragListener { + /** + * Called when the drag state changes. + */ + fun onDragStateChanged( + swipeRevealLayout: SwipeRevealLayout, + state: Int, + ) + + /** + * Called when the drag progress changes. + */ + fun onDragProgress( + swipeRevealLayout: SwipeRevealLayout, + progress: Float, + ) + } + + private val leftDragHelper: ViewDragHelper + private val rightDragHelper: ViewDragHelper + + private var leftDragProgress = 0f + private var rightDragProgress = 0f + private var isVerticalDragEnabled = true + + private var isDownInDragHandle = false + + /** + * Whether the most recent touch-down landed within the configured drag handle. The vertical + * drag-to-reveal gesture is only captured when this is `true`, so that scroll gestures starting + * in the middle of the overlapping content (e.g. the editor) are not stolen. + */ + private val dragHandleLocation = IntArray(2) + + init { + leftDragHelper = ViewDragHelper.create(this, 1f, LeftDragCallback()) + rightDragHelper = ViewDragHelper.create(this, 1f, RightDragCallback()) + } + + private val dragHelperCallback = + object : ViewDragHelper.Callback() { + override fun tryCaptureView( + child: View, + pointerId: Int, + ): Boolean = isVerticalDragEnabled && isDownInDragHandle && child === overlappingContent + + override fun onViewPositionChanged( + changedView: View, + left: Int, + top: Int, + dx: Int, + dy: Int, + ) { + draggingViewTop = top + onDragProgress(min(1f, top.toFloat() / dragHeightMax.toFloat())) + } + + override fun getViewVerticalDragRange(child: View): Int = if (isVerticalDragEnabled) dragHeightMax else 0 + + override fun getOrderedChildIndex(index: Int): Int = OVERLAPPING_CONTENT_INDEX + + override fun clampViewPositionVertical( + child: View, + top: Int, + dy: Int, + ): Int = min(max(top, paddingTop), dragHeightMax) + + override fun onViewDragStateChanged(state: Int) { + if (state == draggingState) { + return + } + + if (isDragging && state == ViewDragHelper.STATE_IDLE) { + isOpen = draggingViewTop >= dragHeightMax + } + + onDragStateChanged(state) + } + + override fun onViewReleased( + releasedChild: View, + xvel: Float, + yvel: Float, + ) { + if (draggingViewTop == 0) { + isOpen = false + return + } + + if (draggingViewTop >= dragHeightMax) { + isOpen = true + return + } + + // whether the view should settle to open or close + val settleDestY = + if (yvel > AUTO_OPEN_VELOCITY_LIM || draggingViewTop > dragHeightMax / 2) { + dragHeightMax + } else { + paddingTop + } + + if (dragHelper.settleCapturedViewAt(0, settleDestY)) { + this@SwipeRevealLayout.postInvalidateOnAnimation() + } + } + } + + private val hiddenContent: View + get() = getChildAt(HIDDEN_CONTENT_INDEX)!! + + private val overlappingContent: View + get() = getChildAt(OVERLAPPING_CONTENT_INDEX)!! + + private var draggingState = -1 + private var draggingViewTop = 0 + private val dragHeightMax + get() = hiddenContent.height + + private lateinit var dragHelper: ViewDragHelper + + /** + * Whether the view is currently in 'dragging' state. + */ + val isDragging: Boolean + get() = + draggingState == ViewDragHelper.STATE_DRAGGING || + draggingState == ViewDragHelper.STATE_SETTLING + + /** + * The ID of the view which will be dragged to reveal the content. + */ + @IdRes + var dragHandleViewId = 0 + + /** + * The current drag progress. + */ + @FloatRange(from = 0.0, to = 1.0) + var dragProgress = 0.0f + private set + + /** + * Listener for drag events. + */ + var dragListener: OnDragListener? = null + + /** + * Whether the view is open. + */ + var isOpen = false + protected set + + companion object { + private const val HIDDEN_CONTENT_INDEX = 0 + private const val OVERLAPPING_CONTENT_INDEX = 1 + + @Suppress("UNUSED") + const val STATE_IDLE = ViewDragHelper.STATE_IDLE + + @Suppress("UNUSED") + const val STATE_DRAGGING = ViewDragHelper.STATE_DRAGGING + + @Suppress("UNUSED") + const val STATE_SETTLING = ViewDragHelper.STATE_SETTLING + + const val AUTO_OPEN_VELOCITY_LIM = 800.0 + } + + init { + if (attrs != null) { + context.withStyledAttributes( + attrs, + R.styleable.SwipeRevealLayout, + defStyleAttr, + defStyleRes, + ) { + dragHandleViewId = + getResourceId( + R.styleable.SwipeRevealLayout_dragHandle, + dragHandleViewId, + ) + } + } + } + + override fun onFinishInflate() { + super.onFinishInflate() + this.dragHelper = ViewDragHelper.create(this, dragHelperCallback) + this.isOpen = false + + check(childCount == 2) { + "SwipeRevealLayout must have exactly two children; the hidden content and the overlapping content" + } + } + + override fun onMeasure( + widthMeasureSpec: Int, + heightMeasureSpec: Int, + ) { + measureChildren(widthMeasureSpec, heightMeasureSpec) + + val maxWidth = MeasureSpec.getSize(widthMeasureSpec) + val maxHeight = MeasureSpec.getSize(heightMeasureSpec) + + setMeasuredDimension( + resolveSizeAndState(maxWidth, widthMeasureSpec, 0), + resolveSizeAndState(maxHeight, heightMeasureSpec, 0), + ) + } + + override fun onLayout( + changed: Boolean, + l: Int, + t: Int, + r: Int, + b: Int, + ) { + hiddenContent.layout(0, paddingTop, r, paddingTop + hiddenContent.measuredHeight) + + val olapTop = paddingTop + (hiddenContent.height * dragProgress).toInt() + // Ensure overlappingContent extends to bottom to fill available space + overlappingContent.layout(0, olapTop, r, b) + } + + override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { + val action = ev.actionMasked + if (action == MotionEvent.ACTION_CANCEL || action == MotionEvent.ACTION_UP) { + leftDragHelper.cancel() + rightDragHelper.cancel() + dragHelper.cancel() + return false + } + if (action == MotionEvent.ACTION_DOWN) { + isDownInDragHandle = isTouchInDragHandle(ev) + } + val isLeft = leftDragHelper.shouldInterceptTouchEvent(ev) + val isRight = rightDragHelper.shouldInterceptTouchEvent(ev) + val isVertical = dragHelper.shouldInterceptTouchEvent(ev) + return isLeft || isRight || isVertical + } + + @SuppressLint("ClickableViewAccessibility") + override fun onTouchEvent(event: MotionEvent): Boolean { + leftDragHelper.processTouchEvent(event) + rightDragHelper.processTouchEvent(event) + dragHelper.processTouchEvent(event) + return true + } + + /** + * Returns whether the given touch event falls within the bounds of the configured drag handle + * (see [dragHandleViewId] / the `app:dragHandle` attribute). When no drag handle is configured, + * the whole overlapping content acts as the handle (legacy behavior). + */ + private fun isTouchInDragHandle(ev: MotionEvent): Boolean { + if (dragHandleViewId == 0) { + return true + } + + val handle = findViewById(dragHandleViewId) + if (handle == null || handle.visibility != VISIBLE) { + return false + } + + handle.getLocationOnScreen(dragHandleLocation) + val left = dragHandleLocation[0] + val top = dragHandleLocation[1] + val right = left + handle.width + val bottom = top + handle.height + val x = ev.rawX + val y = ev.rawY + return x >= left && x <= right && y >= top && y <= bottom + } + + override fun computeScroll() { + if (leftDragHelper.continueSettling(true) or rightDragHelper.continueSettling(true) or dragHelper.continueSettling(true)) { + postInvalidateOnAnimation() + } + } + + /** + * Internal callback. Invoked when the drag state changes. + */ + @CallSuper + protected open fun onDragStateChanged(state: Int) { + draggingState = state + dragListener?.onDragStateChanged(this, state) + } + + /** + * Internal callback. Invoked when the drag progress changes. + */ + @CallSuper + protected open fun onDragProgress(progress: Float) { + if (dragProgress == progress) { + return + } + + dragProgress = progress + applyDragProgress(progress) + dragListener?.onDragProgress(this, progress) + } + + /** + * Applies the drag progress to the content. + */ + protected open fun applyDragProgress(progress: Float) { + val min = 0.97f + val max = 1f + val scale = min + (max - min) * (1 - progress) + overlappingContent.scaleX = scale + overlappingContent.scaleY = scale + (overlappingContent.background as? MaterialShapeDrawable?)?.interpolation = progress + } + + /** + * Toggles the state of the view. + */ + fun toggleState(isOpen: Boolean) { + if (isOpen) { + open() + } else { + close() + } + } + + /** + * Opens the view. + */ + fun open() { + if (isOpen) { + return + } + smoothSlideTo(1f) + } + + /** + * Closes the view. + */ + fun close() { + if (!isOpen) { + return + } + + smoothSlideTo(0f) + } + + fun setVerticalDragEnabled(enabled: Boolean) { + this.isVerticalDragEnabled = enabled + if (!enabled && isOpen) { + close() + } + } + + private fun smoothSlideTo(offset: Float) { + val y = paddingTop + offset * dragHeightMax + if (dragHelper.smoothSlideViewTo(overlappingContent, overlappingContent.left, y.toInt())) { + postInvalidateOnAnimation() + } + } + + private inner class LeftDragCallback : ViewDragHelper.Callback() { + override fun tryCaptureView( + child: View, + pointerId: Int, + ): Boolean { + return child.id == R.id.drawer_sidebar // Your left drawer ID + } + + override fun onViewPositionChanged( + changedView: View, + left: Int, + top: Int, + dx: Int, + dy: Int, + ) { + leftDragProgress = left.toFloat() / changedView.width + dragListener?.onDragProgress(this@SwipeRevealLayout, leftDragProgress) + invalidate() + } + + override fun clampViewPositionHorizontal( + child: View, + left: Int, + dx: Int, + ): Int = max(0, min(left, width - child.width)) + } + + private inner class RightDragCallback : ViewDragHelper.Callback() { + override fun tryCaptureView( + child: View, + pointerId: Int, + ): Boolean { + return true // child.id == R.id.right_drawer_sidebar + } + + override fun onViewPositionChanged( + changedView: View, + left: Int, + top: Int, + dx: Int, + dy: Int, + ) { + rightDragProgress = (width - left).toFloat() / changedView.width + dragListener?.onDragProgress(this@SwipeRevealLayout, rightDragProgress) + invalidate() + } + + override fun clampViewPositionHorizontal( + child: View, + left: Int, + dx: Int, + ): Int = max(width - child.width, min(left, width)) + } + } From 20b0c4624e90d148389f54a0f875555a9817600a Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 13:23:48 -0700 Subject: [PATCH 02/10] fix: stop SwipeRevealLayout's right drag helper capturing every child RightDragCallback.tryCaptureView returned an unconditional `true`, with the intended check commented out as `// child.id == R.id.right_drawer_sidebar` -- an id that exists nowhere in the project. There is no right drawer in activity_editor.xml, so the helper had no legitimate target but captured whichever child sat under a horizontal drag and offset it sideways. Two consequences, both fixed by never capturing: - onViewPositionChanged pushed that horizontal travel straight to dragListener.onDragProgress, bypassing the layout's own onDragProgress. BaseEditorActivity.onSwipeRevealDragProgress then animated the content card's corner interpolation and top padding as if the vertical reveal were being dragged. - onInterceptTouchEvent returns `isLeft || isRight || isVertical`, so the layout stole horizontal gestures from its children. A horizontally scrolling child raced this helper across the same ViewConfiguration touch slop, making the outcome nondeterministic. ADFA-5487 puts a ViewPager2 carousel in exactly that position, which is how this surfaced. No edge tracking is configured, so with capture refused the helper is inert. The callback is left in place as the attachment point for a right drawer, should one ever be added. Verified: :app:compileV8DebugKotlin. ADFA-5487 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../com/itsaky/androidide/ui/SwipeRevealLayout.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt index 5790a55dce..9e3066d0a6 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt @@ -439,12 +439,17 @@ open class SwipeRevealLayout } private inner class RightDragCallback : ViewDragHelper.Callback() { + // There is no right drawer in this layout: R.id.right_drawer_sidebar does not exist, so + // the intended check was commented out and this returned true for every child. That made + // the helper capture whichever child sat under a horizontal drag and offset it sideways, + // and its onViewPositionChanged reported that horizontal travel to dragListener as if it + // were vertical reveal progress. It also stole horizontal gestures from child views, so a + // horizontally scrolling child (ADFA-5487's metrics carousel) raced this helper for them. + // Capture nothing until a right drawer actually exists to name here. override fun tryCaptureView( child: View, pointerId: Int, - ): Boolean { - return true // child.id == R.id.right_drawer_sidebar - } + ): Boolean = false override fun onViewPositionChanged( changedView: View, From 9cd2750467ac353b17a4a3f3bd23e6ca2fd884ca Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 13:35:08 -0700 Subject: [PATCH 03/10] refactor: extract MemoryUsageChartRenderer, render from watcher history BaseEditorActivity drove the memory chart by reaching into binding.memUsageView.chart from six sites and mutating entry.y against a pidToDatasetIdxMap that only resetMemUsageChart() populated. That works only while exactly one chart view exists for the activity's lifetime. ADFA-5487 makes the chart one page of a carousel, where the view can be unbound, recycled, or created long after watching began. MemoryUsageChartRenderer owns the chart wiring instead and holds no sample state: MemoryUsageWatcher already keeps each process's usageHistory ring buffer, so the renderer can rebuild a complete chart from getMemoryUsages() at any time. attach/detach are independent of the data. Two behaviour changes, both deliberate: - attach() renders the full existing history. resetMemUsageChart() used to seed every entry with 0f and wait a tick for real values, which a carousel page bound mid-session would show as a flat line. - onUsagesChanged() rebuilds when the incoming processes no longer match the chart's datasets, instead of logging "No dataset found for process" and dropping that process's samples. This was already reachable without a carousel: ProjectHandlerActivity watches the Gradle Tooling process and then calls resetMemUsageChart(), so any sample arriving between those two lines was discarded. The once-a-second path still mutates the existing Entry objects in place and allocates nothing; the rebuild is the exception, not the rule. The renderer relies on ChartData.getDataSetByIndex returning null for an out-of-range index, which the shipped AndroidChart 3.1.0.21 bytecode confirms (null for index < 0 or >= size) -- the same guard the previous code depended on. Sites swept: all six chart call sites in BaseEditorActivity, both resetMemUsageChart() callers in ProjectHandlerActivity (unchanged, the method keeps its signature), and the now-dead pidToDatasetIdxMap/editorSurfaceContainerBackground members and their imports. No other module referenced either. Tests: 5 new Robolectric tests in MemoryUsageChartRendererTest. Verified they fail without the fix -- reverting the two behaviour changes fails "attach renders the complete existing history", "attach after detach renders the history into the new chart" (all-zero entries) and "onUsagesChanged rebuilds when a process starts being watched" (dataSetCount stays 1), each for the reason it is named for. The in-place-update test passes either way by design, since that path is unchanged. Verified: :app:compileV8DebugKotlin, :app:testV8DebugUnitTest (MemoryUsageChartRendererTest, 5/5). No UI change, so no font-scale check yet; that lands with the carousel. ADFA-5487 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../activities/editor/BaseEditorActivity.kt | 132 ++-------- .../androidide/ui/MemoryUsageChartRenderer.kt | 231 ++++++++++++++++++ .../ui/MemoryUsageChartRendererTest.kt | 157 ++++++++++++ 3 files changed, 403 insertions(+), 117 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt create mode 100644 app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index d1957709da..4da4e552bc 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -52,7 +52,6 @@ import androidx.annotation.GravityInt import androidx.annotation.RequiresApi import androidx.annotation.UiThread import androidx.appcompat.app.ActionBarDrawerToggle -import androidx.collection.MutableIntIntMap import androidx.core.content.ContextCompat import androidx.core.content.IntentCompat import androidx.core.graphics.Insets @@ -67,11 +66,6 @@ import androidx.fragment.app.FragmentManager import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle -import com.github.mikephil.charting.components.AxisBase -import com.github.mikephil.charting.data.Entry -import com.github.mikephil.charting.data.LineData -import com.github.mikephil.charting.data.LineDataSet -import com.github.mikephil.charting.formatter.IAxisValueFormatter import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_COLLAPSED import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_HIDDEN @@ -124,6 +118,7 @@ import com.itsaky.androidide.tasks.cancelIfActive import com.itsaky.androidide.tasks.mainThreadHandler import com.itsaky.androidide.ui.CodeEditorView import com.itsaky.androidide.ui.ContentTranslatingDrawerLayout +import com.itsaky.androidide.ui.MemoryUsageChartRenderer import com.itsaky.androidide.ui.SwipeRevealLayout import com.itsaky.androidide.uidesigner.UIDesignerActivity import com.itsaky.androidide.utils.ActionMenuUtils.showPopupWindow @@ -145,7 +140,6 @@ import com.itsaky.androidide.utils.flashMessage import com.itsaky.androidide.utils.getOrStoreInitialPadding import com.itsaky.androidide.utils.isAtLeastR import com.itsaky.androidide.utils.isDeepLinkTargetOfOpenProject -import com.itsaky.androidide.utils.resolveAttr import com.itsaky.androidide.viewmodel.ApkInstallationViewModel import com.itsaky.androidide.viewmodel.AppLogsCoordinator import com.itsaky.androidide.viewmodel.AppLogsViewModel @@ -173,7 +167,6 @@ import rikka.shizuku.Shizuku import java.io.File import kotlin.math.abs import kotlin.math.roundToInt -import kotlin.math.roundToLong /** * Base class for EditorActivity which handles most of the view related things. @@ -192,7 +185,11 @@ abstract class BaseEditorActivity : private var drawerToggle: ActionBarDrawerToggle? = null private var bottomSheetCallback: BottomSheetBehavior.BottomSheetCallback? = null protected val memoryUsageWatcher = MemoryUsageWatcher() - protected val pidToDatasetIdxMap = MutableIntIntMap(initialCapacity = 3) + private val memUsageChartRenderer = + MemoryUsageChartRenderer( + usagesProvider = memoryUsageWatcher::getMemoryUsages, + lineColorFor = ::getMemUsageLineColorFor, + ) private val fileManagerViewModel by viewModels() private var feedbackButtonManager: FeedbackButtonManager? = null @@ -315,45 +312,7 @@ abstract class BaseEditorActivity : private val memoryUsageListener = MemoryUsageWatcher.MemoryUsageListener { memoryUsage -> - var dataChanged = false - memoryUsage.forEachValue { proc -> - _binding?.memUsageView?.chart?.apply { - val dataset = - ( - data.getDataSetByIndex( - pidToDatasetIdxMap.getOrDefault( - proc.pid, - -1, - ), - ) as LineDataSet? - ) - ?: run { - log.error( - "No dataset found for process: {}: {}", - proc.pid, - proc.pname, - ) - return@forEachValue - } - - dataset.entries.mapIndexed { index, entry -> - entry.y = - (proc.usageHistory[index] / (1024.0 * 1024.0)).toFloat() - } - - dataset.label = "%s - %.2fMB".format(proc.pname, dataset.entries.last().y) - dataset.notifyDataSetChanged() - dataChanged = true - } - } - - if (dataChanged) { - _binding?.memUsageView?.chart?.apply { - data.notifyDataChanged() - notifyDataSetChanged() - invalidate() - } - } + memUsageChartRenderer.onUsagesChanged(memoryUsage) } private val shizukuBinderReceivedListener = @@ -363,10 +322,6 @@ abstract class BaseEditorActivity : private var isImeVisible = false private var contentCardRealHeight: Int? = null - private val editorSurfaceContainerBackground by lazy { - resolveAttr(R.attr.colorSurfaceDim) - } - private var isDebuggerStarting = false @UiThread set(value) { field = value @@ -559,6 +514,7 @@ abstract class BaseEditorActivity : fullscreenManager?.destroy() fullscreenManager = null + memUsageChartRenderer.detach() _binding = null if (isDestroying) { @@ -1000,36 +956,12 @@ abstract class BaseEditorActivity : content.editorAppBarLayout.updatePadding(top = topInset) } - memUsageView.chart.updateLayoutParams { - topMargin = (insetsTop * progress).roundToInt() - } + memUsageChartRenderer.setTopMargin((insetsTop * progress).roundToInt()) } } private fun setupMemUsageChart() { - binding.memUsageView.chart.apply { - val colorAccent = resolveAttr(R.attr.colorAccent) - - isDragEnabled = false - description.isEnabled = false - xAxis.axisLineColor = colorAccent - axisRight.axisLineColor = colorAccent - - setPinchZoom(false) - setBackgroundColor(editorSurfaceContainerBackground) - setDrawGridBackground(true) - setScaleEnabled(true) - - axisLeft.isEnabled = false - axisRight.valueFormatter = - object : - IAxisValueFormatter { - override fun getFormattedValue( - value: Float, - axis: AxisBase?, - ): String = "%dMB".format(value.roundToLong()) - } - } + memUsageChartRenderer.attach(binding.memUsageView.chart) } private fun watchMemory() { @@ -1038,46 +970,12 @@ abstract class BaseEditorActivity : resetMemUsageChart() } + /** + * Rebuilds the memory chart for the currently watched processes. Call after starting or stopping + * watching a process. + */ protected fun resetMemUsageChart() { - val processes = memoryUsageWatcher.getMemoryUsages() - val datasets = - Array(processes.size) { index -> - LineDataSet( - List(MemoryUsageWatcher.MAX_USAGE_ENTRIES) { Entry(it.toFloat(), 0f) }, - processes[index].pname, - ) - } - - val bgColor = editorSurfaceContainerBackground - val textColor = resolveAttr(R.attr.colorOnSurface) - - for ((index, proc) in processes.withIndex()) { - val dataset = datasets[index] - dataset.color = getMemUsageLineColorFor(proc) - dataset.setDrawIcons(false) - dataset.setDrawCircles(false) - dataset.setDrawCircleHole(false) - dataset.setDrawValues(false) - dataset.formLineWidth = 1f - dataset.formSize = 15f - dataset.isHighlightEnabled = false - pidToDatasetIdxMap[proc.pid] = index - } - - binding.memUsageView.chart.setBackgroundColor(bgColor) - - binding.memUsageView.chart.apply { - data = LineData(*datasets) - axisRight.textColor = textColor - axisLeft.textColor = textColor - legend.textColor = textColor - - data.setValueTextColor(textColor) - setBackgroundColor(bgColor) - setGridBackgroundColor(bgColor) - notifyDataSetChanged() - invalidate() - } + memUsageChartRenderer.rebuild() } private fun getMemUsageLineColorFor(proc: MemoryUsageWatcher.ProcessMemoryInfo): Int = diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt new file mode 100644 index 0000000000..331088b32c --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt @@ -0,0 +1,231 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.view.ViewGroup +import androidx.annotation.UiThread +import androidx.collection.IntObjectMap +import androidx.collection.MutableIntIntMap +import androidx.core.view.updateLayoutParams +import com.github.mikephil.charting.components.AxisBase +import com.github.mikephil.charting.data.Entry +import com.github.mikephil.charting.data.LineData +import com.github.mikephil.charting.data.LineDataSet +import com.github.mikephil.charting.formatter.IAxisValueFormatter +import com.itsaky.androidide.R +import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.MemoryUsageWatcher.ProcessMemoryInfo +import com.itsaky.androidide.utils.ShiftedLongArray +import com.itsaky.androidide.utils.resolveAttr +import kotlin.math.roundToLong + +/** + * Renders [MemoryUsageWatcher] samples into a [SafeLineChart]. + * + * The chart view is attached and detached independently of the data: [MemoryUsageWatcher] owns the + * per-process [ProcessMemoryInfo.usageHistory] ring buffers, so this renderer holds no sample state + * of its own and can rebuild a complete chart from [usagesProvider] at any time. That is what makes + * the chart safe to host in a recycling container (ADFA-5487's metrics carousel): a chart view that + * is created long after watching began still shows the full history, and one that is recycled away + * loses nothing. + * + * All methods must be called on the UI thread. MPAndroidChart is not thread-safe; see [SafeLineChart]. + * + * @param usagesProvider Supplies the currently watched processes, newest state each call. + * @param lineColorFor Supplies the plot line color for a process. + */ +class MemoryUsageChartRenderer( + private val usagesProvider: () -> Array, + private val lineColorFor: (ProcessMemoryInfo) -> Int, +) { + private var chart: SafeLineChart? = null + + /** + * Maps a watched pid to its dataset index in the attached chart's [LineData]. Empty whenever no + * chart is attached. + */ + private val pidToDatasetIdx = MutableIntIntMap(initialCapacity = 3) + + /** + * Attaches [chart], applies the static chart configuration, and renders the full current + * history. Replaces any previously attached chart. + */ + @UiThread + fun attach(chart: SafeLineChart) { + this.chart = chart + configure(chart) + rebuild() + } + + /** + * Detaches the current chart. Sample history is unaffected; a later [attach] renders it in full. + */ + @UiThread + fun detach() { + chart = null + pidToDatasetIdx.clear() + } + + /** + * Applies a top margin to the attached chart. No-op when no chart is attached. + */ + @UiThread + fun setTopMargin(margin: Int) { + chart?.updateLayoutParams { + topMargin = margin + } + } + + /** + * Rebuilds the chart's datasets from scratch for the currently watched processes, rendering each + * process's complete [ProcessMemoryInfo.usageHistory]. Call when the set of watched processes + * changes; [onUsagesChanged] calls it on its own when it detects such a change. + */ + @UiThread + fun rebuild() { + val chart = this.chart ?: return + val processes = usagesProvider() + + pidToDatasetIdx.clear() + + val datasets = + Array(processes.size) { index -> + val proc = processes[index] + pidToDatasetIdx[proc.pid] = index + + LineDataSet( + List(proc.usageHistory.size) { entryIdx -> + Entry(entryIdx.toFloat(), proc.usageHistory.megabytesAt(entryIdx)) + }, + proc.pname, + ).apply { + color = lineColorFor(proc) + setDrawIcons(false) + setDrawCircles(false) + setDrawCircleHole(false) + setDrawValues(false) + formLineWidth = 1f + formSize = 15f + isHighlightEnabled = false + label = labelFor(proc.pname, entries.lastOrNull()?.y ?: 0f) + } + } + + val bgColor = chart.context.resolveAttr(R.attr.colorSurfaceDim) + val textColor = chart.context.resolveAttr(R.attr.colorOnSurface) + + chart.apply { + data = LineData(*datasets) + axisRight.textColor = textColor + axisLeft.textColor = textColor + legend.textColor = textColor + + data.setValueTextColor(textColor) + setBackgroundColor(bgColor) + setGridBackgroundColor(bgColor) + notifyDataSetChanged() + invalidate() + } + } + + /** + * Renders a fresh set of samples into the attached chart, mutating the existing entries in place. + * + * Falls back to [rebuild] when [memoryUsage] no longer matches the datasets the chart was built + * with -- a process started or stopped being watched, or the chart was attached before this pid + * existed. The in-place path is the common one and allocates nothing, which matters because this + * runs once a second for the lifetime of the editor. + */ + @UiThread + fun onUsagesChanged(memoryUsage: IntObjectMap) { + val chart = this.chart ?: return + + if (memoryUsage.size != pidToDatasetIdx.size) { + rebuild() + return + } + + var dataChanged = false + memoryUsage.forEachValue { proc -> + val datasetIdx = pidToDatasetIdx.getOrDefault(proc.pid, -1) + val dataset = chart.data?.getDataSetByIndex(datasetIdx) as LineDataSet? + if (dataset == null) { + // The chart's datasets no longer describe the watched processes. Rebuild rather than + // dropping this process's samples on the floor, as the previous code did. + rebuild() + return + } + + for (index in dataset.entries.indices) { + dataset.entries[index].y = proc.usageHistory.megabytesAt(index) + } + + dataset.label = labelFor(proc.pname, dataset.entries.lastOrNull()?.y ?: 0f) + dataset.notifyDataSetChanged() + dataChanged = true + } + + if (dataChanged) { + chart.apply { + data.notifyDataChanged() + notifyDataSetChanged() + invalidate() + } + } + } + + /** + * Applies the configuration that does not depend on the samples. Idempotent. + */ + private fun configure(chart: SafeLineChart) { + chart.apply { + val colorAccent = context.resolveAttr(R.attr.colorAccent) + + isDragEnabled = false + description.isEnabled = false + xAxis.axisLineColor = colorAccent + axisRight.axisLineColor = colorAccent + + setPinchZoom(false) + setBackgroundColor(context.resolveAttr(R.attr.colorSurfaceDim)) + setDrawGridBackground(true) + setScaleEnabled(true) + + axisLeft.isEnabled = false + axisRight.valueFormatter = + object : IAxisValueFormatter { + override fun getFormattedValue( + value: Float, + axis: AxisBase?, + ): String = "%dMB".format(value.roundToLong()) + } + } + } + + private fun labelFor( + pname: String, + megabytes: Float, + ): String = "%s - %.2fMB".format(pname, megabytes) +} + +private const val BYTES_PER_MEGABYTE = 1024.0 * 1024.0 + +/** + * The sample at [index] in megabytes. [MemoryUsageWatcher] stores bytes. + */ +private fun ShiftedLongArray.megabytesAt(index: Int): Float = (this[index] / BYTES_PER_MEGABYTE).toFloat() diff --git a/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt new file mode 100644 index 0000000000..a2959fc4d8 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt @@ -0,0 +1,157 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.graphics.Color +import androidx.collection.MutableIntObjectMap +import androidx.test.core.app.ApplicationProvider +import com.github.mikephil.charting.data.LineDataSet +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.MemoryUsageWatcher.ProcessMemoryInfo +import com.itsaky.androidide.utils.MutableShiftedLongArray +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins the properties ADFA-5487's metrics carousel relies on: the renderer holds no sample state, so + * a chart attached at any time shows the complete history, and a change to the watched process set + * is picked up rather than dropped. + */ +@RunWith(RobolectricTestRunner::class) +class MemoryUsageChartRendererTest { + private val context = ApplicationProvider.getApplicationContext() + + private fun chart() = SafeLineChart(context) + + private fun renderer(processes: () -> Array) = + MemoryUsageChartRenderer( + usagesProvider = processes, + lineColorFor = { Color.BLUE }, + ) + + /** A process whose history ramps from [firstMegabytes] by 1MB per sample. */ + private fun proc( + pid: Int, + pname: String, + firstMegabytes: Long, + ) = ProcessMemoryInfo( + pid, + pname, + MutableShiftedLongArray(MemoryUsageWatcher.MAX_USAGE_ENTRIES) { (firstMegabytes + it) * BYTES_PER_MB }, + ) + + private fun datasetFor( + chart: SafeLineChart, + index: Int, + ) = chart.data.getDataSetByIndex(index) as LineDataSet + + @Test + fun `attach renders the complete existing history, not a flat line`() { + val processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + val chart = chart() + + renderer { processes }.attach(chart) + + val dataset = datasetFor(chart, 0) + assertThat(dataset.entryCount).isEqualTo(MemoryUsageWatcher.MAX_USAGE_ENTRIES) + // The old resetMemUsageChart() seeded every entry with 0f and waited a tick for real values; + // a carousel page attached mid-session would have shown that flat line. + assertThat(dataset.entries.map { it.y }).doesNotContain(0f) + assertThat(dataset.entries.first().y).isEqualTo(100f) + assertThat(dataset.entries.last().y).isEqualTo((100 + MemoryUsageWatcher.MAX_USAGE_ENTRIES - 1).toFloat()) + assertThat(dataset.label).isEqualTo("IDE - %.2fMB".format(dataset.entries.last().y)) + } + + @Test + fun `onUsagesChanged updates entries in place without replacing the datasets`() { + val processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + val chart = chart() + val renderer = renderer { processes } + renderer.attach(chart) + + val datasetBefore = datasetFor(chart, 0) + val entryBefore = datasetBefore.entries.first() + + val updated = proc(pid = 1, pname = "IDE", firstMegabytes = 200) + renderer.onUsagesChanged(MutableIntObjectMap().apply { put(1, updated) }) + + // Same dataset and same Entry objects, new values: this path runs once a second for the + // lifetime of the editor, so it must not allocate. + assertThat(datasetFor(chart, 0)).isSameInstanceAs(datasetBefore) + assertThat(datasetBefore.entries.first()).isSameInstanceAs(entryBefore) + assertThat(entryBefore.y).isEqualTo(200f) + } + + @Test + fun `onUsagesChanged rebuilds when a process starts being watched`() { + var processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + val chart = chart() + val renderer = renderer { processes } + renderer.attach(chart) + + assertThat(chart.data.dataSetCount).isEqualTo(1) + + // Gradle Tooling starts up. The old code looked the new pid up in a map that only reset() + // populated, logged "No dataset found for process", and dropped its samples. + val gradle = proc(pid = 2, pname = "Gradle Tooling", firstMegabytes = 300) + processes = arrayOf(processes[0], gradle) + renderer.onUsagesChanged( + MutableIntObjectMap().apply { + put(1, processes[0]) + put(2, gradle) + }, + ) + + assertThat(chart.data.dataSetCount).isEqualTo(2) + assertThat(datasetFor(chart, 1).label).startsWith("Gradle Tooling - ") + assertThat(datasetFor(chart, 1).entries.first().y).isEqualTo(300f) + } + + @Test + fun `onUsagesChanged after detach is a no-op`() { + val processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + val renderer = renderer { processes } + renderer.attach(chart()) + renderer.detach() + + // A recycled carousel page must not keep the renderer writing into a dead view. + renderer.onUsagesChanged( + MutableIntObjectMap().apply { put(1, processes[0]) }, + ) + } + + @Test + fun `attach after detach renders the history into the new chart`() { + val processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + val renderer = renderer { processes } + renderer.attach(chart()) + renderer.detach() + + val rebound = chart() + renderer.attach(rebound) + + assertThat(datasetFor(rebound, 0).entryCount).isEqualTo(MemoryUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(datasetFor(rebound, 0).entries.first().y).isEqualTo(100f) + } + + private companion object { + const val BYTES_PER_MB = 1024L * 1024L + } +} From fc4bc267efe3380f87b9083178c8c4d77031674d Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 14:06:37 -0700 Subject: [PATCH 04/10] feat: make the editor's memory chart a carousel of metric displays The chart at the top of the editor (revealed by dragging the app bar down) is now a ViewPager2 carousel. Page 1 is the memory chart, still the default; page 2 is the Code On The Go brand mark, a placeholder until there is a real second metric. MetricsCarouselAdapter takes its page list as a constructor argument, so the follow-up tickets (a TrafficStats network chart, and plugin- contributed displays) add pages rather than change this class. The chart page attaches MemoryUsageChartRenderer on bind and detaches on recycle; because the renderer rebuilds from MemoryUsageWatcher's history, swiping away and back shows the full 30-sample series rather than a flat line. Layout notes: - layout_mem_usage.xml stays a single view. SwipeRevealLayout asserts childCount == 2 and indexes its children positionally, so the include cannot gain a sibling; the pager and indicator live inside it. - The status-bar inset now applies to the pager rather than the chart, so MemoryUsageChartRenderer.setTopMargin (a shim from the previous commit, when the activity owned the only chart) is gone. It gains detachIfAttached, which a recycling container needs: RecyclerView can bind a replacement view before recycling the one it replaced, and an unconditional detach would then drop the new chart. - editor_mem_usage_view_height goes 200dp -> 248dp. The indicator is new chrome, so the container grows by its 48dp rather than the chart shrinking. This is a visible change beyond the ticket's literal scope; it is here because of the touch-target point below. - TabLayout has no dot mode, so each tab's background is a selector and the sliding indicator is suppressed. The oval needs a sized, centred layer-list item: a tab background is stretched to fill the tab, which ignores a bare shape's and renders an oval as tall as the whole row. The active dot differs in both size and colour because several of this app's themes resolve colorPrimary to a grey indistinguishable from colorOutline (measured on device: #AAAAAA vs #8F9099). A left-to-right swipe cannot page backwards: that gesture opens the navigation drawer, which is documented app behaviour ("To view the file tree and project options, swipe from left to right", shown in the editor's own onboarding text). InterceptableDrawerLayout's findScrollingChild starts at index 1 and so never examines DrawerLayout's content child, which is consistent with that intent. Backward navigation is therefore by tapping the indicator, which makes the dots a primary control rather than decoration -- hence real 48dp touch targets, measured on device at 48x48dp (168x168px at 560dpi), each carrying a "Metric N of 2" content description. androidx.viewpager2 is declared explicitly. It was already on the compile classpath transitively and pinned to the same 1.1.0-beta02 the version catalog names, so this adds no new dependency; it just stops a compile-time use depending on another library's graph. Verified on a Pixel 6 Pro (arm64), v8 debug: - Both pages render; swipe forward and tap-to-navigate both directions. - Returning to page 1 shows the complete history for both watched processes, including a Gradle Tooling process that started while the carousel was open (the rebuild path from the previous commit). - Font scale 1.0 and 2.0: no clipping, no overlap, status bar clear, touch targets unchanged. MPAndroidChart sizes its own text in pixels so the chart labels do not grow with font scale -- pre-existing, and worth a follow-up for low-vision users. - Landscape: renders correctly, nothing clipped. - :app:testV8DebugUnitTest for ui/activities/fragments: 41 tests green. ADFA-5487 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- app/build.gradle.kts | 3 + .../activities/editor/BaseEditorActivity.kt | 33 ++++- .../androidide/ui/MemoryUsageChartRenderer.kt | 11 +- .../androidide/ui/MetricsCarouselAdapter.kt | 132 ++++++++++++++++++ .../res/drawable/metrics_carousel_dot.xml | 39 ++++++ .../main/res/layout/item_metrics_image.xml | 14 ++ .../res/layout/item_metrics_memory_chart.xml | 13 ++ app/src/main/res/layout/layout_mem_usage.xml | 66 +++++---- app/src/main/res/values/dimens.xml | 53 ++++--- resources/src/main/res/values/strings.xml | 5 + 10 files changed, 300 insertions(+), 69 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt create mode 100644 app/src/main/res/drawable/metrics_carousel_dot.xml create mode 100644 app/src/main/res/layout/item_metrics_image.xml create mode 100644 app/src/main/res/layout/item_metrics_memory_chart.xml diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b21c2d872f..be8c074e7a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -329,6 +329,9 @@ dependencies { implementation(libs.androidx.work) implementation(libs.androidx.work.ktx) implementation(libs.google.material) + // Metrics carousel (ADFA-5487). Already on the classpath transitively; declared so the + // compile-time use in MetricsCarouselAdapter does not depend on another library's graph. + implementation(libs.androidx.viewpager2.v110beta02) implementation(libs.google.flexbox) implementation(libs.libsu.core) diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index 4da4e552bc..87446cac7d 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -71,6 +71,7 @@ import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_COLLAPS import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_HIDDEN import com.google.android.material.tabs.TabLayout import com.google.android.material.tabs.TabLayout.Tab +import com.google.android.material.tabs.TabLayoutMediator import com.itsaky.androidide.FeedbackButtonManager import com.itsaky.androidide.R import com.itsaky.androidide.R.string @@ -119,6 +120,8 @@ import com.itsaky.androidide.tasks.mainThreadHandler import com.itsaky.androidide.ui.CodeEditorView import com.itsaky.androidide.ui.ContentTranslatingDrawerLayout import com.itsaky.androidide.ui.MemoryUsageChartRenderer +import com.itsaky.androidide.ui.MetricsCarouselAdapter +import com.itsaky.androidide.ui.MetricsPage import com.itsaky.androidide.ui.SwipeRevealLayout import com.itsaky.androidide.uidesigner.UIDesignerActivity import com.itsaky.androidide.utils.ActionMenuUtils.showPopupWindow @@ -514,6 +517,7 @@ abstract class BaseEditorActivity : fullscreenManager?.destroy() fullscreenManager = null + _binding?.memUsageView?.metricsPager?.adapter = null memUsageChartRenderer.detach() _binding = null @@ -855,7 +859,7 @@ abstract class BaseEditorActivity : ) feedbackButtonManager?.setupDraggableFab() - setupMemUsageChart() + setupMetricsCarousel() watchMemory() observeFileOperations() @@ -956,12 +960,33 @@ abstract class BaseEditorActivity : content.editorAppBarLayout.updatePadding(top = topInset) } - memUsageChartRenderer.setTopMargin((insetsTop * progress).roundToInt()) + memUsageView.metricsPager.updateLayoutParams { + topMargin = (insetsTop * progress).roundToInt() + } } } - private fun setupMemUsageChart() { - memUsageChartRenderer.attach(binding.memUsageView.chart) + private fun setupMetricsCarousel() { + val pages = + listOf( + // The memory chart is the default page (ADFA-5487). The logo is a placeholder second + // page until there is a real second metric; the network-traffic chart replaces it. + MetricsPage.MemoryChart, + MetricsPage.Image( + drawable = R.drawable.cogo_brand_mark, + description = string.metrics_carousel_brand_mark, + ), + ) + + binding.memUsageView.metricsPager.adapter = MetricsCarouselAdapter(pages, memUsageChartRenderer) + + TabLayoutMediator( + binding.memUsageView.metricsIndicator, + binding.memUsageView.metricsPager, + ) { tab, position -> + // Dots carry no label, but they are still focusable, so give them a spoken position. + tab.contentDescription = getString(string.metrics_carousel_page, position + 1, pages.size) + }.attach() } private fun watchMemory() { diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt index 331088b32c..67d8b94ef3 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt @@ -17,11 +17,9 @@ package com.itsaky.androidide.ui -import android.view.ViewGroup import androidx.annotation.UiThread import androidx.collection.IntObjectMap import androidx.collection.MutableIntIntMap -import androidx.core.view.updateLayoutParams import com.github.mikephil.charting.components.AxisBase import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineData @@ -82,12 +80,13 @@ class MemoryUsageChartRenderer( } /** - * Applies a top margin to the attached chart. No-op when no chart is attached. + * Detaches [chart] only if it is the currently attached one. Use from a recycling container, + * where the replacement view can be bound before the view it replaces is recycled. */ @UiThread - fun setTopMargin(margin: Int) { - chart?.updateLayoutParams { - topMargin = margin + fun detachIfAttached(chart: SafeLineChart) { + if (this.chart === chart) { + detach() } } diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt new file mode 100644 index 0000000000..be4c9fb32c --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt @@ -0,0 +1,132 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.widget.ImageView +import androidx.annotation.DrawableRes +import androidx.annotation.StringRes +import androidx.recyclerview.widget.RecyclerView +import com.itsaky.androidide.R + +/** + * A page of the editor's metrics carousel. + */ +sealed interface MetricsPage { + /** The live memory-usage chart, rendered by [MemoryUsageChartRenderer]. */ + data object MemoryChart : MetricsPage + + /** A static image. Placeholder page until real metrics exist to show alongside memory. */ + data class Image( + @DrawableRes val drawable: Int, + @StringRes val description: Int, + ) : MetricsPage +} + +/** + * Backs the editor's horizontally swipeable carousel of metric displays. + * + * [pages] is a constructor argument rather than a hardcoded list so that new displays -- a network + * traffic chart, or pages contributed by plugins -- can be added without touching this class. + * + * The chart page holds no sample state of its own: [chartRenderer] is attached when the page binds + * and detached when it is recycled, and rebuilds the full history from [MemoryUsageChartRenderer]'s + * watcher each time. Swiping away from the chart and back therefore loses nothing. + */ +class MetricsCarouselAdapter( + private val pages: List, + private val chartRenderer: MemoryUsageChartRenderer, +) : RecyclerView.Adapter() { + sealed class PageViewHolder( + view: View, + ) : RecyclerView.ViewHolder(view) { + class MemoryChart( + val chart: SafeLineChart, + ) : PageViewHolder(chart) + + class Image( + val image: ImageView, + ) : PageViewHolder(image) + } + + override fun getItemCount(): Int = pages.size + + override fun getItemViewType(position: Int): Int = + when (pages[position]) { + is MetricsPage.MemoryChart -> VIEW_TYPE_MEMORY_CHART + is MetricsPage.Image -> VIEW_TYPE_IMAGE + } + + override fun onCreateViewHolder( + parent: ViewGroup, + viewType: Int, + ): PageViewHolder { + val inflater = LayoutInflater.from(parent.context) + return when (viewType) { + VIEW_TYPE_MEMORY_CHART -> { + PageViewHolder.MemoryChart( + inflater.inflate(R.layout.item_metrics_memory_chart, parent, false) as SafeLineChart, + ) + } + + VIEW_TYPE_IMAGE -> { + PageViewHolder.Image( + inflater.inflate(R.layout.item_metrics_image, parent, false) as ImageView, + ) + } + + else -> { + throw IllegalArgumentException("Unknown metrics page view type: $viewType") + } + } + } + + override fun onBindViewHolder( + holder: PageViewHolder, + position: Int, + ) { + when (val page = pages[position]) { + is MetricsPage.MemoryChart -> { + chartRenderer.attach((holder as PageViewHolder.MemoryChart).chart) + } + + is MetricsPage.Image -> { + (holder as PageViewHolder.Image).image.apply { + setImageResource(page.drawable) + contentDescription = context.getString(page.description) + } + } + } + } + + override fun onViewRecycled(holder: PageViewHolder) { + if (holder is PageViewHolder.MemoryChart) { + // Only if this holder's chart is still the attached one: a rebind can create the + // replacement before RecyclerView recycles the view it replaced, and detaching then + // would drop the new chart instead of the old. + chartRenderer.detachIfAttached(holder.chart) + } + } + + private companion object { + const val VIEW_TYPE_MEMORY_CHART = 0 + const val VIEW_TYPE_IMAGE = 1 + } +} diff --git a/app/src/main/res/drawable/metrics_carousel_dot.xml b/app/src/main/res/drawable/metrics_carousel_dot.xml new file mode 100644 index 0000000000..602285af95 --- /dev/null +++ b/app/src/main/res/drawable/metrics_carousel_dot.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/src/main/res/layout/item_metrics_image.xml b/app/src/main/res/layout/item_metrics_image.xml new file mode 100644 index 0000000000..4d8617b328 --- /dev/null +++ b/app/src/main/res/layout/item_metrics_image.xml @@ -0,0 +1,14 @@ + + + diff --git a/app/src/main/res/layout/item_metrics_memory_chart.xml b/app/src/main/res/layout/item_metrics_memory_chart.xml new file mode 100644 index 0000000000..d6eaaa40ab --- /dev/null +++ b/app/src/main/res/layout/item_metrics_memory_chart.xml @@ -0,0 +1,13 @@ + + + diff --git a/app/src/main/res/layout/layout_mem_usage.xml b/app/src/main/res/layout/layout_mem_usage.xml index 92888099bc..33bf21bdc3 100644 --- a/app/src/main/res/layout/layout_mem_usage.xml +++ b/app/src/main/res/layout/layout_mem_usage.xml @@ -1,33 +1,39 @@ - + - + + - - + - \ No newline at end of file + + + diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index e39716e1cf..51ea64c7a0 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -1,33 +1,28 @@ - + - 200dp - 28dp + 248dp + 48dp + 9dp + 6dp + 24dp + 16dp + 28dp - 28dp - 8dp - 24dp - 32sp - 16sp - 12sp + 28dp + 8dp + 24dp + 32sp + 16sp + 12sp - - 44dp - 64dp - 6dp - \ No newline at end of file + + 44dp + 64dp + 6dp + diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index ca8fe80b79..0094cecccf 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1673,4 +1673,9 @@ Dock to editor Close + Memory usage chart + Metric %1$d of %2$d + Code On The Go logo + + From f4f678a3d40f0c25c6c64a7aaa58dc9401e4acca Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 14:29:03 -0700 Subject: [PATCH 05/10] feat: let the metrics carousel own horizontal swipes in its own strip The carousel could only page forwards. A left-to-right swipe opened the navigation drawer instead, so going back needed a tap on the indicator dots, which in turn forced them to be 48dp touch targets. Two mechanisms claim that gesture, and each needs its own answer: - View-hierarchy interceptors. MetricsCarouselLayout, the new root of layout_mem_usage.xml, calls requestDisallowInterceptTouchEvent on its ancestors on ACTION_DOWN. That propagates the whole way up, so any ancestor ViewGroup is out of the way for the rest of the gesture, and only for gestures starting inside this strip. - The editor's activity-level GestureDetector, run from dispatchTouchEvent. It never calls onInterceptTouchEvent, so no disallow-intercept can stop it; this was in fact the one opening the drawer, confirmed on device. isTouchOnMetricsCarousel excludes the carousel's bounds the same way isTouchOnBottomSheetTabs already excludes the bottom-sheet tab strip. The exclusion is gated on swipeReveal.dragProgress > 0. The carousel is laid out at the top of the reveal even while the content card covers it, and siblings do not clip each other, so getGlobalVisibleRect reports it visible either way; without the gate the drawer gesture would have gone dead over the top of a closed editor. The vertical reveal drag is unaffected: SwipeRevealLayout only captures a vertical drag whose touch-down landed in its drag handle (the app bar), never in this strip. With swipe working both ways the dots are a status indicator rather than a control, so they no longer need 48dp targets or accessibility nodes of their own -- ViewPager2 already reports page position, and each page carries its own content description. Touches on the indicator are swallowed so the dots cannot act as tabs, while TabLayoutMediator still tracks the selected page. The row drops 48dp -> 20dp and, with the panel kept at 248dp, that space goes to the chart: the plot area grows from 135dp to 187dp. The now-unused metrics_carousel_page string is removed. Verified on a Pixel 6 Pro (arm64), v8 debug: - Paging forward and backward by swipe, portrait and landscape. - Returning to page 1 still shows full history for both watched processes. - Drawer gesture unaffected: still opens from a rightward fling outside the carousel while the reveal is open, and from one over the region the carousel occupies once the reveal is closed. - Font scale 1.0 and 2.0: geometry is dp-only and unchanged (pager and indicator bounds identical at both), nothing clipped, status bar clear. - :app:testV8DebugUnitTest for ui/activities/fragments: 41 tests green. ADFA-5487 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../activities/editor/BaseEditorActivity.kt | 45 ++++++++++++--- .../androidide/ui/MetricsCarouselLayout.kt | 56 +++++++++++++++++++ app/src/main/res/layout/layout_mem_usage.xml | 4 +- app/src/main/res/values/dimens.xml | 4 +- resources/src/main/res/values/strings.xml | 1 - 5 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index 87446cac7d..53a78057ad 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -983,10 +983,17 @@ abstract class BaseEditorActivity : TabLayoutMediator( binding.memUsageView.metricsIndicator, binding.memUsageView.metricsPager, - ) { tab, position -> - // Dots carry no label, but they are still focusable, so give them a spoken position. - tab.contentDescription = getString(string.metrics_carousel_page, position + 1, pages.size) - }.attach() + ) { _, _ -> }.attach() + + binding.memUsageView.metricsIndicator.apply { + // A pure indicator, not a control: the carousel pages by swipe (MetricsCarouselLayout), + // so the dots need neither 48dp touch targets nor their own accessibility nodes -- + // ViewPager2 already reports page position. Swallowing touches keeps the dots from + // acting as tabs while leaving TabLayoutMediator to track the selected page. + @Suppress("ClickableViewAccessibility") + setOnTouchListener { _, _ -> true } + importantForAccessibility = View.IMPORTANT_FOR_ACCESSIBILITY_NO + } } private fun watchMemory() { @@ -1808,8 +1815,12 @@ abstract class BaseEditorActivity : // Filter out diagonal flings so only an intentional right swipe opens the drawer. // A horizontal fling that started on the bottom-sheet tab strip is the user - // scrolling tabs, not asking for the drawer. - if (isDrawerOpenFling && !isTouchOnBottomSheetTabs(e1)) { + // scrolling tabs, not asking for the drawer; one that started on the metrics + // carousel is the user paging it backwards. + if (isDrawerOpenFling && + !isTouchOnBottomSheetTabs(e1) && + !isTouchOnMetricsCarousel(e1) + ) { binding.editorDrawerLayout.openDrawer(GravityCompat.START) return true } @@ -1831,8 +1842,28 @@ abstract class BaseEditorActivity : private fun isTouchOnBottomSheetTabs(ev: MotionEvent): Boolean { val tabs = contentOrNull?.bottomSheet?.binding?.tabs ?: return false + return containsTouch(tabs, ev) + } + + private fun isTouchOnMetricsCarousel(ev: MotionEvent): Boolean { + val binding = _binding ?: return false + + // The carousel is laid out at the top of the reveal even while the content card covers it, + // and siblings do not clip each other, so getGlobalVisibleRect reports it visible either + // way. Without this check the drawer gesture would be dead over the top of a closed editor. + if (binding.swipeReveal.dragProgress <= 0f) { + return false + } + + return containsTouch(binding.memUsageView.root, ev) + } + + private fun containsTouch( + view: View, + ev: MotionEvent, + ): Boolean { val rect = Rect() - if (!tabs.getGlobalVisibleRect(rect)) return false + if (!view.getGlobalVisibleRect(rect)) return false return rect.contains(ev.rawX.toInt(), ev.rawY.toInt()) } diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt new file mode 100644 index 0000000000..79dd92c872 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt @@ -0,0 +1,56 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.content.Context +import android.util.AttributeSet +import android.view.MotionEvent +import androidx.constraintlayout.widget.ConstraintLayout + +/** + * Host for the editor's metrics carousel, which claims horizontal gestures that begin inside it. + * + * The carousel pages with a horizontal swipe, but a left-to-right swipe elsewhere in the editor + * opens the navigation drawer -- documented behaviour, shown in the editor's own onboarding text. + * Without this, the carousel could only page forwards. Asking every ancestor not to intercept, for + * the rest of the gesture, hands horizontal drags that start in this strip to [ViewPager2] and + * leaves the drawer gesture untouched everywhere else. + * + * This covers ancestors that intercept through the view hierarchy. The editor also runs an + * activity-level [android.view.GestureDetector] from `dispatchTouchEvent`, which never calls + * `onInterceptTouchEvent` and so cannot be stopped this way; `BaseEditorActivity` excludes this + * view's bounds there instead, the same way it already excludes the bottom-sheet tab strip. + * + * The vertical reveal drag is unaffected: `SwipeRevealLayout` only captures a vertical drag whose + * touch-down landed in its configured drag handle (the editor app bar), never in this strip. + */ +class MetricsCarouselLayout + @JvmOverloads + constructor( + context: Context, + attrs: AttributeSet? = null, + defStyleAttr: Int = 0, + ) : ConstraintLayout(context, attrs, defStyleAttr) { + override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { + if (ev.actionMasked == MotionEvent.ACTION_DOWN) { + // Cleared by the framework on the next ACTION_DOWN, so this lasts exactly one gesture. + parent?.requestDisallowInterceptTouchEvent(true) + } + return super.onInterceptTouchEvent(ev) + } + } diff --git a/app/src/main/res/layout/layout_mem_usage.xml b/app/src/main/res/layout/layout_mem_usage.xml index 33bf21bdc3..6000902f22 100644 --- a/app/src/main/res/layout/layout_mem_usage.xml +++ b/app/src/main/res/layout/layout_mem_usage.xml @@ -7,7 +7,7 @@ - - + diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index 51ea64c7a0..17e9a78278 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -7,10 +7,10 @@ 248dp - 48dp + 20dp 9dp 6dp - 24dp + 7dp 16dp 28dp diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 0094cecccf..897133acc9 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1674,7 +1674,6 @@ Close Memory usage chart - Metric %1$d of %2$d Code On The Go logo From ddee12e16279aefed897493fdaa3f3fa63c4572d Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 15:02:56 -0700 Subject: [PATCH 06/10] feat: replace the carousel's dot indicator with a page title Dots said which page you were on but not what it was. A metrics carousel is a set of different displays, so naming the current one carries more information in the same space: "Memory usage" rather than two dots. MetricsPage gains a title, so a page names itself and the follow-up tickets (network chart, plugin-contributed pages) supply one as a matter of course. A ViewPager2.OnPageChangeCallback drives the label; it is unregistered alongside the adapter in preDestroy. The callback does not fire for the page the carousel opens on, so the initial title is set explicitly. The title is sp text, unlike the dp-sized dots, so the layout had to change shape: the title is wrap_content and the pager takes whatever height is left. At 2x font scale the title grows from 22dp to 35dp and the chart gives up that space, rather than the label clipping or the panel changing height. No maxLines or ellipsize -- a long title wraps and the chart absorbs it, which is the right failure mode for text that is not disposable. This drops the TabLayout, the dot selector drawable, its four dimens, and the touch-swallowing needed to stop dots acting as tabs. The dots' theme problem goes with them: the active dot needed to differ in both size and colour because several themes resolve colorPrimary to a grey indistinguishable from colorOutline. Trade-off: a title does not show that further pages exist, which dots did. Worth revisiting if the carousel grows past a handful of pages; at two, swiping finds the second one and the title then says what it is. Verified on a Pixel 6 Pro (arm64), v8 debug: - Titles track the page ("Memory usage", "Code On The Go"); paging both directions still works and page 1 still returns with full history. - Font scale 1.0 and 2.0, measured on a cold start: title 22dp -> 35dp, pager 185dp -> 171dp, panel 248dp throughout, nothing clipped. EditorActivityKt declares fontScale in configChanges, so it is not recreated on a font-scale change -- a warm relaunch reports stale geometry and the app must be force-stopped first to measure this. - :app:testV8DebugUnitTest for ui/activities/fragments: 41 tests green. ADFA-5487 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../activities/editor/BaseEditorActivity.kt | 37 +++++++++++------- .../androidide/ui/MetricsCarouselAdapter.kt | 10 ++++- .../res/drawable/metrics_carousel_dot.xml | 39 ------------------- app/src/main/res/layout/layout_mem_usage.xml | 25 ++++++------ app/src/main/res/values/dimens.xml | 6 +-- resources/src/main/res/values/strings.xml | 2 + 6 files changed, 49 insertions(+), 70 deletions(-) delete mode 100644 app/src/main/res/drawable/metrics_carousel_dot.xml diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index 53a78057ad..3f4794af23 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -66,12 +66,12 @@ import androidx.fragment.app.FragmentManager import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle +import androidx.viewpager2.widget.ViewPager2 import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_COLLAPSED import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_HIDDEN import com.google.android.material.tabs.TabLayout import com.google.android.material.tabs.TabLayout.Tab -import com.google.android.material.tabs.TabLayoutMediator import com.itsaky.androidide.FeedbackButtonManager import com.itsaky.androidide.R import com.itsaky.androidide.R.string @@ -188,6 +188,7 @@ abstract class BaseEditorActivity : private var drawerToggle: ActionBarDrawerToggle? = null private var bottomSheetCallback: BottomSheetBehavior.BottomSheetCallback? = null protected val memoryUsageWatcher = MemoryUsageWatcher() + private var metricsPageCallback: ViewPager2.OnPageChangeCallback? = null private val memUsageChartRenderer = MemoryUsageChartRenderer( usagesProvider = memoryUsageWatcher::getMemoryUsages, @@ -517,6 +518,10 @@ abstract class BaseEditorActivity : fullscreenManager?.destroy() fullscreenManager = null + metricsPageCallback?.let { callback -> + _binding?.memUsageView?.metricsPager?.unregisterOnPageChangeCallback(callback) + } + metricsPageCallback = null _binding?.memUsageView?.metricsPager?.adapter = null memUsageChartRenderer.detach() _binding = null @@ -971,29 +976,31 @@ abstract class BaseEditorActivity : listOf( // The memory chart is the default page (ADFA-5487). The logo is a placeholder second // page until there is a real second metric; the network-traffic chart replaces it. - MetricsPage.MemoryChart, + MetricsPage.MemoryChart(title = string.metrics_title_memory), MetricsPage.Image( drawable = R.drawable.cogo_brand_mark, description = string.metrics_carousel_brand_mark, + title = string.metrics_title_brand_mark, ), ) binding.memUsageView.metricsPager.adapter = MetricsCarouselAdapter(pages, memUsageChartRenderer) - TabLayoutMediator( - binding.memUsageView.metricsIndicator, - binding.memUsageView.metricsPager, - ) { _, _ -> }.attach() - - binding.memUsageView.metricsIndicator.apply { - // A pure indicator, not a control: the carousel pages by swipe (MetricsCarouselLayout), - // so the dots need neither 48dp touch targets nor their own accessibility nodes -- - // ViewPager2 already reports page position. Swallowing touches keeps the dots from - // acting as tabs while leaving TabLayoutMediator to track the selected page. - @Suppress("ClickableViewAccessibility") - setOnTouchListener { _, _ -> true } - importantForAccessibility = View.IMPORTANT_FOR_ACCESSIBILITY_NO + val showTitleFor = { position: Int -> + pages.getOrNull(position)?.let { page -> + binding.memUsageView.metricsTitle.setText(page.title) + } } + + metricsPageCallback = + object : ViewPager2.OnPageChangeCallback() { + override fun onPageSelected(position: Int) { + showTitleFor(position) + } + }.also { binding.memUsageView.metricsPager.registerOnPageChangeCallback(it) } + + // onPageSelected does not fire for the page the carousel opens on. + showTitleFor(binding.memUsageView.metricsPager.currentItem) } private fun watchMemory() { diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt index be4c9fb32c..4b602584fc 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt @@ -28,15 +28,23 @@ import com.itsaky.androidide.R /** * A page of the editor's metrics carousel. + * + * @property title Names the page. Shown below the carousel, and the only cue to which page is + * showing, so every page needs one. */ sealed interface MetricsPage { + @get:StringRes val title: Int + /** The live memory-usage chart, rendered by [MemoryUsageChartRenderer]. */ - data object MemoryChart : MetricsPage + data class MemoryChart( + @StringRes override val title: Int, + ) : MetricsPage /** A static image. Placeholder page until real metrics exist to show alongside memory. */ data class Image( @DrawableRes val drawable: Int, @StringRes val description: Int, + @StringRes override val title: Int, ) : MetricsPage } diff --git a/app/src/main/res/drawable/metrics_carousel_dot.xml b/app/src/main/res/drawable/metrics_carousel_dot.xml deleted file mode 100644 index 602285af95..0000000000 --- a/app/src/main/res/drawable/metrics_carousel_dot.xml +++ /dev/null @@ -1,39 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/app/src/main/res/layout/layout_mem_usage.xml b/app/src/main/res/layout/layout_mem_usage.xml index 6000902f22..e78b5f9dc9 100644 --- a/app/src/main/res/layout/layout_mem_usage.xml +++ b/app/src/main/res/layout/layout_mem_usage.xml @@ -13,27 +13,30 @@ android:layout_width="match_parent" android:layout_height="@dimen/editor_mem_usage_view_height"> + - + tools:text="Memory usage" + xmlns:tools="http://schemas.android.com/tools" /> diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index 17e9a78278..c3bbde87e7 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -7,10 +7,8 @@ 248dp - 20dp - 9dp - 6dp - 7dp + 16dp + 4dp 16dp 28dp diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 897133acc9..808f226785 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1674,6 +1674,8 @@ Close Memory usage chart + Memory usage + Code On The Go Code On The Go logo From 5d00a796ab5af584a84cdf0b7a220cceed849bf6 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 08:13:08 -0700 Subject: [PATCH 07/10] fix(metrics): correctness findings from review on the carousel (ADFA-5487) resetMemUsageChart() ran on two background threads. The renderer documents itself as UI-thread-only, and rebuild() clears and repopulates a non-thread-safe pid-to-dataset map that the once-a-second sample listener reads on the main thread. The tooling server's start callback arrives on its own thread and the metadata correction on a CompletableFuture completion thread, so either could interleave with a tick and plot one process's samples on another's line, or throw out of the entry loop. Both now post to the main thread. The process colour lookup could take the editor down, from a timer. It threw IllegalArgumentException for an unrecognised process name, which was survivable while only two explicit call sites reached it -- this PR routes it through the 1 Hz listener and through RecyclerView's bind pass. An unknown name now falls back to grey. It also moves to the companion: a bound reference to an activity method is handed to the renderer, which the adapter holds, and nothing in the function needs an activity. containsTouch compared window coordinates against screen coordinates. getGlobalVisibleRect reports the rect in window space -- ViewRootImpl intersects with the window and never offsets by its position on screen -- while rawX/rawY are screen coordinates. In split-screen or freeform the window origin is not zero, so the drawer gesture was dead over the carousel and live below it. Now uses getLocationOnScreen, the idiom SwipeRevealLayout.isTouchInDragHandle already used in this same file. The drawer fling was excluded over the whole strip even when the carousel could not use it. A left-to-right fling pages the carousel backwards, and the carousel opens on the first page, so on that page the gesture did nothing at all while the documented right-swipe drawer gesture stayed dead. The exclusion now applies only when there is a previous page, and only over the pager rather than the whole strip. The reveal drag relaid out a ViewPager2 every frame. The inset compensation moved from a chart view to the pager, so a margin change now re-measures the pager, its RecyclerView and every attached page on each frame of the drag. A translationY gives the same result for a pure vertical offset with no layout pass. The viewpager2 dependency pointed at 1.1.0-beta02 while the catalog's other alias for the same module is 1.0.0, so Gradle's conflict resolution upgraded the whole app classpath -- including appintro, compiled against 1.0.0 -- to a pre-release nobody chose. Now uses the stable alias. The brand strings duplicated app_name, were translatable, and had drifted to a different capitalisation of the product name. The title now uses app_name; the content description is one string, not translatable. Two claims in the diff were false and are now either true or gone: the in-place update path does not "allocate nothing" -- it reformats a legend label per series per tick -- and the byte-per-megabyte constant was defined twice, once in main and once in the test, so the test verified its own arithmetic rather than the renderer's. Two tests were strengthened. "onUsagesChanged after detach is a no-op" asserted nothing at all and passed with the guard deleted; it now snapshots the chart and asserts it is unchanged. And the branch production actually hits -- same process count, one pid swapped, which is what a tooling-server pid correction produces -- had no coverage, so correctness rested on getDataSetByIndex(-1) happening to return null. Co-Authored-By: Claude Opus 5 --- app/build.gradle.kts | 2 +- .../activities/editor/BaseEditorActivity.kt | 69 ++++++++++++++----- .../editor/ProjectHandlerActivity.kt | 9 ++- .../androidide/ui/MemoryUsageChartRenderer.kt | 8 ++- .../ui/MemoryUsageChartRendererTest.kt | 43 ++++++++++-- resources/src/main/res/values/strings.xml | 5 +- 6 files changed, 107 insertions(+), 29 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index be8c074e7a..d57f254ba4 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -331,7 +331,7 @@ dependencies { implementation(libs.google.material) // Metrics carousel (ADFA-5487). Already on the classpath transitively; declared so the // compile-time use in MetricsCarouselAdapter does not depend on another library's graph. - implementation(libs.androidx.viewpager2.v110beta02) + implementation(libs.androidx.viewpager2) implementation(libs.google.flexbox) implementation(libs.libsu.core) diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index 3f4794af23..5e00e7a010 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -192,7 +192,7 @@ abstract class BaseEditorActivity : private val memUsageChartRenderer = MemoryUsageChartRenderer( usagesProvider = memoryUsageWatcher::getMemoryUsages, - lineColorFor = ::getMemUsageLineColorFor, + lineColorFor = Companion::getMemUsageLineColorFor, ) private val fileManagerViewModel by viewModels() @@ -439,7 +439,26 @@ abstract class BaseEditorActivity : companion object { const val DEBUGGER_SERVICE_STOP_DELAY_MS: Long = 60 * 1000 + /** + * The plot colour for a watched process. + * + * On the companion rather than the activity: a bound reference to an activity method is + * handed to the renderer, which the carousel adapter holds, so any path that misses the + * adapter teardown would keep the whole editor reachable. Nothing here needs an activity. + * + * An unrecognised name falls back rather than throwing. The renderer now reaches this from + * the once-a-second sample listener and from RecyclerView's bind pass, so a name nobody + * added a colour for would take the editor down from a timer callback or mid-layout. + */ @JvmStatic + fun getMemUsageLineColorFor(proc: MemoryUsageWatcher.ProcessMemoryInfo): Int = + when (proc.pname) { + PROC_IDE -> Color.BLUE + PROC_GRADLE_TOOLING -> Color.RED + PROC_GRADLE_DAEMON -> Color.GREEN + else -> Color.GRAY + } + protected val PROC_IDE = "IDE" @JvmStatic @@ -965,9 +984,11 @@ abstract class BaseEditorActivity : content.editorAppBarLayout.updatePadding(top = topInset) } - memUsageView.metricsPager.updateLayoutParams { - topMargin = (insetsTop * progress).roundToInt() - } + // translationY, not a margin: this runs on every frame of the reveal drag, and a + // margin change calls requestLayout, which now re-measures a ViewPager2, its + // RecyclerView and every attached page rather than the single chart view it used to. + // The visual result is identical for a pure vertical offset. + memUsageView.metricsPager.translationY = insetsTop * progress } } @@ -980,7 +1001,8 @@ abstract class BaseEditorActivity : MetricsPage.Image( drawable = R.drawable.cogo_brand_mark, description = string.metrics_carousel_brand_mark, - title = string.metrics_title_brand_mark, + // The product's own name, from the one place it is defined. + title = string.app_name, ), ) @@ -1017,14 +1039,6 @@ abstract class BaseEditorActivity : memUsageChartRenderer.rebuild() } - private fun getMemUsageLineColorFor(proc: MemoryUsageWatcher.ProcessMemoryInfo): Int = - when (proc.pname) { - PROC_IDE -> Color.BLUE - PROC_GRADLE_TOOLING -> Color.RED - PROC_GRADLE_DAEMON -> Color.GREEN - else -> throw IllegalArgumentException("Unknown process: $proc") - } - override fun onPause() { super.onPause() memoryUsageWatcher.listener = null @@ -1855,6 +1869,14 @@ abstract class BaseEditorActivity : private fun isTouchOnMetricsCarousel(ev: MotionEvent): Boolean { val binding = _binding ?: return false + // A left-to-right fling pages the carousel *backwards*, so there is nothing for it to do + // on the first page -- which is the page the carousel opens on. Excluding the strip + // regardless left the documented right-swipe drawer gesture dead over the whole panel + // while doing nothing in its place. + if (binding.memUsageView.metricsPager.currentItem <= 0) { + return false + } + // The carousel is laid out at the top of the reveal even while the content card covers it, // and siblings do not clip each other, so getGlobalVisibleRect reports it visible either // way. Without this check the drawer gesture would be dead over the top of a closed editor. @@ -1862,16 +1884,29 @@ abstract class BaseEditorActivity : return false } - return containsTouch(binding.memUsageView.root, ev) + // The pager, not the whole strip: the title and its row are not something the carousel + // pages from, and MetricsCarouselLayout has already walled that row off from every + // ancestor, so a fling there would otherwise be swallowed twice over. + return containsTouch(binding.memUsageView.metricsPager, ev) } private fun containsTouch( view: View, ev: MotionEvent, ): Boolean { - val rect = Rect() - if (!view.getGlobalVisibleRect(rect)) return false - return rect.contains(ev.rawX.toInt(), ev.rawY.toInt()) + if (!view.isShown) return false + + // getLocationOnScreen, not getGlobalVisibleRect: the latter reports window coordinates -- + // ViewRootImpl intersects with the window and never offsets by its position on screen -- + // while rawX/rawY are screen coordinates. In split-screen or freeform the window origin is + // not zero, so the two disagree and the hit test lands somewhere else entirely. + // SwipeRevealLayout.isTouchInDragHandle already uses this idiom. + val location = IntArray(2) + view.getLocationOnScreen(location) + val x = ev.rawX.toInt() + val y = ev.rawY.toInt() + return x >= location[0] && x < location[0] + view.width && + y >= location[1] && y < location[1] + view.height } private fun showTooltip(tag: String) { diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt index 26ed2966e3..afc6707577 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt @@ -716,7 +716,11 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() { service.startToolingServer { pid -> memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_TOOLING) - resetMemUsageChart() + // The callback arrives on the tooling server's own thread, and the renderer is + // @UiThread: rebuild() clears and repopulates a non-thread-safe pid map that the + // once-a-second sample listener reads on the main thread, so racing it can plot one + // process's samples on another's line or throw out of the entry loop. + runOnUiThread { resetMemUsageChart() } service.metadata().whenComplete { metadata, err -> if (metadata == null || err != null) { @@ -731,7 +735,8 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() { metadata.pid, ) memoryUsageWatcher.watchProcess(metadata.pid, PROC_GRADLE_TOOLING) - resetMemUsageChart() + // A CompletableFuture completion thread, for the same reason as above. + runOnUiThread { resetMemUsageChart() } } } diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt index 67d8b94ef3..8d348944a7 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryUsageChartRenderer.kt @@ -147,8 +147,10 @@ class MemoryUsageChartRenderer( * * Falls back to [rebuild] when [memoryUsage] no longer matches the datasets the chart was built * with -- a process started or stopped being watched, or the chart was attached before this pid - * existed. The in-place path is the common one and allocates nothing, which matters because this - * runs once a second for the lifetime of the editor. + * existed. The in-place path is the common one: it mutates the existing entries rather than + * rebuilding the datasets, which is what matters because this runs once a second for the + * lifetime of the editor. It is not allocation-free -- each series reformats its legend label + * every tick -- so do not add work here on the assumption that it is. */ @UiThread fun onUsagesChanged(memoryUsage: IntObjectMap) { @@ -222,7 +224,7 @@ class MemoryUsageChartRenderer( ): String = "%s - %.2fMB".format(pname, megabytes) } -private const val BYTES_PER_MEGABYTE = 1024.0 * 1024.0 +internal const val BYTES_PER_MEGABYTE = 1024.0 * 1024.0 /** * The sample at [index] in megabytes. [MemoryUsageWatcher] stores bytes. diff --git a/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt index a2959fc4d8..5d1c8bab2a 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MemoryUsageChartRendererTest.kt @@ -126,15 +126,45 @@ class MemoryUsageChartRendererTest { @Test fun `onUsagesChanged after detach is a no-op`() { - val processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) + var processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 100)) val renderer = renderer { processes } - renderer.attach(chart()) + val detached = chart() + renderer.attach(detached) + val before = datasetFor(detached, 0).entries.map { it.y } + renderer.detach() - // A recycled carousel page must not keep the renderer writing into a dead view. + // Different samples, so a renderer that kept writing would visibly change the chart. + processes = arrayOf(proc(pid = 1, pname = "IDE", firstMegabytes = 900)) renderer.onUsagesChanged( MutableIntObjectMap().apply { put(1, processes[0]) }, ) + + // A recycled carousel page must not keep the renderer writing into a dead view. Asserting + // only that the call does not throw pinned nothing: it would not have thrown anyway. + assertThat(datasetFor(detached, 0).entries.map { it.y }).isEqualTo(before) + } + + @Test + fun `a swapped process rebuilds rather than plotting its samples on another line`() { + // The count stays the same and a pid changes -- what a tooling-server pid correction does. + // The suite covered only the count-changed path, and correctness here rested on + // getDataSetByIndex(-1) happening to return null. + val first = proc(pid = 1, pname = "IDE", firstMegabytes = 100) + var processes = arrayOf(first) + val renderer = renderer { processes } + val chart = chart() + renderer.attach(chart) + + val replacement = proc(pid = 2, pname = "Gradle Tooling", firstMegabytes = 700) + processes = arrayOf(replacement) + renderer.onUsagesChanged( + MutableIntObjectMap().apply { put(2, replacement) }, + ) + + assertThat(chart.data.dataSetCount).isEqualTo(1) + assertThat(datasetFor(chart, 0).label).startsWith("Gradle Tooling - ") + assertThat(datasetFor(chart, 0).entries.first().y).isEqualTo(700f) } @Test @@ -152,6 +182,11 @@ class MemoryUsageChartRendererTest { } private companion object { - const val BYTES_PER_MB = 1024L * 1024L + /** + * The production constant, not a copy of it. With its own literal the test verified its + * own arithmetic: change the renderer to decimal megabytes and every assertion still + * passed because both sides had stopped agreeing. + */ + val BYTES_PER_MB = BYTES_PER_MEGABYTE.toLong() } } diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 808f226785..927526c1ab 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1675,8 +1675,9 @@ Memory usage chart Memory usage - Code On The Go - Code On The Go logo + + Code on the Go logo From 092b633f8046f6ca193e65159032e383d1704833 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 08:14:26 -0700 Subject: [PATCH 08/10] refactor: delete SwipeRevealLayout's two dead horizontal drag helpers No behaviour change. Both were provably unreachable and still ran on every touch event and every animation frame. LeftDragCallback.tryCaptureView required a child whose id is R.id.drawer_sidebar. The layout asserts childCount == 2 and indexes its children positionally -- the hidden content and the overlapping content -- and drawer_sidebar is a FragmentContainerView inside the NavigationView, not a child here, so it was never true. RightDragCallback.tryCaptureView already returned false unconditionally, having been narrowed earlier in this stack when it was found capturing whichever child sat under a horizontal drag. Yet onInterceptTouchEvent still asked both helpers whether to intercept, onTouchEvent still fed both every event, and computeScroll still settled both on every frame. Their onViewPositionChanged also reported horizontal travel to dragListener as though it were vertical reveal progress, which is exactly the kind of thing a reader trusts and then debugs the wrong way round. Gone with them: leftDragProgress and rightDragProgress, both unread. Co-Authored-By: Claude Opus 5 --- .../itsaky/androidide/ui/SwipeRevealLayout.kt | 77 +------------------ 1 file changed, 2 insertions(+), 75 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt index 9e3066d0a6..429b9f6a18 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt @@ -67,11 +67,6 @@ open class SwipeRevealLayout ) } - private val leftDragHelper: ViewDragHelper - private val rightDragHelper: ViewDragHelper - - private var leftDragProgress = 0f - private var rightDragProgress = 0f private var isVerticalDragEnabled = true private var isDownInDragHandle = false @@ -84,8 +79,6 @@ open class SwipeRevealLayout private val dragHandleLocation = IntArray(2) init { - leftDragHelper = ViewDragHelper.create(this, 1f, LeftDragCallback()) - rightDragHelper = ViewDragHelper.create(this, 1f, RightDragCallback()) } private val dragHelperCallback = @@ -277,24 +270,17 @@ open class SwipeRevealLayout override fun onInterceptTouchEvent(ev: MotionEvent): Boolean { val action = ev.actionMasked if (action == MotionEvent.ACTION_CANCEL || action == MotionEvent.ACTION_UP) { - leftDragHelper.cancel() - rightDragHelper.cancel() dragHelper.cancel() return false } if (action == MotionEvent.ACTION_DOWN) { isDownInDragHandle = isTouchInDragHandle(ev) } - val isLeft = leftDragHelper.shouldInterceptTouchEvent(ev) - val isRight = rightDragHelper.shouldInterceptTouchEvent(ev) - val isVertical = dragHelper.shouldInterceptTouchEvent(ev) - return isLeft || isRight || isVertical + return dragHelper.shouldInterceptTouchEvent(ev) } @SuppressLint("ClickableViewAccessibility") override fun onTouchEvent(event: MotionEvent): Boolean { - leftDragHelper.processTouchEvent(event) - rightDragHelper.processTouchEvent(event) dragHelper.processTouchEvent(event) return true } @@ -325,7 +311,7 @@ open class SwipeRevealLayout } override fun computeScroll() { - if (leftDragHelper.continueSettling(true) or rightDragHelper.continueSettling(true) or dragHelper.continueSettling(true)) { + if (dragHelper.continueSettling(true)) { postInvalidateOnAnimation() } } @@ -410,63 +396,4 @@ open class SwipeRevealLayout postInvalidateOnAnimation() } } - - private inner class LeftDragCallback : ViewDragHelper.Callback() { - override fun tryCaptureView( - child: View, - pointerId: Int, - ): Boolean { - return child.id == R.id.drawer_sidebar // Your left drawer ID - } - - override fun onViewPositionChanged( - changedView: View, - left: Int, - top: Int, - dx: Int, - dy: Int, - ) { - leftDragProgress = left.toFloat() / changedView.width - dragListener?.onDragProgress(this@SwipeRevealLayout, leftDragProgress) - invalidate() - } - - override fun clampViewPositionHorizontal( - child: View, - left: Int, - dx: Int, - ): Int = max(0, min(left, width - child.width)) - } - - private inner class RightDragCallback : ViewDragHelper.Callback() { - // There is no right drawer in this layout: R.id.right_drawer_sidebar does not exist, so - // the intended check was commented out and this returned true for every child. That made - // the helper capture whichever child sat under a horizontal drag and offset it sideways, - // and its onViewPositionChanged reported that horizontal travel to dragListener as if it - // were vertical reveal progress. It also stole horizontal gestures from child views, so a - // horizontally scrolling child (ADFA-5487's metrics carousel) raced this helper for them. - // Capture nothing until a right drawer actually exists to name here. - override fun tryCaptureView( - child: View, - pointerId: Int, - ): Boolean = false - - override fun onViewPositionChanged( - changedView: View, - left: Int, - top: Int, - dx: Int, - dy: Int, - ) { - rightDragProgress = (width - left).toFloat() / changedView.width - dragListener?.onDragProgress(this@SwipeRevealLayout, rightDragProgress) - invalidate() - } - - override fun clampViewPositionHorizontal( - child: View, - left: Int, - dx: Int, - ): Int = max(width - child.width, min(left, width)) - } } From 68d7529f3a7e7a28fa0b527573654f294398a84b Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 06:55:57 -0700 Subject: [PATCH 09/10] ADFA-5487: drop the emptied init block, and put the KDoc back on its member 092b633f8 emptied this init { } when it removed the two dead drag helpers, and the blank line it left pushed isDownInDragHandle above its own doc comment -- so the doc described dragHandleLocation, an IntArray scratch, as the flag that gates the vertical drag capture. Both found by review on #1784. Nothing above this branch touches the file, so both were live at the top of the stack. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../java/com/itsaky/androidide/ui/SwipeRevealLayout.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt index 429b9f6a18..86a0ee2503 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/SwipeRevealLayout.kt @@ -69,17 +69,15 @@ open class SwipeRevealLayout private var isVerticalDragEnabled = true - private var isDownInDragHandle = false - /** * Whether the most recent touch-down landed within the configured drag handle. The vertical * drag-to-reveal gesture is only captured when this is `true`, so that scroll gestures starting * in the middle of the overlapping content (e.g. the editor) are not stolen. */ - private val dragHandleLocation = IntArray(2) + private var isDownInDragHandle = false - init { - } + /** Scratch for [View.getLocationOnScreen] while testing a touch against the handle's bounds. */ + private val dragHandleLocation = IntArray(2) private val dragHelperCallback = object : ViewDragHelper.Callback() { From 51821fce888cd2dcf8dbafb07c35cc6e9f433ad3 Mon Sep 17 00:00:00 2001 From: davidschachterADFA Date: Tue, 8 Sep 2026 18:34:57 -0700 Subject: [PATCH 10/10] ADFA-5489: Add a UID-level network traffic page to the metrics carousel (#1787) * feat: add a UID-level network traffic page to the metrics carousel Second page of the editor's metrics carousel (ADFA-5487) is now a live network traffic chart, replacing the brand-mark placeholder. Accounting is UID-level, as decided on the ticket: TrafficStats.getUidRxBytes / getUidTxBytes cover every process sharing the app's UID, so Gradle's downloads are included without any socket tagging -- the Gradle Tooling and daemon processes share it. There is deliberately no per-feature breakdown; the only two tagged sockets in the tree are the local documentation web server and the JDWP listener, neither of which is interesting here. The platform counters are cumulative since boot, so NetworkUsageWatcher records the delta between consecutive samples. Three cases the raw counters would get wrong: - The first sample only establishes a baseline and contributes 0. Otherwise the chart would open with a spike equal to everything the app had transferred since boot. - A counter that goes backwards (reboot, re-based accounting) records 0 rather than plotting negative traffic. - TrafficStats.UNSUPPORTED (-1), which some devices return, is detected once and latched, so -1 is never plotted as a byte count. getUsage() hands out copies rather than the live ring buffers, guarded by a lock. The renderer reads all 30 entries while the sampler thread appends, and MemoryUsageWatcher's equivalent has that race today. Axis, per the ticket's decisions: - Values are log10(bytes + 1). Traffic spans orders of magnitude -- a few hundred bytes of chatter next to a multi-megabyte download -- and a linear axis flattens all of it but the largest burst onto the baseline. MPAndroidChart has no logarithmic axis. - The + 1 floors zero, which is the common sample rather than an edge case: an idle IDE transfers nothing and log10(0) is negative infinity. A zero sample plots at exactly 0.0 and the line stays continuous. - Units are decimal (1 kB = 1000 B), not binary. This was not in the ticket and is a consequence of the log axis: on-device the first cut labelled the gridlines 9B / 99B / 999B / 9.8KB, because powers of ten divided by 1024 stop looking like decades. Decimal units label them 0B / 10B / 100B / 1.0kB, and are the convention for throughput. - Axis labels show 10^value rather than the exact inverse 10^value - 1, which would read 9B / 99B / 999B. One byte is not worth the confusion, and the legend carries the exact current figure. Zero is labelled exactly, since log10(0 + 1) really is 0. MetricsPage.Image and its layout go with the placeholder, having no remaining user; ADFA-5490 will define its own extension surface. The cogo_brand_mark drawable stays -- six other screens use it. Verified on a Pixel 6 Pro (arm64), v8 debug, over wifi with a real Gradle sync: - Both series track real traffic (peaks ~10kB/s against byte-level chatter, both legible on the one scale), idle periods sit flat on the 0B baseline, and the axis reads 0B / 10B / 100B / 1.0kB / 10.0kB. - Swiping to the memory page and back returns the full 30-sample history, so the page is recycling-safe like the memory one. - Font scale 1.0 and 2.0, measured on a cold start (EditorActivityKt declares fontScale in configChanges, so a warm relaunch reports stale geometry): title 22dp -> 35dp, pager 185dp -> 171dp, panel 248dp throughout, nothing clipped. - Landscape renders correctly, nothing clipped. - 16 new tests (7 watcher, 9 renderer); 80 tests green across app ui/utils/activities/fragments. ADFA-5489 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz * fix: label the network axis in whole units and rest zero on the baseline Two axis problems, one cosmetic and one a real rendering bug. Labels now read "10 kB" rather than "10.0kB". Gridlines sit on whole decades (granularity 1), so the mantissa is always exact and the decimal place carried no information. formatBytes takes the precision as an argument: none for axis labels, one place for the legend, where the figure is an arbitrary sample and the decimal does carry information. A space separates value from unit throughout. Zero now rests on the baseline. Two causes, both fixed: - The series were scaled against the wrong axis. LineDataSet defaults to axisDependency LEFT, and the labelled axis here is the right one, so the line was positioned by the disabled, auto-ranged left axis while the labels came from the right. The two only agree while both auto-range over the same data; pinning one made them disagree visibly -- an idle chart drew its zero line halfway up a plot whose baseline was labelled 0 B. - The range was not pinned. With every sample zero the data range is degenerate and the chart pads around it. applyAxisRange now fixes the minimum at 0 and the maximum at whole decades above the peak, with a floor of three decades so an idle chart keeps a sensible scale instead of collapsing onto a single value. Worth noting for review: the unit tests asserting axisMinimum and axisMaximum passed throughout, because the axis really was configured correctly -- the data simply was not drawn against it. Only the device showed it. There is now a test asserting the axis dependency of both series, which is the part that was untested. MemoryUsageChartRenderer has the same LEFT-dependency-with-RIGHT-labels shape and renders correctly, because it pins neither axis and both auto-range over the same data. Left alone. Verified on a Pixel 6 Pro (arm64), v8 debug: - Idle: both series rest exactly on the 0 B baseline, axis reads 0 B / 10 B / 100 B / 1 kB. - Under a Gradle sync: axis grows to 10 kB, peaks and zero-traffic troughs both legible, legend reads "212 B/s". - 49 tests green across app ui/utils, including four new ones covering the axis range, its growth across both series, whole-unit labels, and the axis dependency. ADFA-5489 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz * fix(metrics): make the network sampling loop stoppable and crash-proof (ADFA-5489) CodeRabbit raised three Major findings against this watcher. They were fixed, but on #1785 -- a later PR in the stack than the one that ships the bug. This PR is already approved and ahead of that one, so on its own it still carried all three. Moving the fix to where the defect lives. The scope had no parent Job and startWatching() supplied its own SupervisorJob per launch, so nothing the scope did could cancel the sampler. stopWatching() only lowered a flag the loop checks once per interval, and the loop spends nearly all its time in delay() -- up to 60s once ADFA-5486 makes the rate configurable. A stop and start inside that window left two loops appending to one buffer, splitting each delta between them. The scope now has a parent job, the launch is stored, and stopWatching() cancels it. Nothing caught exceptions inside the loop. An exception -- a misbehaving listener is enough -- ended the coroutine while `watching` stayed true, so every later startWatching() was refused as "already watching" and sampling was dead for the rest of the session. The body is wrapped, and CancellationException is rethrown so structured cancellation still works. The dedicated sampling thread was never released. close() is separate from stopWatching() on purpose: the editor stops and restarts the watcher across its lifecycle, and only the terminal teardown should give up the thread that newSingleThreadContext keeps alive. The activity's destroy path calls it. startWatching() now guards with compareAndSet rather than a read followed by a write, so two callers racing cannot each start a sampler. The watcher takes its dispatchers as parameters, matching MemoryUsageWatcher, so NetworkWatcherLifecycleTest can drive the loop on a virtual clock. Waiting on the wall clock is what hung the test executor the first time this was attempted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz * fix(metrics): actually apply the sampler fix, and stop a failed test spinning The previous commit shipped the commit message for this fix without the fix. An interrupted command had reverted the watcher to its pre-fix shape for a negative check and was killed before it restored it, so what got committed was `launch(SupervisorJob() + dispatcher)` and a scope cancel that cannot reach the sampler -- the very defect being fixed. stopWatching() now cancels the stored job, as its own comment already claimed. That mistake did prove the tests: against the unfixed watcher NetworkWatcherLifecycleTest reported two samples per interval where one was expected, which is exactly the two-loop overlap the fix exists to prevent. The tests also gained the cleanup they should have had. Each body now closes its watcher in a finally. Without it a failed assertion skipped close(), left the sampling loop live, and runTest's trailing advanceUntilIdle advanced virtual time forever -- a synchronous spin no test timeout can interrupt, which pinned a core and took the Gradle task to its ten-minute limit with no output. CodeRabbit raised exactly this about the tests on #1785; the lesson had not been carried over here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz * fix(metrics): re-baseline on resume, and stop sampling a device that cannot (ADFA-5489) Four review findings on the network watcher. A resume reported the whole gap as one interval. stopWatching() left lastRx and lastTx set, so the first sample afterwards took the delta against a counter read minutes earlier: background a Gradle download for three minutes and the legend read hundreds of MB/s while the axis stretched to match. The baseline is now dropped on stop, which is exactly what the null baseline already means elsewhere -- the next sample re-establishes it and contributes nothing. The baseline was also written outside the lock that clears it. sampleOnce wrote lastRx/lastTx on the sampler thread while clearHistory nulled them on the UI thread, so an interleaving could restore a pre-clear baseline and produce the same spike at the moment the user changed the sampling rate -- the failure the "cumulative baseline is dropped too" test exists to prevent, which it cannot see because it drives sampleOnce synchronously. listener was a plain var written by the UI thread and read by the sampler every tick, with no happens-before edge, so a null written in onPause could go unobserved and the sampler keep dispatching into a paused activity. Now @Volatile, as isSupported on the same class already was for the same reason. A device whose counters are unsupported kept the loop running anyway. isSupported latched false and sampleOnce returned immediately, but every interval still snapshotted the buffers, hopped to the main thread and repainted the chart with data known to be permanently zero. The loop now ends, and clears the watching flag as it goes so isWatching does not claim a sampler that has stopped. Not fixed here, deliberately: the legend's "/s" suffix. It is accurate on this branch, where the interval is a constructor value fixed at one second. It only becomes wrong once ADFA-5486 makes the rate user-settable, and only that branch has the interval available to the renderer, so the fix belongs there. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 (1M context) --- .../activities/editor/BaseEditorActivity.kt | 44 ++- .../androidide/ui/MetricsCarouselAdapter.kt | 55 ++-- .../ui/NetworkUsageChartRenderer.kt | 304 ++++++++++++++++++ .../androidide/utils/NetworkUsageWatcher.kt | 295 +++++++++++++++++ ...age.xml => item_metrics_network_chart.xml} | 7 +- app/src/main/res/values/dimens.xml | 1 - .../ui/NetworkUsageChartRendererTest.kt | 228 +++++++++++++ .../utils/NetworkUsageWatcherTest.kt | 203 ++++++++++++ .../utils/NetworkWatcherLifecycleTest.kt | 143 ++++++++ resources/src/main/res/values/strings.xml | 7 +- 10 files changed, 1238 insertions(+), 49 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt create mode 100644 app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt rename app/src/main/res/layout/{item_metrics_image.xml => item_metrics_network_chart.xml} (86%) create mode 100644 app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt create mode 100644 app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt create mode 100644 app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index e53ccd6c7d..e807ca6631 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -122,6 +122,7 @@ import com.itsaky.androidide.ui.ContentTranslatingDrawerLayout import com.itsaky.androidide.ui.MemoryUsageChartRenderer import com.itsaky.androidide.ui.MetricsCarouselAdapter import com.itsaky.androidide.ui.MetricsPage +import com.itsaky.androidide.ui.NetworkUsageChartRenderer import com.itsaky.androidide.ui.SwipeRevealLayout import com.itsaky.androidide.uidesigner.UIDesignerActivity import com.itsaky.androidide.utils.ActionMenuUtils.showPopupWindow @@ -131,6 +132,7 @@ import com.itsaky.androidide.utils.FlashType import com.itsaky.androidide.utils.InstallationResultHandler.onResult import com.itsaky.androidide.utils.IntentUtils import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.StringsInjectionException import com.itsaky.androidide.utils.StringsXmlInjector import com.itsaky.androidide.utils.applyBottomSheetAnchorForOrientation @@ -195,6 +197,15 @@ abstract class BaseEditorActivity : lineColorFor = Companion::getMemUsageLineColorFor, ) + private val networkUsageWatcher = NetworkUsageWatcher() + private val networkUsageChartRenderer = + NetworkUsageChartRenderer(usageProvider = networkUsageWatcher::getUsage) + + private val networkUsageListener = + NetworkUsageWatcher.NetworkUsageListener { usage -> + networkUsageChartRenderer.onUsageChanged(usage) + } + private val fileManagerViewModel by viewModels() private var feedbackButtonManager: FeedbackButtonManager? = null private var fullscreenManager: FullscreenManager? = null @@ -546,11 +557,15 @@ abstract class BaseEditorActivity : metricsPageCallback = null _binding?.memUsageView?.metricsPager?.adapter = null memUsageChartRenderer.detach() + networkUsageChartRenderer.detach() _binding = null if (isDestroying) { memoryUsageWatcher.stopWatching(true) memoryUsageWatcher.listener = null + // close(), not stopWatching(): this is the terminal teardown, and the watcher holds a + // dedicated sampling thread that newSingleThreadContext keeps alive until it is closed. + networkUsageWatcher.close() editorActivityScope.cancelIfActive("Activity is being destroyed") unbindDebuggerService() @@ -999,18 +1014,14 @@ abstract class BaseEditorActivity : private fun setupMetricsCarousel() { val pages = listOf( - // The memory chart is the default page (ADFA-5487). The logo is a placeholder second - // page until there is a real second metric; the network-traffic chart replaces it. + // The memory chart is the default page (ADFA-5487); network traffic is the second + // (ADFA-5489), replacing the brand-mark placeholder that ADFA-5487 shipped. MetricsPage.MemoryChart(title = string.metrics_title_memory), - MetricsPage.Image( - drawable = R.drawable.cogo_brand_mark, - description = string.metrics_carousel_brand_mark, - // The product's own name, from the one place it is defined. - title = string.app_name, - ), + MetricsPage.NetworkChart(title = string.metrics_title_network), ) - binding.memUsageView.metricsPager.adapter = MetricsCarouselAdapter(pages, memUsageChartRenderer) + binding.memUsageView.metricsPager.adapter = + MetricsCarouselAdapter(pages, memUsageChartRenderer, networkUsageChartRenderer) val showTitleFor = { position: Int -> pages.getOrNull(position)?.let { page -> @@ -1047,6 +1058,8 @@ abstract class BaseEditorActivity : super.onPause() memoryUsageWatcher.listener = null memoryUsageWatcher.stopWatching(false) + networkUsageWatcher.listener = null + networkUsageWatcher.stopWatching() this.isDestroying = isFinishing getFileTreeFragment()?.saveTreeState() @@ -1063,8 +1076,17 @@ abstract class BaseEditorActivity : log.warn("Unable to move debugger overlay to display {}", displayId, err) } - memoryUsageWatcher.listener = memoryUsageListener - memoryUsageWatcher.startWatching() + // Not for an instance onCreate already abandoned: the deep-link path calls finish() and + // returns, yet the platform still runs onStart and onResume. The memory watcher is immune + // by design -- it early-returns on an empty process set -- but the network sampler would + // poll TrafficStats and hop to the main thread once a second for an activity with no + // chart to render into. + if (didCompleteLiveOnCreate) { + memoryUsageWatcher.listener = memoryUsageListener + memoryUsageWatcher.startWatching() + networkUsageWatcher.listener = networkUsageListener + networkUsageWatcher.startWatching() + } apkInstallationViewModel.reloadStatus(this) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt index 4b602584fc..51dec3e2b0 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt @@ -20,8 +20,6 @@ package com.itsaky.androidide.ui import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.ImageView -import androidx.annotation.DrawableRes import androidx.annotation.StringRes import androidx.recyclerview.widget.RecyclerView import com.itsaky.androidide.R @@ -40,10 +38,8 @@ sealed interface MetricsPage { @StringRes override val title: Int, ) : MetricsPage - /** A static image. Placeholder page until real metrics exist to show alongside memory. */ - data class Image( - @DrawableRes val drawable: Int, - @StringRes val description: Int, + /** The live network-traffic chart, rendered by [NetworkUsageChartRenderer]. */ + data class NetworkChart( @StringRes override val title: Int, ) : MetricsPage } @@ -54,13 +50,14 @@ sealed interface MetricsPage { * [pages] is a constructor argument rather than a hardcoded list so that new displays -- a network * traffic chart, or pages contributed by plugins -- can be added without touching this class. * - * The chart page holds no sample state of its own: [chartRenderer] is attached when the page binds - * and detached when it is recycled, and rebuilds the full history from [MemoryUsageChartRenderer]'s - * watcher each time. Swiping away from the chart and back therefore loses nothing. + * A chart page holds no sample state of its own: its renderer is attached when the page binds and + * detached when it is recycled, and rebuilds the full history from its watcher each time. Moving + * away from a chart and back therefore loses nothing. */ class MetricsCarouselAdapter( private val pages: List, - private val chartRenderer: MemoryUsageChartRenderer, + private val memoryChartRenderer: MemoryUsageChartRenderer, + private val networkChartRenderer: NetworkUsageChartRenderer, ) : RecyclerView.Adapter() { sealed class PageViewHolder( view: View, @@ -69,9 +66,9 @@ class MetricsCarouselAdapter( val chart: SafeLineChart, ) : PageViewHolder(chart) - class Image( - val image: ImageView, - ) : PageViewHolder(image) + class NetworkChart( + val chart: SafeLineChart, + ) : PageViewHolder(chart) } override fun getItemCount(): Int = pages.size @@ -79,7 +76,7 @@ class MetricsCarouselAdapter( override fun getItemViewType(position: Int): Int = when (pages[position]) { is MetricsPage.MemoryChart -> VIEW_TYPE_MEMORY_CHART - is MetricsPage.Image -> VIEW_TYPE_IMAGE + is MetricsPage.NetworkChart -> VIEW_TYPE_NETWORK_CHART } override fun onCreateViewHolder( @@ -94,9 +91,9 @@ class MetricsCarouselAdapter( ) } - VIEW_TYPE_IMAGE -> { - PageViewHolder.Image( - inflater.inflate(R.layout.item_metrics_image, parent, false) as ImageView, + VIEW_TYPE_NETWORK_CHART -> { + PageViewHolder.NetworkChart( + inflater.inflate(R.layout.item_metrics_network_chart, parent, false) as SafeLineChart, ) } @@ -110,31 +107,29 @@ class MetricsCarouselAdapter( holder: PageViewHolder, position: Int, ) { - when (val page = pages[position]) { + when (pages[position]) { is MetricsPage.MemoryChart -> { - chartRenderer.attach((holder as PageViewHolder.MemoryChart).chart) + memoryChartRenderer.attach((holder as PageViewHolder.MemoryChart).chart) } - is MetricsPage.Image -> { - (holder as PageViewHolder.Image).image.apply { - setImageResource(page.drawable) - contentDescription = context.getString(page.description) - } + is MetricsPage.NetworkChart -> { + networkChartRenderer.attach((holder as PageViewHolder.NetworkChart).chart) } } } override fun onViewRecycled(holder: PageViewHolder) { - if (holder is PageViewHolder.MemoryChart) { - // Only if this holder's chart is still the attached one: a rebind can create the - // replacement before RecyclerView recycles the view it replaced, and detaching then - // would drop the new chart instead of the old. - chartRenderer.detachIfAttached(holder.chart) + // Only if this holder's chart is still the attached one: a rebind can create the replacement + // before RecyclerView recycles the view it replaced, and detaching then would drop the new + // chart instead of the old. + when (holder) { + is PageViewHolder.MemoryChart -> memoryChartRenderer.detachIfAttached(holder.chart) + is PageViewHolder.NetworkChart -> networkChartRenderer.detachIfAttached(holder.chart) } } private companion object { const val VIEW_TYPE_MEMORY_CHART = 0 - const val VIEW_TYPE_IMAGE = 1 + const val VIEW_TYPE_NETWORK_CHART = 1 } } diff --git a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt new file mode 100644 index 0000000000..2dae31b2f2 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt @@ -0,0 +1,304 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.graphics.Color +import androidx.annotation.UiThread +import com.github.mikephil.charting.components.AxisBase +import com.github.mikephil.charting.components.YAxis +import com.github.mikephil.charting.data.Entry +import com.github.mikephil.charting.data.LineData +import com.github.mikephil.charting.data.LineDataSet +import com.github.mikephil.charting.formatter.IAxisValueFormatter +import com.itsaky.androidide.R +import com.itsaky.androidide.utils.NetworkUsageWatcher +import com.itsaky.androidide.utils.NetworkUsageWatcher.NetworkUsage +import com.itsaky.androidide.utils.resolveAttr +import kotlin.math.ceil +import kotlin.math.log10 +import kotlin.math.max +import kotlin.math.pow +import kotlin.math.roundToLong + +/** + * Renders [NetworkUsageWatcher] samples into a [SafeLineChart] on a logarithmic scale (ADFA-5489). + * + * Traffic spans orders of magnitude -- a few hundred bytes of chatter next to a multi-megabyte + * Gradle download -- so a linear axis flattens everything but the largest burst into the baseline. + * MPAndroidChart has no logarithmic axis, so the plotted value is [log10] of the byte count and + * [BytesAxisFormatter] turns the axis labels back into byte units. + * + * Zero is the common sample, not an edge case: an idle IDE transfers nothing, and `log10(0)` is + * negative infinity. Values are therefore `log10(bytes + 1)`, which puts a zero sample at exactly + * `0.0` and keeps the line continuous. + * + * Like [MemoryUsageChartRenderer] this holds no sample state -- [NetworkUsageWatcher] owns the + * history -- so a chart can be attached, detached and recycled by the metrics carousel without + * losing anything. + * + * All methods must be called on the UI thread; MPAndroidChart is not thread-safe (see + * [SafeLineChart]). + * + * @param usageProvider Supplies the current sample history. + */ +class NetworkUsageChartRenderer( + private val usageProvider: () -> NetworkUsage, +) { + private var chart: SafeLineChart? = null + + @UiThread + fun attach(chart: SafeLineChart) { + this.chart = chart + configure(chart) + rebuild() + } + + @UiThread + fun detach() { + chart = null + } + + /** + * Detaches [chart] only if it is the currently attached one. See + * [MemoryUsageChartRenderer.detachIfAttached]. + */ + @UiThread + fun detachIfAttached(chart: SafeLineChart) { + if (this.chart === chart) { + detach() + } + } + + /** + * Rebuilds both series from the full sample history. + */ + @UiThread + fun rebuild() { + val chart = this.chart ?: return + val usage = usageProvider() + + val textColor = chart.context.resolveAttr(R.attr.colorOnSurface) + val bgColor = chart.context.resolveAttr(R.attr.colorSurfaceDim) + + val datasets = + arrayOf( + dataset(usage.received, chart.context.getString(R.string.metrics_network_received), RECEIVED_COLOR), + dataset(usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted), TRANSMITTED_COLOR), + ) + + applyAxisRange(chart, usage) + + chart.apply { + data = LineData(*datasets) + axisRight.textColor = textColor + axisLeft.textColor = textColor + legend.textColor = textColor + + data.setValueTextColor(textColor) + setBackgroundColor(bgColor) + setGridBackgroundColor(bgColor) + notifyDataSetChanged() + invalidate() + } + } + + /** + * Updates both series in place from a fresh sample, rebuilding if the chart's shape no longer + * matches. Allocates nothing on the common path, which runs once a second. + */ + @UiThread + fun onUsageChanged(usage: NetworkUsage) { + val chart = this.chart ?: return + val data = chart.data + + if (data == null || data.dataSetCount != SERIES_COUNT) { + rebuild() + return + } + + val received = data.getDataSetByIndex(RECEIVED_INDEX) as LineDataSet? + val transmitted = data.getDataSetByIndex(TRANSMITTED_INDEX) as LineDataSet? + if (received == null || transmitted == null || + received.entryCount != usage.received.size || + transmitted.entryCount != usage.transmitted.size + ) { + rebuild() + return + } + + update(received, usage.received, chart.context.getString(R.string.metrics_network_received)) + update(transmitted, usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted)) + + applyAxisRange(chart, usage) + + chart.apply { + data.notifyDataChanged() + notifyDataSetChanged() + invalidate() + } + } + + private fun dataset( + samples: LongArray, + label: String, + lineColor: Int, + ): LineDataSet = + LineDataSet( + List(samples.size) { index -> Entry(index.toFloat(), samples[index].toLogBytes()) }, + label, + ).apply { + // The labelled axis is the right one, and applyAxisRange pins its range. Without this the + // series is scaled against the (disabled, auto-ranged) left axis instead, so the line is + // drawn at a position the labels do not describe -- an idle chart plots its zero line + // halfway up a plot whose baseline is labelled 0 B. + axisDependency = YAxis.AxisDependency.RIGHT + color = lineColor + setDrawIcons(false) + setDrawCircles(false) + setDrawCircleHole(false) + setDrawValues(false) + formLineWidth = 1f + formSize = 15f + isHighlightEnabled = false + this.label = labelFor(label, samples.lastOrNull() ?: 0L) + } + + private fun update( + dataset: LineDataSet, + samples: LongArray, + label: String, + ) { + for (index in samples.indices) { + dataset.entries[index].y = samples[index].toLogBytes() + } + dataset.label = labelFor(label, samples.lastOrNull() ?: 0L) + dataset.notifyDataSetChanged() + } + + private fun labelFor( + label: String, + bytes: Long, + ): String = "%s - %s/s".format(label, formatBytes(bytes.toDouble(), decimals = 1)) + + /** + * Pins the axis to whole decades, from zero up to at least [MIN_AXIS_DECADES]. + * + * Two things depend on this. Zero has to sit on the baseline: when every sample is zero -- an + * idle IDE -- the data range is degenerate, and left to itself the chart pads around it and + * floats the flat line up the middle of the plot. And the maximum has to be a whole number, so + * the gridlines (granularity 1) land on exact powers of ten and can be labelled as whole units. + */ + private fun applyAxisRange( + chart: SafeLineChart, + usage: NetworkUsage, + ) { + val peak = max(usage.received.maxOrNull() ?: 0L, usage.transmitted.maxOrNull() ?: 0L) + chart.axisRight.axisMinimum = 0f + chart.axisRight.axisMaximum = ceil(peak.toLogBytes()).coerceAtLeast(MIN_AXIS_DECADES) + } + + private fun configure(chart: SafeLineChart) { + chart.apply { + val colorAccent = context.resolveAttr(R.attr.colorAccent) + + isDragEnabled = false + description.isEnabled = false + xAxis.axisLineColor = colorAccent + axisRight.axisLineColor = colorAccent + + setPinchZoom(false) + setBackgroundColor(context.resolveAttr(R.attr.colorSurfaceDim)) + setDrawGridBackground(true) + setScaleEnabled(true) + + axisLeft.isEnabled = false + axisRight.valueFormatter = BytesAxisFormatter + // One label per decade, so the gridlines read as 1 kB / 1 MB rather than arbitrary + // fractions of a logarithm. The range itself is set per sample by applyAxisRange. + axisRight.granularity = 1f + axisRight.isGranularityEnabled = true + } + } + + /** + * Labels a logarithmic axis value in byte units. + * + * Gridlines land on integer values (granularity 1), so each is a power of ten and is labelled as + * one: 10B, 100B, 1.0kB. The exact inverse of [toLogBytes] would be `10^value - 1`, which labels + * those same lines 9B, 99B, 999B -- correct to the byte but unreadable as a scale. The one byte + * is not worth the confusion; the legend carries the exact current figure. + * + * Zero is the exception and is labelled exactly: `log10(0 + 1)` is 0, so the baseline really is + * no traffic, not one byte. + */ + private object BytesAxisFormatter : IAxisValueFormatter { + override fun getFormattedValue( + value: Float, + axis: AxisBase?, + ): String = + if (value < 0.5f) { + formatBytes(0.0, decimals = 0) + } else { + // Gridlines are whole decades, so the mantissa is exact and needs no decimal place. + formatBytes(10.0.pow(value.toDouble()), decimals = 0) + } + } + + private companion object { + /** + * The axis always spans at least this many decades (0 B to 1 kB), so an idle chart keeps a + * sensible scale instead of collapsing onto a single value. + */ + const val MIN_AXIS_DECADES = 3f + + const val SERIES_COUNT = 2 + const val RECEIVED_INDEX = 0 + const val TRANSMITTED_INDEX = 1 + + val RECEIVED_COLOR = Color.CYAN + val TRANSMITTED_COLOR = Color.MAGENTA + } +} + +/** + * The plotted value for a byte count: `log10(bytes + 1)`. + * + * The `+ 1` is what makes zero plottable -- it maps to `0.0` rather than negative infinity -- and + * zero is the usual sample for an idle IDE. + */ +private fun Long.toLogBytes(): Float = log10(this.coerceAtLeast(0L).toDouble() + 1.0).toFloat() + +/** + * Formats a byte count for an axis label or legend, to at most one decimal place. + * + * Units are decimal (1 kB = 1000 B), not binary. On a log10 axis the gridlines are powers of ten, + * and dividing those by 1024 would label them 9.8KB, 977KB, 954MB -- the decades stop looking like + * decades. Decimal units are also the convention for network throughput. + */ +private fun formatBytes( + bytes: Double, + decimals: Int, +): String { + val clamped = bytes.coerceAtLeast(0.0) + return when { + clamped < 1_000 -> "%d B".format(clamped.roundToLong()) + clamped < 1_000_000 -> "%.${decimals}f kB".format(clamped / 1_000) + clamped < 1_000_000_000 -> "%.${decimals}f MB".format(clamped / 1_000_000) + else -> "%.${decimals}f GB".format(clamped / 1_000_000_000) + } +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt new file mode 100644 index 0000000000..9499f527d9 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -0,0 +1,295 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import android.net.TrafficStats +import android.os.Process +import androidx.annotation.VisibleForTesting +import com.itsaky.androidide.tasks.cancelIfActive +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExecutorCoroutineDispatcher +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.withContext +import org.slf4j.LoggerFactory +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.coroutines.CoroutineContext + +/** + * Samples this app's network traffic (ADFA-5489). + * + * Accounting is UID-level, not per socket: [TrafficStats.getUidRxBytes] and + * [TrafficStats.getUidTxBytes] cover every process sharing the app's UID, which is what makes + * Gradle's downloads show up here -- the Gradle Tooling and daemon processes share it. No socket + * tagging is involved, so there is deliberately no per-feature breakdown. + * + * The platform counters are cumulative since boot, so what is recorded is the *delta* between + * consecutive samples: bytes transferred during that interval. A sampler that reported the raw + * counters would draw a monotonically rising line that says nothing about current activity. + * + * @param updateInterval Milliseconds between samples. + * @param uid The UID to account for. Defaults to this process's own; injectable for tests. + * @param readRxBytes Reads the cumulative received byte count. Injectable for tests. + * @param readTxBytes Reads the cumulative transmitted byte count. Injectable for tests. + */ +class NetworkUsageWatcher + @OptIn(ExperimentalCoroutinesApi::class, DelicateCoroutinesApi::class) + constructor( + private val updateInterval: Long = DEFAULT_UPDATE_INTERVAL, + private val uid: Int = Process.myUid(), + private val readRxBytes: (Int) -> Long = TrafficStats::getUidRxBytes, + private val readTxBytes: (Int) -> Long = TrafficStats::getUidTxBytes, + // Injectable so a test can drive the sampling loop on a virtual clock. Waiting on the wall + // clock instead is what hung the test executor the first time this was attempted. + private val coroutineDispatcher: CoroutineContext = newSingleThreadContext("NetworkUsageWatcher"), + private val mainDispatcher: CoroutineContext = Dispatchers.Main.immediate, + ) { + // A parent job, so cancelling the scope in close() actually reaches the sampler. Without one + // the launch below had to supply its own, and nothing the scope did could stop it. + private val coroutineScope = CoroutineScope(SupervisorJob() + coroutineDispatcher) + private val watching = AtomicBoolean(false) + + /** The running sampling loop, so [stopWatching] can actually stop it. */ + private var samplingJob: Job? = null + + /** Guards the two ring buffers: the sampler writes them, the UI thread snapshots them. */ + private val historyLock = Any() + + private val received = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + private val transmitted = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + + /** + * The previous cumulative readings, or `null` before the first sample. The first sample + * establishes a baseline and contributes no delta -- the alternative would be a spike equal to + * everything the app had transferred since boot. + */ + private var lastRx: Long? = null + private var lastTx: Long? = null + + /** + * Whether the platform reports traffic for this UID at all. Cleared permanently if a read comes + * back [TrafficStats.UNSUPPORTED], which some devices and emulators do. + */ + @Volatile + var isSupported: Boolean = true + private set + + val isWatching: Boolean + get() = watching.get() + + /** + * Notified on the main thread after each sample. + */ + @Volatile + var listener: NetworkUsageListener? = null + + /** + * A snapshot of the sampled history, oldest first. Safe to call from any thread at any time; + * before the first sample every entry is zero. + * + * The arrays are copies. Handing out the live ring buffers would let the caller read them while + * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. + */ + fun getUsage(): NetworkUsage = + synchronized(historyLock) { + NetworkUsage(received.snapshot(), transmitted.snapshot()) + } + + fun startWatching() { + // compareAndSet, not a read then a write: two callers racing here would each start a + // sampler, and both would append to the same buffers. + if (!watching.compareAndSet(false, true)) { + log.warn("Network usage is already being watched") + return + } + + samplingJob = + coroutineScope.launch { + while (isWatching) { + // The loop must outlive a bad sample. Without this an exception -- a + // misbehaving listener is enough -- ends the coroutine while `watching` stays + // true, so every later startWatching() is refused as "already watching" and + // sampling is dead for the rest of the session. + runCatching { + sampleOnce() + + listener?.also { listener -> + val usage = getUsage() + withContext(mainDispatcher) { + listener.onNetworkUsageChanged(usage) + } + } + }.onFailure { failure -> + if (failure is CancellationException) { + throw failure + } + log.error("Network usage sampling failed; continuing", failure) + } + + // A device whose counters are unsupported has nothing further to give, and + // the loop was otherwise repainting three charts a second with data known + // to be permanently zero. Clearing the flag too, so isWatching does not + // claim a sampler that has stopped. + if (!isSupported) { + watching.set(false) + break + } + + delay(updateInterval) + } + } + } + + /** + * Stops sampling. The watcher can be started again; the history is kept. + */ + fun stopWatching() { + watching.set(false) + // Drop the cumulative baseline as well. Left set, the first sample after a resume + // reports everything transferred while the watcher was stopped as a single interval -- + // background a Gradle download for three minutes and the chart reads hundreds of MB/s. + // The next sample re-establishes it, which is what the null baseline means. + synchronized(historyLock) { + lastRx = null + lastTx = null + } + // Cancel the job, not the scope. The loop spends nearly all its time in delay(), so waiting + // for it to notice the flag leaves it sampling for up to a full interval after the editor + // asked it to stop -- long enough for a stop/start to run two samplers at once. Cancelling + // the scope instead would end the watcher for good, and this is a pause, not a teardown. + samplingJob?.cancel() + samplingJob = null + } + + /** + * Stops sampling and releases the sampling thread. Terminal: the watcher cannot be restarted. + * + * Separate from [stopWatching] because the editor stops and restarts the watcher across its + * lifecycle, and only the final teardown should give up the thread that + * [newSingleThreadContext] keeps alive. + */ + fun close() { + stopWatching() + listener = null + coroutineScope.cancelIfActive("Watcher closed") + (coroutineDispatcher as? ExecutorCoroutineDispatcher)?.close() + } + + /** + * Takes one sample. The sampling loop calls this once per [updateInterval]; tests call it + * directly so the delta accounting can be exercised without threads or waiting. + */ + @VisibleForTesting + internal fun sampleOnce() { + if (!isSupported) { + return + } + + val rx = readRxBytes(uid) + val tx = readTxBytes(uid) + + if (rx == UNSUPPORTED || tx == UNSUPPORTED) { + // Not transient: the platform either accounts for this UID or it does not. + isSupported = false + log.info("Network usage is unavailable on this device; the traffic chart will read zero") + return + } + + synchronized(historyLock) { + record(received, previous = lastRx, current = rx) + record(transmitted, previous = lastTx, current = tx) + } + + synchronized(historyLock) { + lastRx = rx + lastTx = tx + } + } + + /** + * Appends the delta between [previous] and [current] to [history]. + * + * A negative delta means the counter went backwards, which happens when it is reset -- the + * device rebooted, or the platform re-based its accounting. Treated as a fresh baseline (zero + * for this interval) rather than plotted as negative traffic. + */ + private fun record( + history: MutableShiftedLongArray, + previous: Long?, + current: Long, + ) { + val delta = + when { + previous == null -> 0L + current < previous -> 0L + else -> current - previous + } + + // Newest entry goes in at index 0 and the shift makes it the last element, so + // history[size - 1] is always the newest. Same convention as MemoryUsageWatcher. + history[0] = delta + history.shift(1) + } + + /** + * Bytes transferred per sampling interval, oldest first. + * + * @property received Bytes received during each interval. + * @property transmitted Bytes transmitted during each interval. + */ + data class NetworkUsage( + val received: LongArray, + val transmitted: LongArray, + ) { + override fun equals(other: Any?): Boolean = + this === other || + ( + other is NetworkUsage && + received.contentEquals(other.received) && + transmitted.contentEquals(other.transmitted) + ) + + override fun hashCode(): Int = 31 * received.contentHashCode() + transmitted.contentHashCode() + } + + fun interface NetworkUsageListener { + fun onNetworkUsageChanged(usage: NetworkUsage) + } + + companion object { + const val MAX_USAGE_ENTRIES = 30 + const val DEFAULT_UPDATE_INTERVAL = 1000L + + /** [TrafficStats.UNSUPPORTED] widened to [Long], which is what the getters return. */ + private const val UNSUPPORTED = TrafficStats.UNSUPPORTED.toLong() + + private val log = LoggerFactory.getLogger(NetworkUsageWatcher::class.java) + } + } + +/** + * Copies this ring buffer into a plain array in logical order, oldest first. + */ +private fun ShiftedLongArray.snapshot(): LongArray = LongArray(size) { this[it] } diff --git a/app/src/main/res/layout/item_metrics_image.xml b/app/src/main/res/layout/item_metrics_network_chart.xml similarity index 86% rename from app/src/main/res/layout/item_metrics_image.xml rename to app/src/main/res/layout/item_metrics_network_chart.xml index 4d8617b328..f011080f06 100644 --- a/app/src/main/res/layout/item_metrics_image.xml +++ b/app/src/main/res/layout/item_metrics_network_chart.xml @@ -5,10 +5,9 @@ PURPOSE. See the ~ GNU General Public License for more details. ~ ~ You should have received a copy of the GNU General Public License ~ along with AndroidIDE. If not, see . --> - + android:contentDescription="@string/metrics_network_chart" /> diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index c3bbde87e7..f1785efd39 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -9,7 +9,6 @@ 248dp 16dp 4dp - 16dp 28dp 28dp diff --git a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt new file mode 100644 index 0000000000..d24b027cd8 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt @@ -0,0 +1,228 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.github.mikephil.charting.components.YAxis +import com.github.mikephil.charting.data.LineDataSet +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.NetworkUsageWatcher +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.math.log10 + +/** + * Pins the two axis decisions ADFA-5489 was scoped around: values are log10, and zero is floored + * via `log10(bytes + 1)` so an idle IDE plots a continuous line at 0 instead of negative infinity. + */ +@RunWith(RobolectricTestRunner::class) +class NetworkUsageChartRendererTest { + private val context = ApplicationProvider.getApplicationContext() + + private fun usage( + received: LongArray, + transmitted: LongArray = received, + ) = NetworkUsageWatcher.NetworkUsage(received, transmitted) + + private fun rendererFor(usage: NetworkUsageWatcher.NetworkUsage): Pair { + val chart = SafeLineChart(context) + val renderer = NetworkUsageChartRenderer(usageProvider = { usage }) + renderer.attach(chart) + return renderer to chart + } + + private fun dataset( + chart: SafeLineChart, + index: Int, + ) = chart.data.getDataSetByIndex(index) as LineDataSet + + @Test + fun `plots log10 of the byte count`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 9L, 99L, 999L))) + + val ys = dataset(chart, 0).entries.map { it.y } + + // log10(n + 1): 0 -> 0, 9 -> 1, 99 -> 2, 999 -> 3. Exact decades, so the floor is visible. + assertThat(ys).containsExactly(0f, 1f, 2f, 3f).inOrder() + } + + @Test + fun `zero bytes plots at zero rather than negative infinity`() { + val (_, chart) = rendererFor(usage(LongArray(30) { 0L })) + + val ys = dataset(chart, 0).entries.map { it.y } + + assertThat(ys.none { it.isInfinite() || it.isNaN() }).isTrue() + assertThat(ys.toSet()).containsExactly(0f) + } + + @Test + fun `a megabyte burst stays on scale with surrounding chatter`() { + val bytes = longArrayOf(0L, 512L, 2L * 1024 * 1024, 256L) + val (_, chart) = rendererFor(usage(bytes)) + + val ys = dataset(chart, 0).entries.map { it.y } + + // The point of the log axis: a 2MB burst is ~6.3 while 512B is ~2.7, so the small values + // stay legible instead of being flattened onto the baseline. + assertThat(ys[2]).isWithin(0.01f).of(log10(2.0 * 1024 * 1024 + 1).toFloat()) + assertThat(ys[1]).isGreaterThan(2f) + assertThat(ys[2] - ys[1]).isLessThan(4f) + } + + @Test + fun `received and transmitted are separate series`() { + val (_, chart) = + rendererFor( + usage( + received = longArrayOf(0L, 999L), + transmitted = longArrayOf(0L, 9L), + ), + ) + + assertThat(chart.data.dataSetCount).isEqualTo(2) + assertThat(dataset(chart, 0).entries.last().y).isEqualTo(3f) + assertThat(dataset(chart, 1).entries.last().y).isEqualTo(1f) + } + + @Test + fun `the legend reports the latest sample in byte units`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 2_000L))) + + // Rendered from the raw byte count, not from the logarithm, and in decimal units so that + // the log10 axis labels come out as clean decades. + assertThat(dataset(chart, 0).label).endsWith("2.0 kB/s") + } + + @Test + fun `onUsageChanged updates entries in place without replacing the datasets`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 9L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + val datasetBefore = dataset(chart, 0) + val entryBefore = datasetBefore.entries.last() + + current = usage(longArrayOf(0L, 999L)) + renderer.onUsageChanged(current) + + assertThat(dataset(chart, 0)).isSameInstanceAs(datasetBefore) + assertThat(datasetBefore.entries.last()).isSameInstanceAs(entryBefore) + assertThat(entryBefore.y).isEqualTo(3f) + } + + @Test + fun `onUsageChanged rebuilds when the sample count changes`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 9L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + assertThat(dataset(chart, 0).entryCount).isEqualTo(2) + + current = usage(longArrayOf(0L, 9L, 99L)) + renderer.onUsageChanged(current) + + assertThat(dataset(chart, 0).entryCount).isEqualTo(3) + } + + @Test + fun `attach after detach renders the history into the new chart`() { + val current = usage(longArrayOf(0L, 99L)) + val (renderer, _) = rendererFor(current) + renderer.detach() + + val rebound = SafeLineChart(context) + renderer.attach(rebound) + + assertThat(dataset(rebound, 0).entries.last().y).isEqualTo(2f) + } + + @Test + fun `axis labels are whole units with no decimal place`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 10_000_000L))) + val formatter = chart.axisRight.valueFormatter + + // Gridlines sit on whole decades, so the mantissa is exact. + assertThat(formatter.getFormattedValue(0f, chart.axisRight)).isEqualTo("0 B") + assertThat(formatter.getFormattedValue(1f, chart.axisRight)).isEqualTo("10 B") + assertThat(formatter.getFormattedValue(3f, chart.axisRight)).isEqualTo("1 kB") + assertThat(formatter.getFormattedValue(4f, chart.axisRight)).isEqualTo("10 kB") + assertThat(formatter.getFormattedValue(6f, chart.axisRight)).isEqualTo("1 MB") + } + + @Test + fun `the series are scaled against the labelled axis`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 100L))) + + // The right axis is the one carrying the labels and the pinned range. A dataset left on the + // default LEFT dependency is drawn against the auto-ranged left axis, so the line lands + // somewhere the labels do not describe -- which is invisible to an assertion on the axis + // alone, and was only caught on a device. + assertThat(dataset(chart, 0).axisDependency).isEqualTo(YAxis.AxisDependency.RIGHT) + assertThat(dataset(chart, 1).axisDependency).isEqualTo(YAxis.AxisDependency.RIGHT) + } + + @Test + fun `an idle chart keeps zero on the baseline`() { + // Every sample zero. Left to itself the chart pads around a degenerate range and floats the + // flat line up the middle of the plot instead of resting it on the axis minimum. + val (_, chart) = rendererFor(usage(LongArray(30) { 0L })) + + assertThat(chart.axisRight.axisMinimum).isEqualTo(0f) + assertThat(chart.axisRight.axisMaximum).isEqualTo(3f) + } + + @Test + fun `the axis grows to whole decades around the peak`() { + // 2 MB peak -> log10 is ~6.3, so the axis tops out at the 10 MB decade. + val (_, chart) = rendererFor(usage(longArrayOf(0L, 2_000_000L))) + + assertThat(chart.axisRight.axisMinimum).isEqualTo(0f) + assertThat(chart.axisRight.axisMaximum).isEqualTo(7f) + } + + @Test + fun `the axis follows the peak across both series`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 100L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + assertThat(chart.axisRight.axisMaximum).isEqualTo(3f) + + // A burst on the transmitted series alone must still lift the axis. + current = usage(received = longArrayOf(0L, 100L), transmitted = longArrayOf(0L, 500_000L)) + renderer.onUsageChanged(current) + + assertThat(chart.axisRight.axisMaximum).isEqualTo(6f) + } + + @Test + fun `onUsageChanged after detach is a no-op`() { + val current = usage(longArrayOf(0L, 99L)) + val (renderer, _) = rendererFor(current) + renderer.detach() + + // A recycled carousel page must not keep the renderer writing into a dead view. + renderer.onUsageChanged(current) + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt new file mode 100644 index 0000000000..963321cd01 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt @@ -0,0 +1,203 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins the accounting decisions ADFA-5489 was scoped around: the platform counters are cumulative, + * so what is plotted is the delta between samples, and a counter reset must not plot as negative + * traffic. + * + * These drive [NetworkUsageWatcher.sampleOnce] directly rather than starting the sampling loop, so + * there is no waiting and no dependence on thread timing. + */ +@RunWith(RobolectricTestRunner::class) +class NetworkUsageWatcherTest { + /** Every watcher built here, so the sampling thread each one starts is released. */ + private val created = mutableListOf() + + @After + fun tearDown() { + created.forEach { it.close() } + created.clear() + } + + /** + * A watcher fed a scripted sequence of cumulative readings, advancing one step per sample. + */ + private inner class Fixture( + rx: List, + tx: List = rx, + ) { + private var index = -1 + private val rxReadings = rx + private val txReadings = tx + + val watcher = + NetworkUsageWatcher( + uid = TEST_UID, + readRxBytes = { rxReadings[index.coerceIn(0, rxReadings.lastIndex)] }, + readTxBytes = { txReadings[index.coerceIn(0, txReadings.lastIndex)] }, + ).also { created += it } + + /** Takes [count] samples, walking the scripted readings. */ + fun sample(count: Int) { + repeat(count) { + index++ + watcher.sampleOnce() + } + } + } + + /** The last [count] recorded samples, ignoring the leading zeros of an unfilled buffer. */ + private fun LongArray.recent(count: Int): List = takeLast(count) + + @Test + fun `history is all zeros before the first sample`() { + val fixture = Fixture(listOf(5_000L)) + + val usage = fixture.watcher.getUsage() + + assertThat(usage.received).hasLength(NetworkUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(usage.received.sum()).isEqualTo(0L) + assertThat(usage.transmitted.sum()).isEqualTo(0L) + } + + @Test + fun `plots deltas between samples, not the cumulative counters`() { + // Cumulative since boot: 1000, then +500, then +2500. + val fixture = Fixture(listOf(1_000L, 1_500L, 4_000L)) + + fixture.sample(3) + val usage = fixture.watcher.getUsage() + + // The first sample only establishes a baseline, so it contributes 0 rather than a + // 1000-byte spike for traffic that happened before the chart existed. + assertThat(usage.received.recent(3)).containsExactly(0L, 500L, 2_500L).inOrder() + assertThat(usage.transmitted.recent(3)).containsExactly(0L, 500L, 2_500L).inOrder() + } + + @Test + fun `a counter reset records zero rather than negative traffic`() { + // A reboot or re-based accounting makes the counter go backwards. + val fixture = Fixture(listOf(10_000L, 10_400L, 200L, 700L)) + + fixture.sample(4) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received.recent(4)).containsExactly(0L, 400L, 0L, 500L).inOrder() + assertThat(usage.received.none { it < 0L }).isTrue() + } + + @Test + fun `received and transmitted are accounted separately`() { + val fixture = + Fixture( + rx = listOf(0L, 1_000L), + tx = listOf(0L, 7L), + ) + + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received.recent(2)).containsExactly(0L, 1_000L).inOrder() + assertThat(usage.transmitted.recent(2)).containsExactly(0L, 7L).inOrder() + } + + @Test + fun `the ring buffer keeps only the most recent samples`() { + val capacity = NetworkUsageWatcher.MAX_USAGE_ENTRIES + // Cumulative readings rising by 10 bytes each sample, for one more sample than fits. + val readings = List(capacity + 2) { it * 10L } + val fixture = Fixture(readings) + + fixture.sample(readings.size) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received).hasLength(capacity) + // The baseline zero has been pushed out; every retained sample is a full 10-byte delta. + assertThat(usage.received.toList()).containsNoneIn(listOf(-10L)) + assertThat(usage.received.last()).isEqualTo(10L) + assertThat(usage.received.sum()).isEqualTo(10L * capacity) + } + + @Test + fun `an unsupported counter is detected and nothing is recorded`() { + // TrafficStats.UNSUPPORTED is -1. + val fixture = Fixture(listOf(-1L)) + + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + assertThat(fixture.watcher.isSupported).isFalse() + // In particular, -1 is not plotted as traffic. + assertThat(usage.received.sum()).isEqualTo(0L) + assertThat(usage.transmitted.sum()).isEqualTo(0L) + } + + @Test + fun `getUsage returns a copy, not the live buffer`() { + val fixture = Fixture(listOf(0L, 100L, 300L)) + + fixture.sample(2) + val first = fixture.watcher.getUsage() + val asHandedOut = first.received.copyOf() + fixture.sample(1) + + // The array handed out earlier must not have been mutated by the later sample. + assertThat(first.received).isEqualTo(asHandedOut) + assertThat(fixture.watcher.getUsage().received).isNotEqualTo(asHandedOut) + } + + @Test + fun `stopping drops the cumulative baseline so a resume does not spike`() { + // 1 MB transferred, then the watcher is stopped while a download keeps running. + val fixture = Fixture(listOf(1_000_000L, 1_000_000L, 250_000_000L, 250_500_000L)) + fixture.sample(2) + + fixture.watcher.stopWatching() + + // Resume: the counter has moved by 249 MB while nothing was watching. + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + // Kept, the baseline turns the whole gap into one interval's traffic -- the legend reads + // hundreds of MB/s and the axis is stretched for the next minute. + assertThat(usage.received.recent(2)).containsExactly(0L, 500_000L).inOrder() + } + + @Test + fun `an unsupported counter stops the watcher rather than sampling zeroes forever`() { + val fixture = Fixture(listOf(-1L)) + + fixture.sample(1) + + // Nothing more to read, so nothing more to do: the loop was repainting the charts once a + // second with data known to be permanently unavailable. + assertThat(fixture.watcher.isSupported).isFalse() + } + + private companion object { + const val TEST_UID = 10_123 + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt new file mode 100644 index 0000000000..b990dfa45f --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt @@ -0,0 +1,143 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins the sampling loop's lifecycle (ADFA-5489). + * + * The loop spends nearly all of its time in `delay()`, so "stopped" cannot mean "will notice a + * flag eventually": between the request and the next tick the watcher is still sampling, and a + * stop followed by a start inside that window used to leave two loops appending to one buffer. + * + * Driven on a virtual clock. Waiting on the wall clock instead is what hung the test executor the + * first time this was attempted. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class NetworkWatcherLifecycleTest { + private fun watcher( + dispatcher: kotlin.coroutines.CoroutineContext, + onSample: () -> Unit = {}, + ): NetworkUsageWatcher { + var counter = 0L + return NetworkUsageWatcher( + updateInterval = INTERVAL_MS, + uid = TEST_UID, + readRxBytes = { + onSample() + counter += 100L + counter + }, + readTxBytes = { counter }, + coroutineDispatcher = dispatcher, + mainDispatcher = dispatcher, + ) + } + + @Test + fun `stopping inside the sampling interval actually stops sampling`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + + try { + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 3) + val whileRunning = samples + + watcher.stopWatching() + advanceTimeBy(INTERVAL_MS * 5) + + // Cancelling the job rather than waiting for the loop to observe a flag is what makes + // this exact: nothing is sampled after the stop. + assertThat(whileRunning).isGreaterThan(0) + assertThat(samples).isEqualTo(whileRunning) + assertThat(watcher.isWatching).isFalse() + } finally { + // Closed here rather than after the assertions: see the class KDoc. + watcher.close() + } + } + + @Test + fun `restarting inside the sampling interval does not leave two loops running`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + + try { + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 2) + watcher.stopWatching() + watcher.startWatching() + + val before = samples + advanceTimeBy(INTERVAL_MS * 4) + + // The raw count, not a rate: integer division passed for anything from four to + // seven samples, so a second loop that only partly overlapped went unnoticed. + assertThat(samples - before).isEqualTo(4) + } finally { + watcher.close() + } + } + + @Test + fun `a listener that throws does not kill sampling for the rest of the session`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + + try { + var thrown = 0 + watcher.listener = + NetworkUsageWatcher.NetworkUsageListener { + if (thrown++ == 0) { + throw IllegalStateException("listener blew up") + } + } + + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 4) + + // Uncaught, the exception ends the coroutine while isWatching stays true, so every + // later startWatching() is refused and the charts freeze for good. + assertThat(samples).isGreaterThan(1) + assertThat(watcher.isWatching).isTrue() + } finally { + watcher.close() + } + } + + private companion object { + const val INTERVAL_MS = 1_000L + const val TEST_UID = 10_123 + } +} diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index f9b2b6908f..e968ccb924 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1682,9 +1682,10 @@ Memory usage chart Memory usage - - Code on the Go logo + Network traffic chart + Network traffic + Received + Sent