[vlc-commits] Revert "macosx: Rework log message storage and fix filtering"

[Marvin Scholz]

vlc | branch: master | Marvin Scholz <epirat07 at gmail.com> | Thu Sep  7 23:18:52 2017 +0200| [344724d7e5da22d4162fda001726528b308c5d02] | committer: Marvin Scholz

Revert "macosx: Rework log message storage and fix filtering"

This reverts commit 660257430f41f106ecb1834af7ccaf2a37a281de.

This turns out to be flawed, as modifying the backing array of the
NSArrayController is apparantly not ok, and leads to crashes in some
cases.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=344724d7e5da22d4162fda001726528b308c5d02
---

 modules/gui/macosx/UI/LogMessageWindow.xib  |  2 +-
 modules/gui/macosx/VLCLogWindowController.m | 86 +++++++++++++++--------------
 2 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/modules/gui/macosx/UI/LogMessageWindow.xib b/modules/gui/macosx/UI/LogMessageWindow.xib
index e8f3b4422c..79d1c01763 100644
--- a/modules/gui/macosx/UI/LogMessageWindow.xib
+++ b/modules/gui/macosx/UI/LogMessageWindow.xib
@@ -442,7 +442,7 @@ IA
             </toolbar>
             <point key="canvasLocation" x="139" y="192"/>
         </window>
-        <arrayController editable="NO" selectsInsertedObjects="NO" clearsFilterPredicateOnInsertion="NO" id="0Or-bh-QJU">
+        <arrayController editable="NO" selectsInsertedObjects="NO" id="0Or-bh-QJU">
             <declaredKeys>
                 <string>component</string>
                 <string>message</string>
diff --git a/modules/gui/macosx/VLCLogWindowController.m b/modules/gui/macosx/VLCLogWindowController.m
index a09839031d..22d5c54162 100644
--- a/modules/gui/macosx/VLCLogWindowController.m
+++ b/modules/gui/macosx/VLCLogWindowController.m
@@ -32,19 +32,16 @@
 /* This array stores messages that are managed by the arrayController */
 @property (retain) NSMutableArray *messagesArray;
 
+/* This array stores messages before they are added to the messagesArray on refresh */
+ at property (retain) NSMutableArray *messageBuffer;
+
 /* We do not want to refresh the table for every message, as that would be very frequent when
- * there are a lot of messages, therefore we use a timer to refresh the table every now and
- * then, which is much more efficient and still fast enough for a good user experience.
+ * there are a lot of messages, therefore we use a timer to refresh the table with new data
+ * from the messageBuffer every now and then, which is much more efficient and still fast
+ * enough for a good user experience
  */
 @property (retain) NSTimer        *refreshTimer;
 
-/*
- * Indicates if an update is needed, which is the case when new messages were added
- * after the last firing of the update timer. It is used to prevent unnecessary
- * rearranging of the NSArrayController content.
- */
- at property (atomic) BOOL           needsUpdate;
-
 - (void)addMessage:(NSDictionary *)message;
 
 @end
@@ -90,7 +87,8 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
 {
     self = [super initWithWindowNibName:@"LogMessageWindow"];
     if (self) {
-        _messagesArray = [[NSMutableArray alloc] initWithCapacity:1000];
+        _messagesArray = [[NSMutableArray alloc] initWithCapacity:500];
+        _messageBuffer = [[NSMutableArray alloc] initWithCapacity:100];
     }
     return self;
 }
@@ -142,7 +140,7 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
     vlc_LogSet(getIntf()->obj.libvlc, MsgCallback, (__bridge void*)self);
     _refreshTimer = [NSTimer scheduledTimerWithTimeInterval:0.3
                                                      target:self
-                                                   selector:@selector(updateArrayController:)
+                                                   selector:@selector(appendMessageBuffer)
                                                    userInfo:nil
                                                     repeats:YES];
     return [super showWindow:sender];
@@ -154,24 +152,14 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
     vlc_LogSet( getIntf()->obj.libvlc, NULL, NULL );
 
     // Remove all messages
-    [self removeAllMessages];
+    [self clearMessageBuffer];
+    [self clearMessageTable];
 
     // Invalidate timer
     [_refreshTimer invalidate];
     _refreshTimer = nil;
 }
 
-/**
- Called by the timer to re-sync the array controller with the backing array
- */
-- (void)updateArrayController:(NSTimer *)timer
-{
-    if (_needsUpdate)
-        [_arrayController rearrangeObjects];
-
-    _needsUpdate = NO;
-}
-
 #pragma mark -
 #pragma mark Delegate methods
 
@@ -248,7 +236,8 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
     vlc_LogSet(getIntf()->obj.libvlc, NULL, NULL);
 
     // Remove all messages
-    [self removeAllMessages];
+    [self clearMessageBuffer];
+    [self clearMessageTable];
 
     // Reregister handler, to write new header to log
     vlc_LogSet(getIntf()->obj.libvlc, MsgCallback, (__bridge void*)self);
@@ -258,7 +247,7 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
  */
 - (IBAction)refreshLog:(id)sender
 {
-    [_arrayController rearrangeObjects];
+    [self appendMessageBuffer];
     [_messageTable scrollToEndOfDocument:self];
 }
 
@@ -275,7 +264,7 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
 
 /* Called when the user hits CMD + C or copy is clicked in the edit menu
  */
-- (void)copy:(id)sender {
+- (void) copy:(id)sender {
     NSPasteboard *pasteBoard = [NSPasteboard generalPasteboard];
     [pasteBoard clearContents];
     for (NSDictionary *line in [_arrayController selectedObjects]) {
@@ -312,30 +301,47 @@ static void MsgCallback(void *data, int type, const vlc_log_t *item, const char
 #pragma mark Data handling
 
 /** 
- Adds a message, it is only added visibly to the table on the next firing
- of the update timer.
+ Adds a message to the messageBuffer, it does not has to be called from the main thread, as
+ items are only added to the messageArray on refresh.
  */
 - (void)addMessage:(NSDictionary *)messageDict
 {
-    @synchronized (_messagesArray) {
-        if ([_messagesArray count] > 1000000) {
-            [_messagesArray removeObjectsInRange:NSMakeRange(0, 2)];
-        }
-        [_messagesArray addObject:messageDict];
+    @synchronized (_messageBuffer) {
+        [_messageBuffer addObject:messageDict];
+    }
+}
 
-        _needsUpdate = YES;
+/**
+ Clears the message buffer
+ */
+- (void)clearMessageBuffer
+{
+    @synchronized (_messageBuffer) {
+        [_messageBuffer removeAllObjects];
     }
-    
 }
 
 /**
- Clears all messages in the message table by removing all items from the array and
- calling `rearrangeObjects` on the array controller.
+ Clears all messages in the message table by removing all items from the arrayController
  */
-- (void)removeAllMessages
+- (void)clearMessageTable
 {
-    [_messagesArray removeAllObjects];
-    [_arrayController rearrangeObjects];
+    NSRange range = NSMakeRange(0, [[_arrayController arrangedObjects] count]);
+    [_arrayController removeObjectsAtArrangedObjectIndexes:[NSIndexSet indexSetWithIndexesInRange:range]];
+}
+
+/**
+ Appends all messages from the buffer to the arrayController and clears the buffer
+ */
+- (void)appendMessageBuffer
+{
+    if ([_messagesArray count] > 1000000) {
+        [_messagesArray removeObjectsInRange:NSMakeRange(0, 2)];
+    }
+    @synchronized (_messageBuffer) {
+        [_arrayController addObjects:_messageBuffer];
+        [_messageBuffer removeAllObjects];
+    }
 }
 
 @end