date:20110504

This is probably html unit honoring the locale and changing its behavior
from what the tests expect.
On May 4, 2011 2:02 AM, t.bro...@gmail.com wrote:
 On 2011/05/03 17:51:18, rjrjr wrote:
 Running ant clean dist-dev test, this appears to break the i18n suite
 under
 html unit.

 Oops! Only tested with a -Dgwt.junit.testcase.includes (which obviously
 didn't include I18N tests)

 That seems weird though that simply running the JVM in a different
 locale suddenly breaks tests of things that are emulated.

 Testcase: testMessageDateTime took 0.15 sec
 FAILED
 Remote test failed at 172.31.131.172 / Mozilla/5.0 (Windows; U;
 Windows NT
 5.1; en-US; rv:1.9.0.19) Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15
 junit.framework.AssertionFailedError: Remote test failed at
 172.31.131.172 /
 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.19)
 Gecko/2010031422 Firefox/3.0.19
 expected=It is Feb 15, 2010 actual=It is 2010 Feb 15

 Actually, 3 tests always fail for me, whichever locale I'm running them
 in (including using es_MX, or removing the -Duser.language/user.region
 system props altogether!)
 And they always fail with the exact same error; so it looks more like a
 regression in the I18N code (particularly as all other I18N tests pass).

 test.dev.htmlunit:
 [echo] Writing test results to C:\Documents and Settings\tbr\Mes
 documents\Mes Projets\gwt\trunk\build\out\user\test/dev-htmlunit/reports
 for test.dev.htmlunit.tests
 [junit] WARNING: multiple versions of ant detected in path for junit

 [junit]

jar:file:/C:/eclipse/plugins/org.apache.ant_1.7.1.v20100518-1145/lib/ant.jar!/org/apache/tools/ant/Project.class
 [junit] and

jar:file:/C:/Documents%20and%20Settings/tbr/Mes%20documents/Mes%20Projets/gwt/trunk/build/lib/gwt-dev.jar!/org/apache/tools/ant/Project.class
 [junit] Running com.google.gwt.i18n.I18NSuite
 [junit] expected=It is Feb 15, 2010 actual=It is 2010 Feb 15)
 [junit] expected=Es ist 15. Feb 2010 actual=It is 2010 Feb 15)
 [junit] expected=Short: 2010-02-01 actual=Short: 2010-02-02)


 http://gwt-code-reviews.appspot.com/1430801/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread rchandia


LGTM

http://gwt-code-reviews.appspot.com/1428810/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: 1. Assert, at runtime, that GWT is running in Standards Mode (i.e. with an appropriate DOCTYPE d... (issue1422816)


