You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ag...@apache.org on 2013/12/20 05:12:11 UTC

ios commit: CB-5134 Fix up bugs with new hash-based exec() bridge.

Updated Branches:
  refs/heads/master 0f28be660 -> d6fd0afdc


CB-5134 Fix up bugs with new hash-based exec() bridge.


Project: http://git-wip-us.apache.org/repos/asf/cordova-ios/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-ios/commit/d6fd0afd
Tree: http://git-wip-us.apache.org/repos/asf/cordova-ios/tree/d6fd0afd
Diff: http://git-wip-us.apache.org/repos/asf/cordova-ios/diff/d6fd0afd

Branch: refs/heads/master
Commit: d6fd0afdc430db947e257c0e80a8fcae2bee55bd
Parents: 0f28be6
Author: Andrew Grieve <ag...@chromium.org>
Authored: Thu Dec 19 23:09:01 2013 -0500
Committer: Andrew Grieve <ag...@chromium.org>
Committed: Thu Dec 19 23:11:44 2013 -0500

----------------------------------------------------------------------
 CordovaLib/Classes/CDVCommandDelegateImpl.h |  2 ++
 CordovaLib/Classes/CDVCommandDelegateImpl.m | 24 +++++++++++++++++++-----
 CordovaLib/Classes/CDVCommandQueue.h        |  2 +-
 CordovaLib/Classes/CDVCommandQueue.m        |  6 +++---
 CordovaLib/Classes/CDVURLProtocol.m         |  3 ++-
 CordovaLib/Classes/CDVViewController.m      | 20 ++++++++++++++++++--
 6 files changed, 45 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVCommandDelegateImpl.h
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVCommandDelegateImpl.h b/CordovaLib/Classes/CDVCommandDelegateImpl.h
index 6735136..d35b32d 100644
--- a/CordovaLib/Classes/CDVCommandDelegateImpl.h
+++ b/CordovaLib/Classes/CDVCommandDelegateImpl.h
@@ -28,6 +28,8 @@
     __weak CDVViewController* _viewController;
     @protected
     __weak CDVCommandQueue* _commandQueue;
+    BOOL _delayResponses;
 }
 - (id)initWithViewController:(CDVViewController*)viewController;
+- (void)flushCommandQueueWithDelayedJs;
 @end

http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVCommandDelegateImpl.m
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVCommandDelegateImpl.m b/CordovaLib/Classes/CDVCommandDelegateImpl.m
index 1a6b6ed..5bb56b0 100644
--- a/CordovaLib/Classes/CDVCommandDelegateImpl.m
+++ b/CordovaLib/Classes/CDVCommandDelegateImpl.m
@@ -53,6 +53,13 @@
     return [mainBundle pathForResource:filename ofType:@"" inDirectory:directoryStr];
 }
 
+- (void)flushCommandQueueWithDelayedJs
+{
+    _delayResponses = YES;
+    [_commandQueue executePending];
+    _delayResponses = NO;
+}
+
 - (void)evalJsHelper2:(NSString*)js
 {
     CDV_EXEC_LOG(@"Exec: evalling: %@", [js substringToIndex:MIN([js length], 160)]);
@@ -62,18 +69,25 @@
     }
 
     [_commandQueue enqueueCommandBatch:commandsJSON];
