#setOwner should rewrite resource.owner_ value if it exist (#5898)

* fix * add test * add instance method to update owner value
ampproject · Oct 31, 2016 · 79a4253 · 79a4253
1 parent c723386
commit 79a4253
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 0 deletions.
diff --git a/src/service/resource.js b/src/service/resource.js
@@ -99,6 +99,9 @@ export class Resource {
    */
   static setOwner(element, owner) {
     dev().assert(owner.contains(element), 'Owner must contain the element');
+    if (Resource.forElementOptional(element)) {
+      Resource.forElementOptional(element).updateOwner(owner);
+    }
     element[OWNER_PROP_] = owner;
   }
 
@@ -185,6 +188,14 @@ export class Resource {
     return this.id_;
   }
 
+  /**
+   * Update owner element
+   * @param {!AmpElement} owner
+   */
+  updateOwner(owner) {
+    this.owner_ = owner;
+  }
+
   /**
    * Returns an owner element or null.
    * @return {?AmpElement}

diff --git a/test/functional/test-resource.js b/test/functional/test-resource.js
@@ -513,6 +513,41 @@ describe('Resource', () => {
     });
   });
 
+  describe('Resource set/get ownership', () => {
+    let child;
+    let parentResource;
+    let resources;
+    beforeEach(() => {
+      const parent = {
+        ownerDocument: {defaultView: window},
+        tagName: 'AMP-STICKY-AD',
+        isBuilt: () => false,
+        contains: () => true,
+      };
+      child = {
+        ownerDocument: {defaultView: window},
+        tagName: 'AMP-AD',
+        isBuilt: () => false,
+        contains: () => true,
+      };
+      resources = new Resources(new AmpDocSingle(window));
+      parentResource = new Resource(1, parent, resources);
+    });
+
+    it('should set resource before Resource created for child element', () => {
+      resources.setOwner(child, parentResource.element);
+      const childResource = new Resource(1, child, resources);
+      expect(childResource.getOwner()).to.equal(parentResource.element);
+    });
+
+    it('should always get the lastest owner value', () => {
+      const childResource = new Resource(1, child, resources);
+      expect(childResource.getOwner()).to.be.null;
+      resources.setOwner(childResource.element, parentResource.element);
+      expect(childResource.owner_).to.equal(parentResource.element);
+      expect(childResource.getOwner()).to.equal(parentResource.element);
+    });
+  });
 
   describe('unlayoutCallback', () => {
     it('should NOT call unlayoutCallback on unbuilt element', () => {