> On June 21, 2013, 2:33 a.m., Andrew Grieve wrote: > > framework/src/org/apache/cordova/api/PluginManager.java, line 226 > > <https://reviews.apache.org/r/12013/diff/1/?file=309778#file309778line226> > > > > These params don't need to be final.
Isn't it better to make them final unless there is a need to modify them? > On June 21, 2013, 2:33 a.m., Andrew Grieve wrote: > > framework/src/org/apache/cordova/api/PluginManager.java, line 434 > > <https://reviews.apache.org/r/12013/diff/1/?file=309778#file309778line434> > > > > I realized tonight that a boolean here isn't enough. > > Consider this case: > > > > 1. set runExecOnUiThread = true > > 2. runExecOnUiThread=false Runnable queued. > > 3. Exec call "foo" gets made, Runnable queued. > > 4. runExecOnUiThread=false runs > > 5. Exec call "bar" gets made, executes right away > > 6. Exec call "foo" Runnable is run > > > > We don't want to ever execute calls out-of-order. "bar" shouldn't get > > run before "foo". > > > > I think to fix this, we can use an AtomicInteger. Increment it every > > time an exec is queued, decrement it each time an exec finishes. Turn off > > runExecOnUiThread only once the count reaches 0. > > > > make sense? > > > > If an app is making execs too fast couldn't it cause every exec to be run the UI thread and never switch to using the WebCore thread? Is it possible to leave runExecOnUiThread getting set like it currently is, use your AtomicInteger idea to track the number of pending execs on the UI thread and then make WebCore execs block until this value is 0. We would have to make sure that if multiple WebCore execs get blocked, when they get unblocked they get execute in the same order that they came in. - Jeffrey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12013/#review22229 ----------------------------------------------------------- On June 20, 2013, 8:16 p.m., Jeffrey Willms wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12013/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 8:16 p.m.) > > > Review request for cordova and Andrew Grieve. > > > Description > ------- > > Fixing exec bug. > > > This addresses bug CB-3927. > https://issues.apache.org/jira/browse/CB-3927 > > > Diffs > ----- > > framework/src/org/apache/cordova/api/PluginEntry.java > 9b9af6bc303965e7263bca75037256da81868fb2 > framework/src/org/apache/cordova/api/PluginManager.java > 71fc25817b5d58bb2fbf5a29158ef2c7d424d4ab > > Diff: https://reviews.apache.org/r/12013/diff/ > > > Testing > ------- > > > Thanks, > > Jeffrey Willms > >