+    [_commandQueue executePending];
 }
 
 - (void)evalJsHelper:(NSString*)js
 {
     // Cycle the run-loop before executing the JS.
-    // This works around a bug where sometimes alerts() within callbacks can cause
-    // dead-lock.
-    // If the commandQueue is currently executing, then we know that it is safe to
-    // execute the callback immediately.
+    // For _delayResponses -
+    //    This ensures that we don't eval JS during the middle of an existing JS
+    //    function (possible since UIWebViewDelegate callbacks can be synchronous).
+    // For !isMainThread -
+    //    It's a hard error to eval on the non-UI thread.
+    // For !_commandQueue.currentlyExecuting -
+    //     This works around a bug where sometimes alerts() within callbacks can cause
+    //     dead-lock.
+    //     If the commandQueue is currently executing, then we know that it is safe to
+    //     execute the callback immediately.
     // Using    (dispatch_get_main_queue()) does *not* fix deadlocks for some reason,
     // but performSelectorOnMainThread: does.
-    if (![NSThread isMainThread] || !_commandQueue.currentlyExecuting) {
+    if (_delayResponses || ![NSThread isMainThread] || !_commandQueue.currentlyExecuting) {
         [self performSelectorOnMainThread:@selector(evalJsHelper2:) withObject:js waitUntilDone:NO];
     } else {
         [self evalJsHelper2:js];

http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVCommandQueue.h
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVCommandQueue.h b/CordovaLib/Classes/CDVCommandQueue.h
index 48eea0f..3329078 100644
--- a/CordovaLib/Classes/CDVCommandQueue.h
+++ b/CordovaLib/Classes/CDVCommandQueue.h
@@ -32,7 +32,7 @@
 - (void)resetRequestId;
 - (void)enqueueCommandBatch:(NSString*)batchJSON;
 
-- (void)maybeFetchCommandsFromJs:(NSNumber*)requestId;
+- (void)processXhrExecBridgePoke:(NSNumber*)requestId;
 - (void)fetchCommandsFromJs;
 - (void)executePending;
 - (BOOL)execute:(CDVInvokedUrlCommand*)command;

http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVCommandQueue.m
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVCommandQueue.m b/CordovaLib/Classes/CDVCommandQueue.m
index 1e25ecc..1491073 100644
--- a/CordovaLib/Classes/CDVCommandQueue.m
+++ b/CordovaLib/Classes/CDVCommandQueue.m
@@ -58,14 +58,12 @@
 
 - (void)enqueueCommandBatch:(NSString*)batchJSON
 {
-    CDV_EXEC_LOG(@"Exec: Flushed JS->native queue (hadCommands=%d).", [batchJSON length] > 0);
     if ([batchJSON length] > 0) {
         [_queue addObject:batchJSON];
-        [self executePending];
     }
 }
 
-- (void)maybeFetchCommandsFromJs:(NSNumber*)requestId
+- (void)processXhrExecBridgePoke:(NSNumber*)requestId
 {
     NSInteger rid = [requestId integerValue];
 
@@ -85,6 +83,7 @@
     if (rid > _lastCommandQueueFlushRequestId) {
         _lastCommandQueueFlushRequestId = [requestId integerValue];
         [self fetchCommandsFromJs];
+        [self executePending];
     }
 }
 
@@ -94,6 +93,7 @@
     NSString* queuedCommandsJSON = [_viewController.webView stringByEvaluatingJavaScriptFromString:
         @"cordova.require('cordova/exec').nativeFetchMessages()"];
 
+    CDV_EXEC_LOG(@"Exec: Flushed JS->native queue (hadCommands=%d).", [queuedCommandsJSON length] > 0);
     [self enqueueCommandBatch:queuedCommandsJSON];
 }
 

http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVURLProtocol.m
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVURLProtocol.m b/CordovaLib/Classes/CDVURLProtocol.m
index 03334ea..eeee916 100644
--- a/CordovaLib/Classes/CDVURLProtocol.m
+++ b/CordovaLib/Classes/CDVURLProtocol.m
@@ -124,8 +124,9 @@ static CDVViewController *viewControllerForRequest(NSURLRequest* request)
             if (hasCmds) {
                 SEL sel = @selector(enqueueCommandBatch:);
                 [viewController.commandQueue performSelectorOnMainThread:sel withObject:queuedCommandsJSON waitUntilDone:NO];
+                [viewController.commandQueue performSelectorOnMainThread:@selector(executePending) withObject:nil waitUntilDone:NO];
             } else {
-                SEL sel = @selector(maybeFetchCommandsFromJs:);
+                SEL sel = @selector(processXhrExecBridgePoke:);
                 [viewController.commandQueue performSelectorOnMainThread:sel withObject:[NSNumber numberWithInteger:[requestId integerValue]] waitUntilDone:NO];
             }
             // Returning NO here would be 20% faster, but it spams WebInspector's console with failure messages.

http://git-wip-us.apache.org/repos/asf/cordova-ios/blob/d6fd0afd/CordovaLib/Classes/CDVViewController.m
----------------------------------------------------------------------
diff --git a/CordovaLib/Classes/CDVViewController.m b/CordovaLib/Classes/CDVViewController.m
index ab86dfe..7a6c951 100644
--- a/CordovaLib/Classes/CDVViewController.m
+++ b/CordovaLib/Classes/CDVViewController.m
@@ -655,17 +655,33 @@
      */
     if ([[url scheme] isEqualToString:@"gap"]) {
         [_commandQueue fetchCommandsFromJs];
+        // The delegate is called asynchronously in this case, so we don't have to use
+        // flushCommandQueueWithDelayedJs (setTimeout(0)) as we do with hash changes.
+        [_commandQueue executePending];
         return NO;
     }
 
-    if ([[url fragment] hasPrefix:@"%01"]) {
+    if ([[url fragment] hasPrefix:@"%01"] || [[url fragment] hasPrefix:@"%02"]) {
+        // Delegate is called *immediately* for hash changes. This means that any
+        // calls to stringByEvaluatingJavascriptFromString will occur in the middle
+        // of an existing (paused) call stack. This doesn't cause errors, but may
+        // be unexpected to callers (exec callbacks will be called before exec() even
+        // returns). To avoid this, we do not do any synchronous JS evals by using
+        // flushCommandQueueWithDelayedJs.
         NSString* inlineCommands = [[url fragment] substringFromIndex:3];
         if ([inlineCommands length] == 0) {
+            // Reach in right away since the WebCore / Main thread are already synchronized.
             [_commandQueue fetchCommandsFromJs];
         } else {
-            inlineCommands = [inlineCommands stringByRemovingPercentEncoding];
+            inlineCommands = [inlineCommands stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
             [_commandQueue enqueueCommandBatch:inlineCommands];
         }
+        // Switch these for minor performance improvements, and to really live on the wild side.
+        // Callbacks will occur in the middle of the location.hash = ... statement!
+        [(CDVCommandDelegateImpl*)_commandDelegate flushCommandQueueWithDelayedJs];
+        // [_commandQueue executePending];
+
+        // Although we return NO, the hash change does end up taking effect.
         return NO;
     }