I'm sorry I've only started to review the files (over the last few days) but I have a first question/comment about where this is going:
There are many things that are not needed in the case of MobileWebApp as the host page is protected behind authentication. Because the user won't ever see this page when being unauthenticated: - LoginWidget does not need to have a dual state (there's no need for a "login" link) - the username could be inlined in the script generated by the JSP (that might not work very well with the ApplicationCache/offline though) Also, there's probably no need to pass MobileWebApp.jsp as an init-param to the GaeAuthFilter as it could just use getContextPath() (MobileWebApp.jsp is in the welcome-file-list in the web.xml), or "/" (the context path is always "/" on GAE) As far as offline is concerned, it might get in our way here, but I'm no expert in mobile dev. Anyway, it seems to cause us more harm than anything, and the linker is known not to be great (it will load all permutations in the client's cache, only to use one of them) This would greatly simplify the code, at the expense of removing a bunch of reusable code (LoginWidget, etc.) http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java File samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java (right): http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java#newcode18 samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthRequestTransport.java:18: import com.google.gwt.event.shared.EventBus; Why this change? http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java File samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java (right): http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java#newcode18 samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java:18: import com.google.gwt.event.shared.EventBus; Same question: what's the reason behind this change? http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java#newcode64 samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/client/GaeAuthenticationFailureEvent.java:64: * @param state a {@link State} instance This looks like a leftover from an experiment (remark also applies to the javadoc on the loginUrl field) http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp File samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp (right): http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp#newcode37 samples/mobilewebapp/src/main/webapp/MobileWebApp.jsp:37: window['__gwtsample_mobilewebapp__'].loginUrl = '<%=appUrls.getLoginUrl()%>'; Wouldn't an object literal be more readable? window['__gwtsample_mobilewebapp__'] = { loginUrl: '<%= appUrls.getLoginUrl() %>', logoutUrl: '<%= appUrls.getLogoutUrl() %>' }; (I'd also use "var __gwtsample_mobilewebapp__ =" instead of "window['__gwtsample_mobilewebapp__'] =" but that's more a matter of personal taste) Oh, and BTW, the page is protected (in web.xml), so there's no need for the loginUrl here. http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml File samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml (right): http://gwt-code-reviews.appspot.com/1829803/diff/1/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml#newcode10 samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml:10: auth filters in place for all other RPC calls. Thre's no real need to protect the compiled "authentication auth filters" http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java File samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java (right): http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java#newcode93 samples/mobilewebapp/src/main/java/com/google/gwt/sample/gaerequest/server/GaeAuthFilter.java:93: homePageUrl.append(scheme); No need for all this; UserService accepts paths, so "/" + homePageFileName + (queryString == null ? "" : "?" + queryString) would be enough (even though I'd rather keep only the "gwt.codesvr" param from the queryString) http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml File samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml (right): http://gwt-code-reviews.appspot.com/1829803/diff/9003/samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml#newcode58 samples/mobilewebapp/src/main/webapp/WEB-INF/web.xml:58: <welcome-file>MobileWebApp.html</welcome-file> Should be .jsp here http://gwt-code-reviews.appspot.com/1829803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