This is a breaking change for existing apps that run in quirks mode.
I'm alright with that because I don't see an alternative, but we'll need
to call if out in the release notes and tell people the workaround.


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml
File user/src/com/google/gwt/user/UserAgent.gwt.xml (right):

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode29
user/src/com/google/gwt/user/UserAgent.gwt.xml:29:
extra newline

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode56
user/src/com/google/gwt/user/UserAgent.gwt.xml:56:
define-configuration-property name=document.compatMode
I think we should break this out into a separate DocumentMode.gwt.xml
file and inherit it in User.gwt.xml.  It isn't part of the UserAgent.

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode67
user/src/com/google/gwt/user/UserAgent.gwt.xml:67:
extra newline

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java
File user/src/com/google/gwt/user/client/DocumentModeAsserter.java
(right):

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java#newcode27
user/src/com/google/gwt/user/client/DocumentModeAsserter.java:27: *
rendering mode is of of the values allowed by the
/r/of of/one of

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
File user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
(right):

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode49
user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:49:
logger.log(TreeLogger.ERROR, OOPS, e);
Maybe something better than OOPS

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode57
user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:57:
JClassType remoteService = typeOracle.findType(typeName);
Isn't removeService the same as userType?

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode76
user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:76:
logger.log(TreeLogger.WARN, Unable to find value for '
If we are going to throw an exception, this should be an ERROR instead
of a WARN

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode102
user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:102: for
(IteratorString iterator = documentModes.iterator();
iterator.hasNext();) {
You can shorten this to:
for (String next : documentModes) {

http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode103
user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:103:
sw.println(\ + iterator.next() + \, );
This array will always end with a comma. Does Java handle that
correctly?
return new String[]{a,b,};

http://gwt-code-reviews.appspot.com/1422816/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)


I have a few questions about appcache and linkers, but it looks pretty
good.


http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode49
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:49: *
Add p before each paragraph

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode55
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:55: *
and overrides {@code otherCAchedFiles()}, and use it as a linker
instead:
/r/otherCAchedFiles/otherCachedFiles

You might mention that wildcards, such as /audio/*, will work (I think
thats true).

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode68
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:68:
public class SimpleAppCacheLinker extends AbstractLinker {
I didn't see a rule in a gwt.xml file.  Do linkers run automatically?

If so, can this linker be disabled? I'm not sure everyone will want to
use appcache.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode102
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:102:
protected String[] otherCachedFiles() {
This is fine for now, but it would be nice if users could specify other
cached files in a gwt.xml or appcache.manifest file instead of
overriding this class.

http://gwt-code-reviews.appspot.com/1428811/diff/1/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java
File
samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java#newcode37
samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java:37:
/audio/error.wav
also add:
video/tutorial.mp4
video/tutorial.ogv

http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)

2011-05-04 Thread pdr


You'll need to add this to the mobile web app .gwt.xml:
add-linker name=simpleappcachelinker /


http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode17
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:17:
This linker should be defined in a gwt.xml file somewhere with this
line:
define-linker name=simpleappcachelinker
class=com.google.gwt.core.linker.SimpleAppCacheLinker /

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode55
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:55: *
and overrides {@code otherCAchedFiles()}, and use it as a linker
instead:
otherCAchedFiles - otherCachedFiles

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode166
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:166:
// build cache list
This code is essentially a duplicate of the above code. Break it out
into a method?

http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] [google-web-toolkit] r10134 committed - Including the TaskProxy (when available) in TaskEditPlace so we do not...


Revision: 10134
Author:   jlaba...@google.com
Date: Wed May  4 05:01:35 2011
Log:  Including the TaskProxy (when available) in TaskEditPlace so we  
do not do an extra round trip to the server to lookup the task.


Also fixing a bug where the Task List menu item isn't selected when the  
task list is visible because TaskListPlace is no longer a singleton.  We  
now do an instanceof check instead of an == check to compare the  
TaskListPlace in the menu item to the TaskListPlace from the place change  
event.


Review at http://gwt-code-reviews.appspot.com/1428810

Review by: rchan...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10134

Modified:
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskListActivity.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/place/TaskEditPlace.java


===
---  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java	 
Tue May  3 10:43:13 2011
+++  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java	 
Wed May  4 05:01:35 2011

@@ -78,6 +78,7 @@
*/
   public TaskEditActivity(TaskEditPlace place, ClientFactory  
clientFactory) {

 this.taskId = place.getTaskId();
+this.task = place.getTask();
 this.clientFactory = clientFactory;
   }

@@ -163,40 +164,45 @@
   // Lock the display until the task is loaded.
   isEditing = true;
   view.setEditing(true);
-  view.setLocked(true);
-
-  // Load the existing task.
-   
clientFactory.getRequestFactory().taskRequest().findTask(this.taskId).fire(

-  new ReceiverTaskProxy() {
-@Override
-public void onFailure(ServerFailure error) {
-  Window.alert(An error occurred on the server while loading  
this task.

-  +  Please select a different task from the task list.);
-  doCancelTask();
-}
-
-@Override
-public void onSuccess(TaskProxy response) {
-  // Early exit if this activity has already been cancelled.
-  if (isDead) {
-return;
-  }
-
-  // Task not found.
-  if (response == null) {
-Window.alert(The task with id ' + taskId + ' could not  
be found.

+
+  if (task == null) {
+// Load the existing task.
+view.setLocked(true);
+ 
clientFactory.getRequestFactory().taskRequest().findTask(this.taskId).fire(

+new ReceiverTaskProxy() {
+  @Override
+  public void onFailure(ServerFailure error) {
+Window.alert(An error occurred on the server while  
loading this task.
 +  Please select a different task from the task  
list.);

 doCancelTask();
-return;
   }

-  // Show the task.
-  task = response;
-  view.getEditorDriver()
-  .edit(response,  
clientFactory.getRequestFactory().taskRequest());

-  view.setLocked(false);
-}
-  });
+  @Override
+  public void onSuccess(TaskProxy response) {
+// Early exit if this activity has already been cancelled.
+if (isDead) {
+  return;
+}
+
+// Task not found.
+if (response == null) {
+  Window.alert(The task with id ' + taskId + ' could  
not be found.
+  +  Please select a different task from the task  
list.);

+  doCancelTask();
+  return;
+}
+
+// Show the task.
+task = response;
+view.getEditorDriver().edit(response,
+clientFactory.getRequestFactory().taskRequest());
+view.setLocked(false);
+  }
+});
+  } else {
+// Use the task that was passed with the place.
+view.getEditorDriver().edit(task,  
clientFactory.getRequestFactory().taskRequest());

+  }
 }

 // Display the view.
===
---  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskListActivity.java	 
Tue May  3 10:43:13 2011
+++  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskListActivity.java	 
Wed May  4 05:01:35 2011

@@ -221,7 +221,8 @@

   public void selectTask(TaskProxy selected) {
 // Go into edit mode when a task is selected.

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)


committed as r10134

http://gwt-code-reviews.appspot.com/1428810/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: 1. Assert, at runtime, that GWT is running in Standards Mode (i.e. with an appropriate DOCTYPE d... (issue1422816)

Please tell me that this is the last step in the following chain. It seems
unlikely if you're only providing the property now.

First you provide the property to allow quirks, and give compiler warnings
that only go away if you set standards mode or set the quirks property. (And
publicize that it will be the default in the next release.) Then you change
the default and break existing apps.

This change should also include updates to the javadoc of widgets that
require quirks to point to the new property.
 On May 4, 2011 7:38 AM, jlaba...@google.com wrote:
 This is a breaking change for existing apps that run in quirks mode.
 I'm alright with that because I don't see an alternative, but we'll need
 to call if out in the release notes and tell people the workaround.



http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml
 File user/src/com/google/gwt/user/UserAgent.gwt.xml (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode29
 user/src/com/google/gwt/user/UserAgent.gwt.xml:29:
 extra newline


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode56
 user/src/com/google/gwt/user/UserAgent.gwt.xml:56:
 define-configuration-property name=document.compatMode
 I think we should break this out into a separate DocumentMode.gwt.xml
 file and inherit it in User.gwt.xml. It isn't part of the UserAgent.


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAgent.gwt.xml#newcode67
 user/src/com/google/gwt/user/UserAgent.gwt.xml:67:
 extra newline


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java
 File user/src/com/google/gwt/user/client/DocumentModeAsserter.java
 (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/client/DocumentModeAsserter.java#newcode27
 user/src/com/google/gwt/user/client/DocumentModeAsserter.java:27: *
 rendering mode is of of the values allowed by the
 /r/of of/one of


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
 File user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
 (right):


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode49
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:49:
 logger.log(TreeLogger.ERROR, OOPS, e);
 Maybe something better than OOPS


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode57
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:57:
 JClassType remoteService = typeOracle.findType(typeName);
 Isn't removeService the same as userType?


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode76
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:76:
 logger.log(TreeLogger.WARN, Unable to find value for '
 If we are going to throw an exception, this should be an ERROR instead
 of a WARN


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode102
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:102: for
 (IteratorString iterator = documentModes.iterator();
 iterator.hasNext();) {
 You can shorten this to:
 for (String next : documentModes) {


http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java#newcode103
 user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java:103:
 sw.println(\ + iterator.next() + \, );
 This array will always end with a comma. Does Java handle that
 correctly?
 return new String[]{a,b,};

 http://gwt-code-reviews.appspot.com/1422816/

 --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Allows enum ordinalization to proceed for enums with static methods/fields (issue1428808)

2011-05-04 Thread scottb


LGTM, but it'd be good if you could add some comments to record some of
the ideas we discussed.


http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
(right):

http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode273
dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:273:
String y = fruit.staticField;);
Thing is, I don't believe the generated code actually checks the NPE.
It only evaluates 'fruit' for side effects, and 'fruit' would eventually
get optimized out with a full optimization.  You can't simply convert to
Fruit because it could be a side-effecty qualifier like
maybeGetFruit().staticField.

http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode456
dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:456:
});
Okay, looking at this formulation, it makes me question that this should
even blacklist Fruit.  As long as CastNormalizer does something smart, I
can't imagine that this instanceof would necessitate blacklisting: if
the test could possibly succeed, it would have implied an upcast
elsewhere.  Maybe CastNormalizer could treat instanceof OrdinalizedEnum
specially and always make the result trivially true or trivially false?

http://gwt-code-reviews.appspot.com/1428808/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)

2011-05-04 Thread tobyr



http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode39
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:39: *
liAdd a mime-mapping to your web.xml file (this is for AppEngine,
adjust
The sample XML below looks standard to me. Why are we mentioning App
Engine?

http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

I mean that you should have been able to fix it by replace place1 == place2
with place1.equals(place2).

On Wed, May 4, 2011 at 8:39 AM, jlaba...@google.com wrote:

 @rjrjr -

 What do you mean?


 http://gwt-code-reviews.appspot.com/1428810/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

Also, this change is a stark reminder that we need to get real about request
caching in RequestFactory. You fundamentally shouldn't be doing this, you
should be able to make the redundant request counting on a lower layer to
send it only if necessary.

Bob, I've lost track -- are there hooks in place yet that we could implement
app specific client side caching in a sample like this?

On Wed, May 4, 2011 at 8:56 AM, Ray Ryan rj...@google.com wrote:

 I mean that you should have been able to fix it by replace place1 == place2
 with place1.equals(place2).


 On Wed, May 4, 2011 at 8:39 AM, jlaba...@google.com wrote:

 @rjrjr -

 What do you mean?


 http://gwt-code-reviews.appspot.com/1428810/




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


Sorry Rafa, forgot to hit send on this yesterday.


http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
(right):

http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode205
user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:205: if
(isFullyInitialized()) {
This seems pretty unexpected. I would have thought it would either be an
error (RuntimeException), or else that I would do the opposite of this:
replace my existing element with the new one.

http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode208
user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:208: //
we already built.
/* for long comments please

http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

On Wed, May 4, 2011 at 9:47 AM, Bob Vawter robertvaw...@google.com wrote:

  Bob, I've lost track -- are there hooks in place yet that we could
 implement
  app specific client side caching in a sample like this?

 You can call RequestFactory.getSerializer() with an implementation of
 a ProxyStore (e.g. DefaultProxyStore) in order to indefinitely persist
 a proxy.  Beyond simple serialization, there's no mechanism in place
 to short-circuit requests.


That's proxies. Can requests be serialized? For that matter, any reason not
to just hold on to them? I'm not talking about LocalStorage or anything
here, just optimizing within a single session.

I know we can extend or replace DefaultRequestTransport on the client side,
e.g. as done in Expenses for GAE
integrationhttps://cs.corp.google.com/#google3/third_party/java_src/gwt/svn/trunk/samples/expenses/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java.
If requests have reasonable equality semantics and are somewhat
introspectable, even just at the instanceof level, we might be able to get
some simple caching done in this sample.

@jlabanca, I know your time is short and you gotta do what you gotta do. I
don't mean to hold up this change for caching. I just want to have the
conversation while we have a use case staring us in the face.



 --
 Bob Vawter
 Google Web Toolkit Team


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)



http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
(right):

http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode205
user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:205: if
(isFullyInitialized()) {
On 2011/05/04 16:47:17, rjrjr wrote:

This seems pretty unexpected. I would have thought it would either be

an error

(RuntimeException), or else that I would do the opposite of this:

replace my

existing element with the new one.


I'd be totally fine with a RuntimeError, it'll probably be m ore
manageable in the long-term. I was trying to support this because even
though it's less efficient, we can still make it work. The scenario here
is:

1-) Build Attachable tree. This includes calling all the render() stuff
and so on. Let's assume that this particular AttachableHTMLPanel is in
the middle of the tree.

2-) For some reason do something to this particular panel (that's in the
middle of the tree) that calls its getElement() [let's say attach it to
the document]. This triggers the process of building the widget tree as
if this panel was the root (i.e., hidden div, set innerHTML, getting
elements for all children and initializing them). At this point
everything works

3-) Now you go and attach the real root of the tree, an ancestor of
this panel. We can do one of 3 things:
  (a) throw an error. You probably didn't mean to do this as it's less
efficient
  (b) replace the element that our parent assigned us (along with its
subtree) with the subtree we already built in step 2
  (c) re-do everything in step 2, ignoring the fact that we already
initialized all children widget

I think (a) would be ideal, specially for debugging purposes. Honestly I
implemented (b) because I was a little afraid and wanted to be as
backwards-compatible as possible.  I don't think (c) would work
out-of-the-box, as we'd have to support initializing Widgets twice
(i.e., re-setting the element, etc.)

Makes sense?

http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

2011-05-04 Thread John LaBanca

On Wed, May 4, 2011 at 12:58 PM, Ray Ryan rj...@google.com wrote:

On Wed, May 4, 2011 at 9:47 AM, Bob Vawter robertvaw...@google.comwrote:

Bob, I've lost track -- are there hooks in place yet that we could
implement
app specific client side caching in a sample like this?

You can call RequestFactory.getSerializer() with an implementation of
a ProxyStore (e.g. DefaultProxyStore) in order to indefinitely persist
a proxy. Beyond simple serialization, there's no mechanism in place
to short-circuit requests.

That's proxies. Can requests be serialized? For that matter, any reason not
to just hold on to them? I'm not talking about LocalStorage or anything
here, just optimizing within a single session.

I know we can extend or replace DefaultRequestTransport on the client side,
e.g. as done in Expenses for GAE
integrationhttps://cs.corp.google.com/#google3/third_party/java_src/gwt/svn/trunk/samples/expenses/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java.
If requests have reasonable equality semantics and are somewhat
introspectable, even just at the instanceof level, we might be able to get
some simple caching done in this sample.

@jlabanca, I know your time is short and you gotta do what you gotta do. I
don't mean to hold up this change for caching. I just want to have the
conversation while we have a use case staring us in the face.

This change was already submitted, but a more comprehensive solution would
be nice.

Bob Vawter
Google Web Toolkit Team

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

I'd be inclined to start with a) and see what happens.

On Wed, May 4, 2011 at 10:02 AM, rdcas...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode205
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:205: if
 (isFullyInitialized()) {
 On 2011/05/04 16:47:17, rjrjr wrote:

 This seems pretty unexpected. I would have thought it would either be

 an error

 (RuntimeException), or else that I would do the opposite of this:

 replace my

 existing element with the new one.


 I'd be totally fine with a RuntimeError, it'll probably be m ore
 manageable in the long-term. I was trying to support this because even
 though it's less efficient, we can still make it work. The scenario here
 is:

 1-) Build Attachable tree. This includes calling all the render() stuff
 and so on. Let's assume that this particular AttachableHTMLPanel is in
 the middle of the tree.

 2-) For some reason do something to this particular panel (that's in the
 middle of the tree) that calls its getElement() [let's say attach it to
 the document]. This triggers the process of building the widget tree as
 if this panel was the root (i.e., hidden div, set innerHTML, getting
 elements for all children and initializing them). At this point
 everything works

 3-) Now you go and attach the real root of the tree, an ancestor of
 this panel. We can do one of 3 things:
  (a) throw an error. You probably didn't mean to do this as it's less
 efficient
  (b) replace the element that our parent assigned us (along with its
 subtree) with the subtree we already built in step 2
  (c) re-do everything in step 2, ignoring the fact that we already
 initialized all children widget

 I think (a) would be ideal, specially for debugging purposes. Honestly I
 implemented (b) because I was a little afraid and wanted to be as
 backwards-compatible as possible.  I don't think (c) would work
 out-of-the-box, as we'd have to support initializing Widgets twice
 (i.e., re-setting the element, etc.)

 Makes sense?


 http://gwt-code-reviews.appspot.com/1427812/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Including the TaskProxy (when available) in TaskEditPlace so we do not do an extra round trip to... (issue1428810)

Something as simple as short circuiting all equivalent
RequestFactory.find(EntityProxyIdP) requests, where equivalent takes into
account the proxy version as well as its id, could be very effective.

On Wed, May 4, 2011 at 10:04 AM, John LaBanca jlaba...@google.com wrote:

On Wed, May 4, 2011 at 12:58 PM, Ray Ryan rj...@google.com wrote:

On Wed, May 4, 2011 at 9:47 AM, Bob Vawter robertvaw...@google.comwrote:

Bob, I've lost track -- are there hooks in place yet that we could
implement
app specific client side caching in a sample like this?

That's proxies. Can requests be serialized? For that matter, any reason
not to just hold on to them? I'm not talking about LocalStorage or anything
here, just optimizing within a single session.

I know we can extend or replace DefaultRequestTransport on the client
side, e.g. as done in Expenses for GAE
integrationhttps://cs.corp.google.com/#google3/third_party/java_src/gwt/svn/trunk/samples/expenses/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java.
If requests have reasonable equality semantics and are somewhat
introspectable, even just at the instanceof level, we might be able to get
some simple caching done in this sample.

This change was already submitted, but a more comprehensive solution would
be nice.

Bob Vawter
Google Web Toolkit Team

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


How's that?

Is the bit I wrote about after adding it to a panel accurate? Seems
like we're trying to get to a world where the add would be fine, and the
wrap call  wouldn't happen until the parent is wrapped — are we there
already?


http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
(right):

http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode211
user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:211:
throw new IllegalStateException(
wrapElement() cannot be called twice, or after a call to getElement()
has forced the widget to render itself (e.g. after adding it to a panel)

http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

2011-05-04 Thread Rafael Castro

Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
this already works pretty well. I have to review some other tricky cases
(like if you add a non-attachable widget to an Attachable panel before
finishing the initialization), but we're pretty close. The other cases that
could trigger this is calling some UIObject method that we haven't yet
@Override (like we did for setStyleName). Those call getElement() and hurt
us.

On Wed, May 4, 2011 at 2:15 PM, rj...@google.com wrote:

 How's that?

 Is the bit I wrote about after adding it to a panel accurate? Seems
 like we're trying to get to a world where the add would be fine, and the
 wrap call  wouldn't happen until the parent is wrapped — are we there
 already?



 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java

 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode211
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:211:
 throw new IllegalStateException(
 wrapElement() cannot be called twice, or after a call to getElement()
 has forced the widget to render itself (e.g. after adding it to a panel)


 http://gwt-code-reviews.appspot.com/1427812/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] [google-web-toolkit] r10135 committed - Allows enum ordinalization to proceed for enums with static methods/fi...


Revision: 10135
Author:   jbrosenb...@google.com
Date: Wed May  4 07:26:14 2011
Log:  Allows enum ordinalization to proceed for enums with static  
methods/fields
-- This offers modest code-size gains, and will open way for further  
optimizations


-- Also, fixes a bug in checking for instanceof arguments for  
EnumOrdinalizer


-- Fixed some formatting in EnumOrdinalizer

-- Added new tests to EnumOrdinalizerTest

Review at http://gwt-code-reviews.appspot.com/1428808

http://code.google.com/p/google-web-toolkit/source/detail?r=10135

Modified:
 /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java
 /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java
 /trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
 /trunk/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java

===
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java	Mon Jun   
7 12:20:31 2010
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/ast/JEnumType.java	Wed May   
4 07:26:14 2011

@@ -29,6 +29,7 @@
*/

   private ListJEnumField enumList = Lists.create();
+  private boolean isOrdinalized = false;

   public JEnumType(SourceInfo info, String name, boolean isAbstract) {
 super(info, name, isAbstract, false);
@@ -63,4 +64,12 @@
   public JEnumType isEnumOrSubclass() {
 return this;
   }
-}
+
+  public boolean isOrdinalized() {
+return isOrdinalized;
+  }
+
+  public void setOrdinalized() {
+isOrdinalized = true;
+  }
+}
===
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java	 
Thu Apr 28 08:18:20 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java	 
Wed May  4 07:26:14 2011

@@ -71,9 +71,13 @@
  * This optimizer modifies enum classes to change their field constants to  
ints,

  * and to remove initialization of those constants in the clinit method. An
  * ordinalized enum class will not be removed from the AST by this  
optimizer,

- * but as long as all references to it are replaced (which is one of the
- * requirements for ordinalization), then the enum class itself will be  
pruned

- * by subsequent optimizer passes.
+ * but as long as all references to it are replaced, then the enum class  
itself
+ * will be pruned by subsequent optimizer passes. Some enum classes may  
not be
+ * completely removed however. Ordinalization can proceed in cases where  
there
+ * are added static fields or methods in the enum class. In such cases a  
reduced
+ * version of the original enum class can remain in the AST, containing  
only
+ * static fields and methods which aren't part of the enum infrastructure  
(in

+ * which case it will no longer behave as an enum class at all).
  *
  * Regardless of whether an ordinalized enum class ends up being completely
  * pruned away, the AST is expected to be in a coherent and usable state  
after

@@ -295,13 +299,29 @@
* does this by keeping track of a black-list for ordinals which  
violate the

* conditions for ordinalization, below.
*
-   * An enum cannot be ordinalized, if it: is implicitly upcast. is  
implicitly
-   * cast to from a nullType. is implicitly cast to or from a  
javaScriptObject

-   * type. is explicitly cast to another type (or vice-versa). it's class
-   * literal is used explicitly. it has an artificial rescue recorded for  
it.

-   * has any field referenced, except for: one of it's enum constants
-   * Enum.ordinal has any method called, except for: ordinal()  
Enum.ordinal()

-   * Enum() super constructor Enum.createValueOfMap()
+   * An enum cannot be ordinalized, if it:
+   * ul
+   * liis implicitly upcast./li
+   * liis implicitly cast to from a nullType./li
+   * liis implicitly cast to or from a javaScriptObject type./li
+   * liis explicitly cast to another type (or vice-versa)./li
+   * liis tested in an instanceof expression./li
+   * liit's class literal is used explicitly./li
+   * liit has an artificial rescue recorded for it./li
+   * lihas any field referenced, except for:/li
+   * ul
+   * listatic fields, other than the synthetic $VALUES field./li
+   * liEnum.ordinal./li
+   * /ul
+   * lihas any method called, except for:/li
+   * ul
+   * liordinal()./li
+   * liEnum.ordinal()./li
+   * liEnum() super constructor./li
+   * liEnum.createValueOfMap()./li
+   * listatic methods, other than values() or valueOf()./li
+   * /ul
+   * /ul
*
* This visitor extends the ImplicitUpcastAnalyzer, which encapsulates  
all the
* conditions where implicit upcasting can occur in an AST. The rest of  
the

@@ -394,6 +414,11 @@
   JEnumType maybeEnum = x.isEnumOrSubclass();
   if (maybeEnum != null) {
 enumsVisited.put(program.getClassLiteralName(maybeEnum),  
maybeEnum);

+
+// don't need to re-ordinalize a previously ordinalized enum
+if (maybeEnum.isOrdinalized()) {
+  addToBlackList(maybeEnum,

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

2011-05-04 Thread Rafael Castro

Done. I didn't know how verbose I should be on the error message. Please
advise :-)

On Wed, May 4, 2011 at 2:04 PM, Ray Ryan rj...@google.com wrote:

 I'd be inclined to start with a) and see what happens.


 On Wed, May 4, 2011 at 10:02 AM, rdcas...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode205
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:205: if
 (isFullyInitialized()) {
 On 2011/05/04 16:47:17, rjrjr wrote:

 This seems pretty unexpected. I would have thought it would either be

 an error

 (RuntimeException), or else that I would do the opposite of this:

 replace my

 existing element with the new one.


 I'd be totally fine with a RuntimeError, it'll probably be m ore
 manageable in the long-term. I was trying to support this because even
 though it's less efficient, we can still make it work. The scenario here
 is:

 1-) Build Attachable tree. This includes calling all the render() stuff
 and so on. Let's assume that this particular AttachableHTMLPanel is in
 the middle of the tree.

 2-) For some reason do something to this particular panel (that's in the
 middle of the tree) that calls its getElement() [let's say attach it to
 the document]. This triggers the process of building the widget tree as
 if this panel was the root (i.e., hidden div, set innerHTML, getting
 elements for all children and initializing them). At this point
 everything works

 3-) Now you go and attach the real root of the tree, an ancestor of
 this panel. We can do one of 3 things:
  (a) throw an error. You probably didn't mean to do this as it's less
 efficient
  (b) replace the element that our parent assigned us (along with its
 subtree) with the subtree we already built in step 2
  (c) re-do everything in step 2, ignoring the fact that we already
 initialized all children widget

 I think (a) would be ideal, specially for debugging purposes. Honestly I
 implemented (b) because I was a little afraid and wanted to be as
 backwards-compatible as possible.  I don't think (c) would work
 out-of-the-box, as we'd have to support initializing Widgets twice
 (i.e., re-setting the element, etc.)

 Makes sense?


 http://gwt-code-reviews.appspot.com/1427812/




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)

2011-05-04 Thread rchandia



http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode17
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:17:
On 2011/05/04 15:07:33, pdr wrote:

This linker should be defined in a gwt.xml file somewhere


Done. It seems I missed that change in the shuffle of splitting the
original change in two.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode39
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:39: *
liAdd a mime-mapping to your web.xml file (this is for AppEngine,
adjust
On 2011/05/04 15:56:54, tobyr wrote:

The sample XML below looks standard to me. Why are we mentioning App

Engine?

Done.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode49
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:49: *
On 2011/05/04 14:59:48, jlabanca wrote:

Add p before each paragraph


Done.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode55
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:55: *
and overrides {@code otherCAchedFiles()}, and use it as a linker
instead:
On 2011/05/04 14:59:48, jlabanca wrote:

/r/otherCAchedFiles/otherCachedFiles



You might mention that wildcards, such as /audio/*, will work (I think

thats

true).


I could not find that in
http://dev.w3.org/html5/spec/offline.html#offline

There is a wildcard for online resources, though.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode55
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:55: *
and overrides {@code otherCAchedFiles()}, and use it as a linker
instead:
On 2011/05/04 15:07:33, pdr wrote:

otherCAchedFiles - otherCachedFiles


Done.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode68
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:68:
public class SimpleAppCacheLinker extends AbstractLinker {
On 2011/05/04 14:59:48, jlabanca wrote:

I didn't see a rule in a gwt.xml file.

Done. It seems I got that change misplaced when I split the original
patch in two.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode102
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:102:
protected String[] otherCachedFiles() {
On 2011/05/04 14:59:48, jlabanca wrote:

This is fine for now, but it would be nice if users could specify

other cached

files in a gwt.xml or appcache.manifest file instead of overriding

this class.

Agreed.

http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java#newcode166
dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java:166:
// build cache list
On 2011/05/04 15:07:33, pdr wrote:

This code is essentially a duplicate of the above code. Break it out

into a

method?


Done.

http://gwt-code-reviews.appspot.com/1428811/diff/1/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java
File
samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java
(right):

http://gwt-code-reviews.appspot.com/1428811/diff/1/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java#newcode37
samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java:37:
/audio/error.wav
On 2011/05/04 14:59:48, jlabanca wrote:

also add:
video/tutorial.mp4
video/tutorial.ogv


Done.

http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

Once we've validated the work, seems like a lot of the Attachable support
should be baked into UiObject, Widget and Panel in some kind of opt-in
manner.

On Wed, May 4, 2011 at 10:20 AM, Rafael Castro rdcas...@google.com wrote:

 Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
 this already works pretty well. I have to review some other tricky cases
 (like if you add a non-attachable widget to an Attachable panel before
 finishing the initialization), but we're pretty close. The other cases that
 could trigger this is calling some UIObject method that we haven't yet
 @Override (like we did for setStyleName). Those call getElement() and hurt
 us.


 On Wed, May 4, 2011 at 2:15 PM, rj...@google.com wrote:

 How's that?

 Is the bit I wrote about after adding it to a panel accurate? Seems
 like we're trying to get to a world where the add would be fine, and the
 wrap call  wouldn't happen until the parent is wrapped — are we there
 already?



 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java

 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode211
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:211:
 throw new IllegalStateException(
 wrapElement() cannot be called twice, or after a call to getElement()
 has forced the widget to render itself (e.g. after adding it to a panel)


 http://gwt-code-reviews.appspot.com/1427812/


  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Re: [gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)

2011-05-04 Thread Rafael Castro

Exactly, that's my hope too :-)
Hopefully the numbers will be enough to convince us the work is worth doing.

On Wed, May 4, 2011 at 2:57 PM, Ray Ryan rj...@google.com wrote:

 Once we've validated the work, seems like a lot of the Attachable support
 should be baked into UiObject, Widget and Panel in some kind of opt-in
 manner.

 On Wed, May 4, 2011 at 10:20 AM, Rafael Castro rdcas...@google.comwrote:

 Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
 this already works pretty well. I have to review some other tricky cases
 (like if you add a non-attachable widget to an Attachable panel before
 finishing the initialization), but we're pretty close. The other cases that
 could trigger this is calling some UIObject method that we haven't yet
 @Override (like we did for setStyleName). Those call getElement() and hurt
 us.


 On Wed, May 4, 2011 at 2:15 PM, rj...@google.com wrote:

 How's that?

 Is the bit I wrote about after adding it to a panel accurate? Seems
 like we're trying to get to a world where the add would be fine, and the
 wrap call  wouldn't happen until the parent is wrapped — are we there
 already?



 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java

 File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
 (right):


 http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode211
 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:211:
 throw new IllegalStateException(
 wrapElement() cannot be called twice, or after a call to getElement()
 has forced the widget to render itself (e.g. after adding it to a panel)


 http://gwt-code-reviews.appspot.com/1427812/


  --
 http://groups.google.com/group/Google-Web-Toolkit-Contributors




-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)


LGTM

http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)


Reviewers: rjrjr,

Description:
Adds user authentication to MobileWebApp.  Tasks are now specific to
each user.  Authentication is done using a Filter, which was copied
almost verbatim from the expenses sample.  If a user is not logged in,
they are forwarded to the login page, then redirected back to their last
place in the app.  If a user tries to access a task that they do not
own, an error messge says that the task cannot be found.

You can view a demo at http://jlabanca-testing.appspot.com

This change also replaces the auto generated datastore index file with a
manual datastore index file.  There aren't any indexes yet, but we may
add them eventually, and using an auto-generated file is asking for
trouble because its easy enough to miss a possible index.


Please review this at http://gwt-code-reviews.appspot.com/1432801/

Affected files:
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/GaeRequest.gwt.xml
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/ReloadOnAuthenticationFailure.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/shared/GaeHelper.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/MobileWebApp.gwt.xml
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactoryImpl.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/MobileWebApp.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/UserServiceWrapper.java

  A samples/mobilewebapp/war/WEB-INF/datastore-indexes.xml
  M samples/mobilewebapp/war/WEB-INF/web.xml


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)



http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
File
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
(right):

http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java#newcode48
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java:48:
}
This seems bad. I'd never allow code like this in a production app. An
unauthorized user shouldn't even be able to reach this class.

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)

I guess I'm speaking strictly of the null checks. It's fine for our sample
code not to have a real auth system in its storage, of course.

Seems like your UserServiceWrapper should have a requireCurrentUserId()
method that throws an exception if there is no id, and Task shouldn't be
friendly. Is that reasonable?

On Wed, May 4, 2011 at 11:19 AM, rj...@google.com wrote:



 http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
 File


 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
 (right):


 http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java#newcode48

 samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java:48:
 }
 This seems bad. I'd never allow code like this in a production app. An
 unauthorized user shouldn't even be able to reach this class.


 http://gwt-code-reviews.appspot.com/1432801/


-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)


Reviewers: jlabanca,

Description:
Step one in making mobilewebapp more DI friendly. The goal is to
remove all knowledge of ClientFactory from any class but the entry
point.

This gets the basic pattern in place, submitting it as a checkpoint so
that new features can appear in the right place.

In particular, introduces the App class, and elminates a lot of low
hanging ClientFactory leakage. The interesting bit will be getting it
out of the activities. And a particularly nasty spot is where
MobileWebAppShellTablet (view class) is kicking off a
TaskListActivity. That's next.


Please review this at http://gwt-code-reviews.appspot.com/1433801/

Affected files:
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/App.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactoryImpl.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactoryImplMobile.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactoryImplTablet.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/MobileWebApp.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/MobileWebAppShellBase.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/Provider.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/AppActivityMapper.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/activity/TaskListActivity.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/mobile/MobileWebAppShellMobile.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/place/AppPlaceHistoryMapper.java
  M  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/tablet/MobileWebAppShellTablet.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationChangeEvent.java
  A  
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationMonitor.java

  M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)


Sorry for the noise, not yet ready for review. Soon.

http://gwt-code-reviews.appspot.com/1433801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)


It shouldn't ever happen because GaeFilter blocks users if they aren't
logged in.  I'll modify getCurrentUser/getCurrentUserId to throw an
exception if the user isn't logged in.


http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
File
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
(right):

http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java#newcode48
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java:48:
}
GaeAuthFilter should prevent this from ever happening, but I'll throw an
exception for good measure.

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)


http://gwt-code-reviews.appspot.com/1433801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)


Ready for review.

http://gwt-code-reviews.appspot.com/1433801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] [google-web-toolkit] r10136 committed - Adds HTML5 App Cache support to MobileWebApp sample....


Revision: 10136
Author:   rchan...@google.com
Date: Wed May  4 09:12:17 2011
Log:  Adds HTML5 App Cache support to MobileWebApp sample.
Uses a custom linker to figure out which files were generated by GWTC.

Review at http://gwt-code-reviews.appspot.com/1428811

http://code.google.com/p/google-web-toolkit/source/detail?r=10136

Added:
 /trunk/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
  
/trunk/dev/core/test/com/google/gwt/core/linker/SimpleAppCacheLinkerTest.java

 /trunk/samples/mobilewebapp/src/dev
 /trunk/samples/mobilewebapp/src/dev/com
 /trunk/samples/mobilewebapp/src/dev/com/google
 /trunk/samples/mobilewebapp/src/dev/com/google/gwt
 /trunk/samples/mobilewebapp/src/dev/com/google/gwt/sample
 /trunk/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp
  
/trunk/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker
  
/trunk/samples/mobilewebapp/src/dev/com/google/gwt/sample/mobilewebapp/linker/AppCacheLinker.java

Modified:
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/MobileWebApp.gwt.xml

 /trunk/samples/mobilewebapp/user-build.xml
 /trunk/samples/mobilewebapp/war/MobileWebApp.html
 /trunk/samples/mobilewebapp/war/WEB-INF/web.xml
 /trunk/user/test/com/google/gwt/core/ext/LinkerSuite.java

===
--- /dev/null
+++  
/trunk/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java	 
Wed May  4 09:12:17 2011

@@ -0,0 +1,181 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may  
not
+ * use this file except in compliance with the License. You may obtain a  
copy of

+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations  
under

+ * the License.
+ */
+package com.google.gwt.core.linker;
+
+import com.google.gwt.core.ext.LinkerContext;
+import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.core.ext.linker.AbstractLinker;
+import com.google.gwt.core.ext.linker.Artifact;
+import com.google.gwt.core.ext.linker.ArtifactSet;
+import com.google.gwt.core.ext.linker.EmittedArtifact;
+import com.google.gwt.core.ext.linker.LinkerOrder;
+import com.google.gwt.core.ext.linker.LinkerOrder.Order;
+import com.google.gwt.core.ext.linker.impl.SelectionInformation;
+
+import java.util.Arrays;
+import java.util.Date;
+
+/**
+ * AppCacheLinker - linker for public path resources in the Application  
Cache.

+ * p
+ * To use:
+ * ol
+ * liAdd {@code manifest=YOURMODULENAME/appcache.nocache.manifest} to  
the

+ * {@code html} tag in your base html file. E.g.,
+ * {@code html manifest=mymodule/appcache.nocache.manifest}/li
+ * liAdd a mime-mapping to your web.xml file:
+ * p
+ * pre{@code mime-mapping
+ * extensionmanifest/extension
+ * mime-typetext/cache-manifest/mime-type
+ * /mime-mapping
+ * }/pre
+ * /li
+ * /ol
+ * p
+ * On every compile, this linker will regenerate the  
appcache.nocache.manifest

+ * file with files from the public path of your module.
+ * p
+ * To obtain a manifest that contains other files in addition to those
+ * generated by this linker, create a class that inherits from this one
+ * and overrides {@code otherCachedFiles()}, and use it as a linker  
instead:

+ * p
+ * preblockquote
+ * {@code @Shardable}
+ * public class MyAppCacheLinker extends AbstractAppCacheLinker {
+ *   {@code @Override}
+ *   protected String[] otherCachedFiles() {
+ * return new String[] {/MyApp.html,/MyApp.css};
+ *   }
+ * }
+ * /blockquote/pre
+ */
+@LinkerOrder(Order.POST)
+public class SimpleAppCacheLinker extends AbstractLinker {
+
+  private static final String MANIFEST = appcache.nocache.manifest;
+
+  @Override
+  public String getDescription() {
+return AppCacheLinker;
+  }
+
+  @Override
+  public ArtifactSet link(TreeLogger logger, LinkerContext context,  
ArtifactSet artifacts,

+  boolean onePermutation)
+  throws UnableToCompleteException {
+
+ArtifactSet toReturn = new ArtifactSet(artifacts);
+
+if (onePermutation) {
+  return toReturn;
+}
+
+if (toReturn.find(SelectionInformation.class).isEmpty()) {
+  logger.log(TreeLogger.INFO, devmode: generating empty  + MANIFEST);
+  artifacts = null;
+}
+
+// Create the general cache-manifest resource for the landing page:
+toReturn.add(emitLandingPageCacheManifest(context, logger, artifacts));
+return toReturn;
+  }
+
+  /**
+   * Override this method to force the linker to also include more files
+   * in the manifest.
+   */
+  protected String[] otherCachedFiles() {
+return null;
+  }
+
+  /**
+   * Creates the

[gwt-contrib] Re: Adds HTML5 App Cache support to MobileWebApp sample. (issue1428811)

2011-05-04 Thread rchandia


Submitted as of r10136



http://gwt-code-reviews.appspot.com/1428811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)


We now throw an exception instead of trying to work around the problem.
The code inside of Task should never get called if the user is logged
out because GaeFilter catches it.

We also now redirect correctly when the user hits the default page
(without specifying a file).  The app loads, but redirects to the login
page after the first request.

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)



http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml
File samples/mobilewebapp/war/WEB-INF/web.xml (right):

http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml#newcode11
samples/mobilewebapp/war/WEB-INF/web.xml:11: --
Is the request factory servlet protected?

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)


http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)



http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml
File samples/mobilewebapp/war/WEB-INF/web.xml (right):

http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml#newcode11
samples/mobilewebapp/war/WEB-INF/web.xml:11: --
Its protected by GaeAuthFilter, not here.  If we protect it here, then
GAE will respond to RequestFactory requests with a URL redirect, which
RequestFactory client code has no idea how to handle.

This is a shortcut to quickly redirect users to the login page if they
try to access the app without being logged in.  The filter is the main
check as to whether the user is logged in.

http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml#newcode47
samples/mobilewebapp/war/WEB-INF/web.xml:47:
url-pattern/gwtRequest/*/url-pattern
GaeAuthFilter checks that the user is logged in when a gwtRequest is
sent. This needs to work this way because a user could log out while the
app is running. The expenses sample does the same thing.

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Step one in making mobilewebapp more DI friendly. The goal is to (issue1433801)


LGTM if you make OrientationMonitor.provider() reference a singleton
boolean instead of recalculating.


http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
File
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
(right):

http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java#newcode50
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java:50:
* @return the local {@link Storage} object, or null unsupporting
browsers
/r/null unsupporting/null in unsupporting

http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationMonitor.java
File
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationMonitor.java
(right):

http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationMonitor.java#newcode43
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ui/OrientationMonitor.java:43:
return calculate();
Calling calculate() every time we access the getter could lead to
performance degradation because the browser has to perform layout to
calculate window.clientHeight/Width.  Instead, can we use a singleton
instance of OrientationMonitor to maintain an orientation bit and check
it in the getter?

http://gwt-code-reviews.appspot.com/1433801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)

2011-05-04 Thread rice


LGTM


http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java
File user/src/com/google/gwt/event/dom/client/DragEndEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java#newcode42
user/src/com/google/gwt/event/dom/client/DragEndEvent.java:42: * to fire
drag end events.
also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEvent.java
File user/src/com/google/gwt/event/dom/client/DragEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEvent.java#newcode40
user/src/com/google/gwt/event/dom/client/DragEvent.java:40: * {@link
DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java
File user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java#newcode41
user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java:41: *
{@link DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragStartEvent.java
File user/src/com/google/gwt/event/dom/client/DragStartEvent.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragStartEvent.java#newcode41
user/src/com/google/gwt/event/dom/client/DragStartEvent.java:41: *
{@link DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DropEvent.java
File user/src/com/google/gwt/event/dom/client/DropEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DropEvent.java#newcode21
user/src/com/google/gwt/event/dom/client/DropEvent.java:21: public class
DropEvent extends DragEventBaseDropHandler {
Should DragEventBase be named something like DragDropEventBase?

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java
File
user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java#newcode123
user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java:123:
if (typeInt  0) {
Is this test needed?  Seems like typeInt cannot be 0 and even if it is
the | will be harmless

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/client/ui/Widget.java
File user/src/com/google/gwt/user/client/ui/Widget.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode105
user/src/com/google/gwt/user/client/ui/Widget.java:105:
sinkEvents(Event.getTypeInt(type.getName()));
Use 'typeInt' rather than a new call to Event.getTypeInt(type.getName())

http://gwt-code-reviews.appspot.com/1420811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)



http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java
File user/src/com/google/gwt/event/dom/client/DragEndEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java#newcode42
user/src/com/google/gwt/event/dom/client/DragEndEvent.java:42: * to fire
drag end events.
On 2011/05/04 20:31:47, rice wrote:

also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?


Done.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEvent.java
File user/src/com/google/gwt/event/dom/client/DragEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEvent.java#newcode40
user/src/com/google/gwt/event/dom/client/DragEvent.java:40: * {@link
DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
On 2011/05/04 20:31:47, rice wrote:

also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?


Done.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java
File user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java#newcode41
user/src/com/google/gwt/event/dom/client/DragLeaveEvent.java:41: *
{@link DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
On 2011/05/04 20:31:47, rice wrote:

also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?


Done.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragStartEvent.java
File user/src/com/google/gwt/event/dom/client/DragStartEvent.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragStartEvent.java#newcode41
user/src/com/google/gwt/event/dom/client/DragStartEvent.java:41: *
{@link DomEvent#fireNativeEvent(com.google.gwt.dom.client.NativeEvent,
com.google.gwt.event.shared.HasHandlers)}
On 2011/05/04 20:31:47, rice wrote:

also link to fireNativeEvent(NativeEvent, HasHandlers, Element) here?


Done.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DropEvent.java
File user/src/com/google/gwt/event/dom/client/DropEvent.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DropEvent.java#newcode21
user/src/com/google/gwt/event/dom/client/DropEvent.java:21: public class
DropEvent extends DragEventBaseDropHandler {
On 2011/05/04 20:31:47, rice wrote:

Should DragEventBase be named something like DragDropEventBase?


Done.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java
File
user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java
(right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java#newcode123
user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java:123:
if (typeInt  0) {
It can be -1 if the implementation subclass specially handles the event
type. I think or'ing the -1 can mess up the events to sink.

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/client/ui/Widget.java
File user/src/com/google/gwt/user/client/ui/Widget.java (right):

http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode105
user/src/com/google/gwt/user/client/ui/Widget.java:105:
sinkEvents(Event.getTypeInt(type.getName()));
On 2011/05/04 20:31:47, rice wrote:

Use 'typeInt' rather than a new call to

Event.getTypeInt(type.getName())

Done.

http://gwt-code-reviews.appspot.com/1420811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)


committed as r10138

http://gwt-code-reviews.appspot.com/1420811/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Add runtime-locale support for Localizable subtypes. (issue1425816)

2011-05-04 Thread jat


Ping

http://gwt-code-reviews.appspot.com/1425816/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix error in usage of newly-creted helper method in AttachableHTMLPanel, correct double-calling ... (issue1427812)


LGTM

http://gwt-code-reviews.appspot.com/1427812/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] [google-web-toolkit] r10139 committed - Adds user authentication to MobileWebApp. Tasks are now specific to e...


Revision: 10139
Author:   jlaba...@google.com
Date: Wed May  4 11:30:38 2011
Log:  Adds user authentication to MobileWebApp.  Tasks are now specific  
to each user.  Authentication is done using a Filter, which was copied  
almost verbatim from the expenses sample.  If a user is not logged in, they  
are forwarded to the login page, then redirected back to their last place  
in the app.  If a user tries to access a task that they do not own, an  
error messge says that the task cannot be found.


You can view a demo at http://jlabanca-testing.appspot.com

This change also replaces the auto generated datastore index file with a  
manual datastore index file.  There aren't any indexes yet, but we may add  
them eventually, and using an auto-generated file is asking for trouble  
because its easy enough to miss a possible index.


Review at http://gwt-code-reviews.appspot.com/1432801

Review by: rj...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10139

Added:
 /trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/GaeRequest.gwt.xml
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/ReloadOnAuthenticationFailure.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/server
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/shared
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/shared/GaeHelper.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/UserServiceWrapper.java

 /trunk/samples/mobilewebapp/war/WEB-INF/datastore-indexes.xml
Modified:
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/MobileWebApp.gwt.xml
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactoryImpl.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/MobileWebApp.java
  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java

 /trunk/samples/mobilewebapp/war/WEB-INF/web.xml

===
--- /dev/null
+++  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/GaeRequest.gwt.xml	 
Wed May  4 11:30:38 2011

@@ -0,0 +1,8 @@
+?xml version=1.0 encoding=UTF-8?
+!DOCTYPE module PUBLIC -//Google Inc.//DTD Google Web Toolkit  
0.0.999//EN http://google-web-toolkit.googlecode.com/svn/tags/0.0.999/distro-source/core/src/gwt-module.dtd;

+module
+  inherits name='com.google.web.bindery.requestfactory.RequestFactory'/
+
+  source path='client'/
+  source path='shared'/
+/module
===
--- /dev/null
+++  
/trunk/samples/mobilewebapp/src/main/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java	 
Wed May  4 11:30:38 2011

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may  
not
+ * use this file except in compliance with the License. You may obtain a  
copy of

+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,  
WITHOUT

+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations  
under

+ * the License.
+ */
+package com.google.gwt.sample.gaerequest.client;
+
+import com.google.gwt.event.shared.EventBus;
+import com.google.gwt.http.client.Request;
+import com.google.gwt.http.client.RequestCallback;
+import com.google.gwt.http.client.Response;
+import com.google.gwt.sample.gaerequest.shared.GaeHelper;
+import com.google.gwt.user.client.Window;
+import  
com.google.web.bindery.requestfactory.gwt.client.DefaultRequestTransport;

+import com.google.web.bindery.requestfactory.shared.ServerFailure;
+
+/**
+ * Extends DefaultRequestTransport to handle the authentication failures
+ * reported by {@link  
com.google.gwt.sample.gaerequest.server.GaeAuthFilter}.

+ */
+public class GaeAuthRequestTransport extends DefaultRequestTransport {
+  private final EventBus eventBus;
+
+  public GaeAuthRequestTransport(EventBus eventBus) {
+this.eventBus = eventBus;
+  }
+
+  @Override
+  protected RequestCallback createRequestCallback(final TransportReceiver  
receiver) {
+final RequestCallback superCallback =  
super.createRequestCallback(receiver);

[gwt-contrib] Re: Adds user authentication to MobileWebApp. Tasks are now specific to each user. Authentication ... (issue1432801)


committed as r10139

http://gwt-code-reviews.appspot.com/1432801/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] [google-web-toolkit] r10140 committed - Fix error in usage of newly-creted helper method in AttachableHTMLPane...