Add methods for cache_temperature: ClearCache, WarmCache

telemetry/telemetry/page/cache_temperature.py

Index: telemetry/telemetry/page/cache_temperature.py
diff --git a/telemetry/telemetry/page/cache_temperature.py b/telemetry/telemetry/page/cache_temperature.py
index 749ab864149ba1460d56e85b5ea6c352d785d299..1937da07ee041db7f71a4546d80f372a3fb2b0c4 100644
--- a/telemetry/telemetry/page/cache_temperature.py
+++ b/telemetry/telemetry/page/cache_temperature.py
@@ -54,37 +54,43 @@ class MarkTelemetryInternal(object):
         "console.timeEnd({{ marker }});", marker=marker)
     return True
 
+def ClearCache(browser):
+  with MarkTelemetryInternal(browser, 'clear_cache'):
+    any_tab = browser.tabs[0]

[shimazu, 2017/09/19 09:37:31]

nit: |tab| seems enough.

[yukiy, 2017/09/21 08:23:44]

Done.

+    any_tab.ClearCache(force=True)
+
+def AvoidDoubleHashNavigation(page, browser):
+  if '#' in page.url:

[shimazu, 2017/09/19 09:37:31]

nit: The opposite condition might be better to reduce indent.

if not '#' in page.url:
  return

[kouhei (in TOK), 2017/09/19 09:38:51]

Would you mind keeping AvoidDoubleHashNavigation as is?
Having this here in EnsurePageCacheTemperature is a *hack* and I think this
should ideally live in tab.py Navigate. Factoring out feels like making the hack
permanent.

[nednguyen, 2017/09/19 09:50:05]

+1. If the purpose is to make the EnsurePageCacheTemperature code not too
clustered, I would suggest refactor as follow:


class CacheManipulator(object):
    TEMPERATURE = None
    def  PrepareCache(self, previous_page, page, browser):
       raise UnimplementedError

Then you can subclass: AnyCache(CacheManipulator), ColdCache(CacheManipulator),
WarmCache(CacheManipulator), HotCache(CacheManipulator).

The code in EnsurePageCacheTemperature then would be:

 def EnsurePageCacheTemperature(page, browser, previous_page=None):
     for c in all_cache_manipulators:
        if page.temperature == c.TEMPERATURE: 
            c.PrepareCache(previous_page, page, browser)
            return 
     raise UnimplementedError('Unrecognized cache temperature %s' %
page.temperature)

This way, you rely on polymorphism to branch the cache handling logic instead of
if-else.

[yukiy, 2017/09/21 08:23:44]

Done.

[yukiy, 2017/09/21 08:23:44]

I chatted with kouhei@, and decided not to do this now (because my internship
will end at 09/29/2017, sadly).
This will be done in follow-up CLs.

[yukiy, 2017/09/21 08:23:44]

Acknowledged.

+    # Navigate to inexistent URL to avoid in-page hash navigation.
+    # Note: Unlike PCv1, PCv2 iterates the same URL for different cache
+    #       configurations. This may issue blink in-page hash navigations,
+    #       which isn't intended here.
+    with MarkTelemetryInternal(browser, 'avoid_double_hash_navigation'):
+      tab = browser.tabs[0]
+      tab.Navigate("http://does.not.exist")
+      tab.WaitForDocumentReadyStateToBeComplete()
+
+def WarmCache(page, browser):
+  with MarkTelemetryInternal(browser, 'warm_cache'):
+    tab = browser.tabs[0]
+    tab.Navigate(page.url)
+    py_utils.WaitFor(tab.HasReachedQuiescence, 60)
+    tab.WaitForDocumentReadyStateToBeComplete()
+    tab.Navigate("about:blank")
+    tab.WaitForDocumentReadyStateToBeComplete()
+
 def EnsurePageCacheTemperature(page, browser, previous_page=None):
   temperature = page.cache_temperature
   logging.info('PageCacheTemperature: %s', temperature)
-
   if temperature == ANY:
     return
-
   if temperature == COLD:
-    with MarkTelemetryInternal(browser, 'clear_cache'):
-      any_tab = browser.tabs[0]
-      any_tab.ClearCache(force=True)
+    ClearCache(browser)
   elif temperature == WARM:
     if (previous_page is not None and
         previous_page.url == page.url and
         (previous_page.cache_temperature == COLD or
          previous_page.cache_temperature == WARM)):
-      if '#' in page.url:
-        # Navigate to inexistent URL to avoid in-page hash navigation.
-        # Note: Unlike PCv1, PCv2 iterates the same URL for different cache
-        #       configurations. This may issue blink in-page hash navigations,
-        #       which isn't intended here.
-        with MarkTelemetryInternal(browser, 'avoid_double_hash_navigation'):
-          tab = browser.tabs[0]
-          tab.Navigate("http://does.not.exist")
-          tab.WaitForDocumentReadyStateToBeComplete()
+      AvoidDoubleHashNavigation(page, browser)
       return
-
-    with MarkTelemetryInternal(browser, 'warm_cache'):
-      tab = browser.tabs[0]
-      tab.Navigate(page.url)
-      py_utils.WaitFor(tab.HasReachedQuiescence, 60)
-      tab.WaitForDocumentReadyStateToBeComplete()
-      tab.Navigate("about:blank")
-      tab.WaitForDocumentReadyStateToBeComplete()
+    WarmCache(page, browser)