Clean up cr.ui.pageManager.Page prototype

BUG=313244
R=dbeam@chromium.org

Review URL: https://codereview.chromium.org/514503002

Cr-Commit-Position: refs/heads/master@{#295628}

chrome/browser/resources/options/chromeos/change_picture_options.js

@@ -104,9 +104,7 @@ cr.define('options', function() {
       chrome.send('onChangePicturePageInitialized');
     },
 
-    /**
-     * Called right after the page has been shown to user.
-     */
+    /** @override */
     didShowPage: function() {
       var imageGrid = $('user-image-grid');
       // Reset camera element.
@@ -115,9 +113,7 @@ cr.define('options', function() {
       chrome.send('onChangePicturePageShown');
     },
 
-    /**
-     * Called right before the page is hidden.
-     */
+    /** @override */
     willHidePage: function() {
       var imageGrid = $('user-image-grid');
       imageGrid.blur();  // Make sure the image grid is not active.
@@ -130,10 +126,10 @@ cr.define('options', function() {
     },
 
     /**
-     * Called right after the page has been hidden.
+     * Either willHidePage or didClosePage may be called depending on the way
+     * the page was closed.
+     * @override
      */
-    // TODO(ivankr): both callbacks are required as only one of them is called
-    // depending on the way the page was closed, see http://crbug.com/118923.
     didClosePage: function() {
       this.willHidePage();
     },

chrome/browser/resources/options/font_settings.js

@@ -66,9 +66,7 @@ cr.define('options', function() {
       };
     },
 
-    /**
-     * Called by the options page when this page has been shown.
-     */
+    /** @override */
     didShowPage: function() {
       // The fonts list may be large so we only load it when this page is
       // loaded for the first time.  This makes opening the options window

chrome/browser/resources/options/search_page.js

@@ -164,13 +164,11 @@ cr.define('options', function() {
       return true;
     },
 
-    /**
-     * Called after this page has shown.
-     */
+    /** @override */
     didShowPage: function() {
-      // This method is called by the Options page after all pages have
-      // had their visibilty attribute set.  At this point we can perform the
-      // search specific DOM manipulation.
+      // This method is called by the PageManager after all pages have had their
+      // visibility attribute set. At this point we can perform the
+      // search-specific DOM manipulation.
       this.setSearchActive_(true);
     },
 
@@ -179,14 +177,11 @@ cr.define('options', function() {
       this.setSearchActive_(true);
     },
 
-    /**
-     * Called before this page will be hidden.
-     */
+    /** @override */
     willHidePage: function() {
-      // This method is called by the Options page before all pages have
-      // their visibilty attribute set.  Before that happens, we need to
-      // undo the search specific DOM manipulation that was performed in
-      // didShowPage.
+      // This method is called by the PageManager before all pages have their
+      // visibility attribute set. Before that happens, we need to undo the
+      // search-specific DOM manipulation that was performed in didShowPage.
       this.setSearchActive_(false);
     },
 

ui/webui/resources/js/cr/ui/page_manager/page.js

@@ -137,6 +137,22 @@ cr.define('cr.ui.pageManager', function() {
       PageManager.onPageHashChanged(this);
     },
 
+    /**
+     * Called after the page has been shown.
+     */
+    didShowPage: function() {},
+
+    /**
+     * Called before the page will be hidden, e.g., when a different root page
+     * will be shown.
+     */
+    willHidePage: function() {},
+
+    /**
+     * Called after the overlay has been closed.
+     */
+    didClosePage: function() {},
+
     /**
      * Gets the container div for this page if it is an overlay.
      * @type {HTMLDivElement}

ui/webui/resources/js/cr/ui/page_manager/page_manager.js

@@ -185,10 +185,8 @@ cr.define('cr.ui.pageManager', function() {
 
       // Notify pages if they will be hidden.
       this.forEachPage_(!isRootPageLocked, function(page) {
-        if (page.willHidePage && page.name != pageName &&
-            !this.isAncestorOfPage(page, targetPage)) {
+        if (page.name != pageName && !this.isAncestorOfPage(page, targetPage))
           page.willHidePage();
-        }
       });
 
       // Update the page's hash.
@@ -213,7 +211,7 @@ cr.define('cr.ui.pageManager', function() {
 
       // Notify pages if they were shown.
       this.forEachPage_(!isRootPageLocked, function(page) {
-        if (!targetPageWasVisible && page.didShowPage &&
+        if (!targetPageWasVisible &&
             (page.name == pageName ||
              this.isAncestorOfPage(page, targetPage))) {
           page.didShowPage();
@@ -333,9 +331,8 @@ cr.define('cr.ui.pageManager', function() {
         return;
 
       overlay.visible = false;
+      overlay.didClosePage();
 
-      if (overlay.didClosePage)
-        overlay.didClosePage();
       this.updateHistoryState_(false);
       this.updateTitle_();
 
@@ -422,7 +419,7 @@ cr.define('cr.ui.pageManager', function() {
                     this.defaultPage_;
       if (currentOverlay && !this.isAncestorOfPage(currentOverlay, newPage)) {
         currentOverlay.visible = false;
-        if (currentOverlay.didClosePage) currentOverlay.didClosePage();
+        currentOverlay.didClosePage();
       }
       this.showPageByName(pageName, false, {hash: hash});
     },
@@ -442,7 +439,7 @@ cr.define('cr.ui.pageManager', function() {
      */
     willClose: function() {
       var overlay = this.getVisibleOverlay_();
-      if (overlay && overlay.didClosePage)
+      if (overlay)
         overlay.didClosePage();
     },
 
@@ -500,8 +497,7 @@ cr.define('cr.ui.pageManager', function() {
       overlay.hash = hash;
       if (!overlay.visible) {
         overlay.visible = true;
-        if (overlay.didShowPage)
-          overlay.didShowPage();
+        overlay.didShowPage();
       } else {
         overlay.didChangeHash();
       }