Odd, I didn't before, but do now. I'll create a new patch and see if that fixes things; I've been getting this error for a few patches now. Thanks for the heads-up.
On Tue, Oct 27, 2009 at 6:46 PM, Chirag Shah <[email protected]> wrote: > I'm not able to view the changes to > BrowserSpecificFeatureResource.java. The review tool is displaying > "error: old chunk mismatch." > Do you see the same thing? http://codereview.appspot.com/143046/diff/1/36 > > On Tue, Oct 27, 2009 at 5:59 PM, <[email protected]> wrote: > > Reviewers: shindig.remailer_gmail.com, jasvir, Jon Weygandt, > > > > Description: > > The feature.xml and JS system has served us well, yet various new > > requirements/requests have arisen that have stressed the current > > implementation. Among them: > > * Ability to cleanly represent "code" features and "JS" features in a > > dependency tree w/ one another. This allows, for instance, a clean way > > for "opensocial" to depend on "security-token" (not yet introduced), > > which in turn depends on "locked-domain" to ensure token security. > > * Ability to let a gadget opt out of all "core" JavaScript, instead only > > including sub-components that are used, to shrink render size. > > * Ability to serve JS in a context-sensitive fashion ie. > > browser-specific code. > > * Ability to serve JS in a feature-sensitive fashion ie. only inject > > certain code when feature="caja" is enabled/active. > > * Ability in development mode to easily serve JS that changes when edits > > are made on it. > > > > For these and general code-cleanup purposes, I've rewritten the > > Feature/JS inclusion implementation as presented in this CL. > > > > This implementation is functionally 100% backward-compatible with its > > predecessor. It is only NOT backward-compatible with existing extensions > > (outside of Shindig's code base) that override > > JsFeatureLoader/GadgetFeatureRegistry. > > > > With this change, the unit of extension in the features system becomes > > the <script ...> line in a feature.xml. The processor for this line is a > > FeatureResourceLoader, for which a default implementation is provided. > > Each <script> line yields a FeatureResource object, which is in turn > > used to resolve content and debugContent by rendering/JS code. > > > > Other components are overridable/subclassable as well, but to date I've > > yet to hear a JS/features extension use case that does not fit in the > > ResourceLoader model (or, in the current model with dynamically > > register()'ed features). Hopefully that's right. > > > > An overview of the implementation follows. > > > > FeatureRegistry > > - Replaces GadgetFeatureRegistry and JsFeatureLoader > > - Performs both register() and getFeaturesXXX() functions > > - Does NOT implicitly include "core*" APIs anymore! These are included > > in the dependency tree. @see GadgetSpec for more below. > > - register(...) > > + Registers one or more features in the system, incorporating them > > into the list of known features and building a validated dependency tree > > with them. If a circular dependency exists, an error is thrown. > > + Method may be called at ANY time -- the registry is not > > programmatically locked. This allows (though this use is not tested!) > > for dynamic feature injection at some later point. > > - getFeatureResources(...) > > + Queries the feature tree for resources matching the specified > > context (GADGET || CONTAINER), which satisfy all the needed dependencies > > given in the query. Optionally, fills in unknown/unsupported features > > from the needed list. > > - getFeatures(List<String>) > > + Returns feature strings in dependency order that satisfy the given > > needed features. > > > > FeatureResourceLoader > > - @Injected into FeatureRegistry, main extension point for feature > > system. > > - Loads a given FeatureResource from a feature.xml <script src="..."/> > > line (inline features are handled by FeatureRegistry). > > - Key API: load(URI, Map<String,String> attribs) > > + URI is pre-resolved to be absolute by FeatureParser (relative to > > parent, if a relative URI in feature.xml) > > + "attribs" allows for arbitrary context-sensitive extensions. Example > > to come soon: limiting by feature="..." to include Caja tamings.js > > per-feature only when Caja is available. > > > > FeatureParser > > - Mostly an implementation detail of FeatureRegistry for now > > (package-private). > > - Parses a String into an intermediary object for further processing. > > - Resolves URIs of Resources relative to parent. > > > > GadgetSpec > > - If no feature is explicitly <Optional> or <Require>'d starting with > > "core", automatically inserts the "core" feature for backward > > compatibility. > > - No way (in this CL) to include "absolutely no core" yet. > > > > Gadget > > - Modified how addFeature and removeFeature work. They now manage a > > "directlyRequestedDeps" list, which in turn is used by > > rendering/processing code for feature resolution. > > > > RenderingGadgetRewriter > > - Update injectFeatureLibraries(...) to use new API. > > - Hopefully more straightforward: obtain resources requested by the > > gadget, and resources specified externally. Subtract latter from the > > former and inline those. Extern the rest. No "core" assumptions, as > > "core" will be included by the GadgetSpec if needed. > > > > Core features > > - Break "core" into core.config, core.json, core.prefs et al. > > - "core" now an aggregating feature that has no JS of its own but > > includes all sub-libs. > > > > All other features > > - Specific assignment of dependencies on core libs. > > > > Other > > - feature.xml updates, pom.xml updates for separated core libs > > - test updates across the board for all affected classes > > > > > > Please review this at http://codereview.appspot.com/143046 > > > > Affected files: > > features/pom.xml > > features/src/main/javascript/features/analytics/feature.xml > > features/src/main/javascript/features/auth-refresh/feature.xml > > features/src/main/javascript/features/core.auth/feature.xml > > features/src/main/javascript/features/core.config/feature.xml > > features/src/main/javascript/features/core.io/feature.xml > > features/src/main/javascript/features/core.json/feature.xml > > features/src/main/javascript/features/core.legacy/feature.xml > > features/src/main/javascript/features/core.log/feature.xml > > features/src/main/javascript/features/core.prefs/feature.xml > > features/src/main/javascript/features/core.util/feature.xml > > features/src/main/javascript/features/core/auth-init.js > > features/src/main/javascript/features/core/auth.js > > features/src/main/javascript/features/core/config.js > > features/src/main/javascript/features/core/core.js > > features/src/main/javascript/features/core/feature.xml > > features/src/main/javascript/features/core/json.js > > features/src/main/javascript/features/core/legacy.js > > features/src/main/javascript/features/core/log.js > > features/src/main/javascript/features/core/prefs.js > > features/src/main/javascript/features/core/util.js > > features/src/main/javascript/features/features.txt > > features/src/main/javascript/features/flash/feature.xml > > features/src/main/javascript/features/i18n/feature.xml > > features/src/main/javascript/features/opensocial-0.6/feature.xml > > features/src/main/javascript/features/opensocial-current/feature.xml > > features/src/main/javascript/features/opensocial-data/feature.xml > > features/src/main/javascript/features/opensocial-jsonrpc/feature.xml > > features/src/main/javascript/features/opensocial-reference/feature.xml > > features/src/main/javascript/features/osapi/feature.xml > > features/src/main/javascript/features/rpc/feature.xml > > features/src/main/javascript/features/setprefs/feature.xml > > features/src/main/javascript/features/skins/feature.xml > > features/src/main/javascript/features/views/feature.xml > > java/common/src/main/java/org/apache/shindig/common/Pair.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/BrowserSpecificRpcJsFeatureLoader.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java > > java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java > > java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeature.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetFeatureRegistry.java > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/JsFeatureLoader.java > > java/gadgets/src/main/java/org/apache/shindig/gadgets/JsLibrary.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/features/BrowserSpecificFeatureResource.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureParser.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResource.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureResourceLoader.java > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/render/OpenSocialI18NGadgetRewriter.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java > > java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Feature.java > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java > > java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/Preload.java > > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/FlashTagHandler.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/BrowserSpecificRpcJsFeatureLoaderTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultUrlGeneratorTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetFeatureRegistryTest.java > > java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/HashLockedDomainServiceTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/JsFeatureLoaderTest.java > > java/gadgets/src/test/java/org/apache/shindig/gadgets/JsLibraryTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureParserTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureResourceLoaderTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/PipelineDataGadgetRewriterTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/TemplateRewriterTest.java > > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/tags/FlashTagHandlerTest.java > > > > > > >
