Move file paging into ScreenRecoveryUI.

This fixes the N9 performance problem.

Change-Id: I00c10d4162ff266a6243285e5a5e768217f6f799

recovery.cpp

@@ -84,9 +84,6 @@ static const char *LAST_KMSG_FILE = "/cache/recovery/last_kmsg";
 
 #define KEEP_LOG_COUNT 10
 
-// Number of lines per page when displaying a file on screen
-#define LINES_PER_PAGE 30
-
 RecoveryUI* ui = NULL;
 char* locale = NULL;
 char recovery_version[PROPERTY_VALUE_MAX+1];
@@ -737,58 +734,6 @@ static bool wipe_cache(bool should_confirm, Device* device) {
     }
 }
 
-static void file_to_ui(const char* fn) {
-    FILE *fp = fopen_path(fn, "re");
-    if (fp == NULL) {
-        ui->Print("  Unable to open %s: %s\n", fn, strerror(errno));
-        return;
-    }
-    char line[1024];
-    int ct = 0;
-    int key = 0;
-    redirect_stdio("/dev/null");
-    while(fgets(line, sizeof(line), fp) != NULL) {
-        ui->Print("%s", line);
-        ct++;
-        if (ct % LINES_PER_PAGE == 0) {
-            // give the user time to glance at the entries
-            key = ui->WaitKey();
-
-            if (key == KEY_POWER) {
-                break;
-            }
-
-            if (key == KEY_VOLUMEUP) {
-                // Go back by seeking to the beginning and dumping ct - n
-                // lines.  It's ugly, but this way we don't need to store
-                // the previous offsets.  The files we're dumping here aren't
-                // expected to be very large.
-                int i;
-
-                ct -= 2 * LINES_PER_PAGE;
-                if (ct < 0) {
-                    ct = 0;
-                }
-                fseek(fp, 0, SEEK_SET);
-                for (i = 0; i < ct; i++) {
-                    fgets(line, sizeof(line), fp);
-                }
-                ui->Print("^^^^^^^^^^\n");
-            }
-        }
-    }
-
-    // If the user didn't abort, then give the user time to glance at
-    // the end of the log, sorry, no rewind here
-    if (key != KEY_POWER) {
-        ui->Print("\n--END-- (press any key)\n");
-        ui->WaitKey();
-    }
-
-    redirect_stdio(TEMPORARY_LOG_FILE);
-    fclose(fp);
-}
-
 static void choose_recovery_file(Device* device) {
     unsigned int i;
     unsigned int n;
@@ -823,10 +768,14 @@ static void choose_recovery_file(Device* device) {
 
     const char* headers[] = { "Select file to view", "", NULL };
 
-    while(1) {
+    while (true) {
         int chosen_item = get_menu_selection(headers, entries, 1, 0, device);
         if (chosen_item == 0) break;
-        file_to_ui(entries[chosen_item]);
+
+        // TODO: do we need to redirect? ShowFile could just avoid writing to stdio.
+        redirect_stdio("/dev/null");
+        ui->ShowFile(entries[chosen_item]);
+        redirect_stdio(TEMPORARY_LOG_FILE);
     }
 
     for (i = 0; i < (sizeof(entries) / sizeof(*entries)); i++) {

screen_ui.cpp

@@ -86,8 +86,7 @@ ScreenRecoveryUI::ScreenRecoveryUI() :
 
 // Clear the screen and draw the currently selected background icon (if any).
 // Should only be called with updateMutex locked.
-void ScreenRecoveryUI::draw_background_locked(Icon icon)
+void ScreenRecoveryUI::draw_background_locked(Icon icon) {
-{
     pagesIdentical = false;
     gr_color(0, 0, 0, 255);
     gr_clear();
@@ -132,8 +131,7 @@ void ScreenRecoveryUI::draw_background_locked(Icon icon)
 
 // Draw the progress bar (if any) on the screen.  Does not flip pages.
 // Should only be called with updateMutex locked.
-void ScreenRecoveryUI::draw_progress_locked()
+void ScreenRecoveryUI::draw_progress_locked() {
-{
     if (currentIcon == ERROR) return;
 
     if (currentIcon == INSTALLING_UPDATE || currentIcon == ERASING) {
@@ -204,8 +202,7 @@ void ScreenRecoveryUI::SetColor(UIElement e) {
 
 // Redraw everything on the screen.  Does not flip pages.
 // Should only be called with updateMutex locked.
-void ScreenRecoveryUI::draw_screen_locked()
+void ScreenRecoveryUI::draw_screen_locked() {
-{
     if (!show_text) {
         draw_background_locked(currentIcon);
         draw_progress_locked();
@@ -260,16 +257,14 @@ void ScreenRecoveryUI::draw_screen_locked()
 
 // Redraw everything on the screen and flip the screen (make it visible).
 // Should only be called with updateMutex locked.
-void ScreenRecoveryUI::update_screen_locked()
+void ScreenRecoveryUI::update_screen_locked() {
-{
     draw_screen_locked();
     gr_flip();
 }
 
 // Updates only the progress bar, if possible, otherwise redraws the screen.
 // Should only be called with updateMutex locked.
-void ScreenRecoveryUI::update_progress_locked()
+void ScreenRecoveryUI::update_progress_locked() {
-{
     if (show_text || !pagesIdentical) {
         draw_screen_locked();    // Must redraw the whole screen
         pagesIdentical = true;
@@ -354,8 +349,7 @@ static char** Alloc2d(size_t rows, size_t cols) {
     return result;
 }
 
-void ScreenRecoveryUI::Init()
+void ScreenRecoveryUI::Init() {
-{
     gr_init();
 
     gr_font_size(&char_width, &char_height);
@@ -416,8 +410,7 @@ void ScreenRecoveryUI::SetLocale(const char* new_locale) {
     }
 }
 
-void ScreenRecoveryUI::SetBackground(Icon icon)
+void ScreenRecoveryUI::SetBackground(Icon icon) {
-{
     pthread_mutex_lock(&updateMutex);
 
     currentIcon = icon;
@@ -426,8 +419,7 @@ void ScreenRecoveryUI::SetBackground(Icon icon)
     pthread_mutex_unlock(&updateMutex);
 }
 
-void ScreenRecoveryUI::SetProgressType(ProgressType type)
+void ScreenRecoveryUI::SetProgressType(ProgressType type) {
-{
     pthread_mutex_lock(&updateMutex);
     if (progressBarType != type) {
         progressBarType = type;
@@ -439,8 +431,7 @@ void ScreenRecoveryUI::SetProgressType(ProgressType type)
     pthread_mutex_unlock(&updateMutex);
 }
 
-void ScreenRecoveryUI::ShowProgress(float portion, float seconds)
+void ScreenRecoveryUI::ShowProgress(float portion, float seconds) {
-{
     pthread_mutex_lock(&updateMutex);
     progressBarType = DETERMINATE;
     progressScopeStart += progressScopeSize;
@@ -452,8 +443,7 @@ void ScreenRecoveryUI::ShowProgress(float portion, float seconds)
     pthread_mutex_unlock(&updateMutex);
 }
 
-void ScreenRecoveryUI::SetProgress(float fraction)
+void ScreenRecoveryUI::SetProgress(float fraction) {
-{
     pthread_mutex_lock(&updateMutex);
     if (fraction < 0.0) fraction = 0.0;
     if (fraction > 1.0) fraction = 1.0;
@@ -476,8 +466,7 @@ void ScreenRecoveryUI::SetStage(int current, int max) {
     pthread_mutex_unlock(&updateMutex);
 }
 
-void ScreenRecoveryUI::Print(const char *fmt, ...)
+void ScreenRecoveryUI::Print(const char *fmt, ...) {
-{
     char buf[256];
     va_list ap;
     va_start(ap, fmt);
@@ -486,10 +475,9 @@ void ScreenRecoveryUI::Print(const char *fmt, ...)
 
     fputs(buf, stdout);
 
-    // This can get called before ui_init(), so be careful.
     pthread_mutex_lock(&updateMutex);
     if (text_rows > 0 && text_cols > 0) {
-        for (char* ptr = buf; *ptr != '\0'; ++ptr) {
+        for (const char* ptr = buf; *ptr != '\0'; ++ptr) {
             if (*ptr == '\n' || text_col >= text_cols) {
                 text[text_row][text_col] = '\0';
                 text_col = 0;
@@ -504,6 +492,75 @@ void ScreenRecoveryUI::Print(const char *fmt, ...)
     pthread_mutex_unlock(&updateMutex);
 }
 
+// TODO: replace this with something not line-based so we can wrap correctly without getting
+// confused about what line we're on.
+void ScreenRecoveryUI::print_no_update(const char* s) {
+    pthread_mutex_lock(&updateMutex);
+    if (text_rows > 0 && text_cols > 0) {
+        for (const char* ptr = s; *ptr != '\0'; ++ptr) {
+            if (*ptr == '\n' || text_col >= text_cols) {
+                text[text_row][text_col] = '\0';
+                text_col = 0;
+                text_row = (text_row + 1) % text_rows;
+                if (text_row == text_top) text_top = (text_top + 1) % text_rows;
+            }
+            if (*ptr != '\n') text[text_row][text_col++] = *ptr;
+        }
+        text[text_row][text_col] = '\0';
+    }
+    pthread_mutex_unlock(&updateMutex);
+}
+
+void ScreenRecoveryUI::ShowFile(const char* filename) {
+    FILE* fp = fopen_path(filename, "re");
+    if (fp == nullptr) {
+        Print("  Unable to open %s: %s\n", filename, strerror(errno));
+        return;
+    }
+
+    char line[1024];
+    int ct = 0;
+    int key = 0;
+    while (fgets(line, sizeof(line), fp) != nullptr) {
+        print_no_update(line);
+        ct++;
+        if (ct % text_rows == 0) {
+            Redraw();
+
+            // give the user time to glance at the entries
+            key = WaitKey();
+
+            if (key == KEY_POWER) {
+                break;
+            } else if (key == KEY_VOLUMEUP) {
+                // Go back by seeking to the beginning and dumping ct - n
+                // lines.  It's ugly, but this way we don't need to store
+                // the previous offsets.  The files we're dumping here aren't
+                // expected to be very large.
+                ct -= 2 * text_rows;
+                if (ct < 0) {
+                    ct = 0;
+                }
+                fseek(fp, 0, SEEK_SET);
+                for (int i = 0; i < ct; i++) {
+                    fgets(line, sizeof(line), fp);
+                }
+                Print("^^^^^^^^^^\n");
+            } else {
+                // Next page.
+            }
+        }
+    }
+
+    // If the user didn't abort, then give the user time to glance at
+    // the end of the log, sorry, no rewind here
+    if (key != KEY_POWER) {
+        Print("\n--END-- (press any key)\n");
+        WaitKey();
+    }
+    fclose(fp);
+}
+
 void ScreenRecoveryUI::StartMenu(const char* const * headers, const char* const * items,
                                  int initial_selection) {
     pthread_mutex_lock(&updateMutex);
@@ -554,33 +611,29 @@ void ScreenRecoveryUI::EndMenu() {
     pthread_mutex_unlock(&updateMutex);
 }
 
-bool ScreenRecoveryUI::IsTextVisible()
+bool ScreenRecoveryUI::IsTextVisible() {
-{
     pthread_mutex_lock(&updateMutex);
     int visible = show_text;
     pthread_mutex_unlock(&updateMutex);
     return visible;
 }
 
-bool ScreenRecoveryUI::WasTextEverVisible()
+bool ScreenRecoveryUI::WasTextEverVisible() {
-{
     pthread_mutex_lock(&updateMutex);
     int ever_visible = show_text_ever;
     pthread_mutex_unlock(&updateMutex);
     return ever_visible;
 }
 
-void ScreenRecoveryUI::ShowText(bool visible)
+void ScreenRecoveryUI::ShowText(bool visible) {
-{
     pthread_mutex_lock(&updateMutex);
     show_text = visible;
-    if (show_text) show_text_ever = 1;
+    if (show_text) show_text_ever = true;
     update_screen_locked();
     pthread_mutex_unlock(&updateMutex);
 }
 
-void ScreenRecoveryUI::Redraw()
+void ScreenRecoveryUI::Redraw() {
-{
     pthread_mutex_lock(&updateMutex);
     update_screen_locked();
     pthread_mutex_unlock(&updateMutex);

screen_ui.h

@@ -48,6 +48,7 @@ class ScreenRecoveryUI : public RecoveryUI {
 
     // printing messages
     void Print(const char* fmt, ...) __printflike(2, 3);
+    void ShowFile(const char* filename);
 
     // menu display
     void StartMenu(const char* const * headers, const char* const * items,
@@ -80,11 +81,10 @@ class ScreenRecoveryUI : public RecoveryUI {
     float progressScopeStart, progressScopeSize, progress;
     double progressScopeTime, progressScopeDuration;
 
-    // true when both graphics pages are the same (except for the
+    // true when both graphics pages are the same (except for the progress bar).
-    // progress bar)
     bool pagesIdentical;
 
-    // Log text overlay, displayed when a magic key is pressed
+    // Log text overlay, displayed when a magic key is pressed.
     char** text;
     size_t text_cols, text_rows;
     size_t text_col, text_row, text_top;
@@ -112,6 +112,8 @@ class ScreenRecoveryUI : public RecoveryUI {
     static void* progress_thread(void* cookie);
     void progress_loop();
 
+    void print_no_update(const char*);
+
     void LoadBitmap(const char* filename, gr_surface* surface);
     void LoadBitmapArray(const char* filename, int* frames, gr_surface** surface);
     void LoadLocalizedBitmap(const char* filename, gr_surface* surface);

ui.h

@@ -31,10 +31,10 @@ class RecoveryUI {
     // Initialize the object; called before anything else.
     virtual void Init();
     // Show a stage indicator.  Call immediately after Init().
-    virtual void SetStage(int current, int max) { }
+    virtual void SetStage(int current, int max) = 0;
 
     // After calling Init(), you can tell the UI what locale it is operating in.
-    virtual void SetLocale(const char* locale) { }
+    virtual void SetLocale(const char* locale) = 0;
 
     // Set the overall recovery state ("background image").
     enum Icon { NONE, INSTALLING_UPDATE, ERASING, NO_COMMAND, ERROR };
@@ -65,6 +65,8 @@ class RecoveryUI {
     // toggled on the text display).
     virtual void Print(const char* fmt, ...) __printflike(2, 3) = 0;
 
+    virtual void ShowFile(const char* filename) = 0;
+
     // --- key handling ---
 
     // Wait for keypress and return it.  May return -1 after timeout.

verifier_test.cpp

@@ -124,6 +124,8 @@ RecoveryUI* ui = NULL;
 // nothing but print.
 class FakeUI : public RecoveryUI {
     void Init() { }
+    void SetStage(int, int) { }
+    void SetLocale(const char*) { }
     void SetBackground(Icon icon) { }
 
     void SetProgressType(ProgressType determinate) { }
@@ -139,6 +141,7 @@ class FakeUI : public RecoveryUI {
         vfprintf(stderr, fmt, ap);
         va_end(ap);
     }
+    void ShowFile(const char*) { }
 
     void StartMenu(const char* const * headers, const char* const * items,
                            int initial_selection) { }