Update access info when an offline page is being visited

chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java
index 67fe59f1eae066ff679018866b6f2c3362815316..72f060e2cf3faaa980dbfb4476044851311e8648 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeTest.java
@@ -18,6 +18,8 @@ import org.chromium.components.bookmarks.BookmarkId;
 import org.chromium.components.bookmarks.BookmarkType;
 import org.chromium.components.offlinepages.DeletePageResult;
 import org.chromium.components.offlinepages.SavePageResult;
+import org.chromium.content.browser.test.util.Criteria;
+import org.chromium.content.browser.test.util.CriteriaHelper;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -92,6 +94,17 @@ public class OfflinePageBridgeTest extends ChromeActivityTestCaseBase<ChromeActi
     }
 
     @MediumTest
+    public void testMarkPageAccessed() throws Exception {
+        loadUrl(TEST_PAGE);
+        savePage(SavePageResult.SUCCESS, TEST_PAGE);
+        OfflinePageItem offlinePage = mOfflinePageBridge.getPageByBookmarkId(BOOKMARK_ID);
+        assertNotNull("Offline page should be available, but it is not.", offlinePage);
+        assertEquals("Offline page access count should be 0.", 0, offlinePage.getAccessCount());
+
+        markPageAccessed(BOOKMARK_ID, 1);
+    }
+
+    @MediumTest
     public void testGetPageByBookmarkId() throws Exception {
         loadUrl(TEST_PAGE);
         savePage(SavePageResult.SUCCESS, TEST_PAGE);
@@ -148,6 +161,27 @@ public class OfflinePageBridgeTest extends ChromeActivityTestCaseBase<ChromeActi
         assertTrue(semaphore.tryAcquire(TIMEOUT_MS, TimeUnit.MILLISECONDS));
     }
 
+    private void markPageAccessed(final BookmarkId bookmarkId, final int expectedAccessCount)
+            throws InterruptedException {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mOfflinePageBridge.markPageAccessed(bookmarkId);
+                try {

[newt (away), 2015/09/23 21:52:56]

lines 170-180 should be outside of the inner class Runnable. e.g.

  ThreadUtils.runOnUiThreadBlocking(new Runnable() {
     ...
  });
  assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
     ...
  });

since you can't poll the UI thread from inside the UI thread.

But actually, in this case, it looks like polling isn't needed at all. The
access count will be updated synchronously, right? So you could just write:

  ThreadUtils.runOnUiThreadBlocking(new Runnable() {
     ...
     mOfflinePageBridge.markPageAccessed(bookmarkId);
     OfflinePageItem offlinePage = ...
     assertEquals(expectedAccessCount, offlinePage.getAccessCount());
  });

[jianli, 2015/09/23 23:21:07]

The access count should not be updated synchronously. We should only update the
cache upon successful store write. I have found out this problem when I was
working on making markPageAccessed test work but I reverted this fix when I
reverted the temporary. I  logging change. I now bring it back in
offline_page_model.cc. I changed to pull the polling part out.

+                    CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
+                        @Override
+                        public boolean isSatisfied() {
+                            OfflinePageItem offlinePage =
+                                    mOfflinePageBridge.getPageByBookmarkId(bookmarkId);
+                            return offlinePage.getAccessCount() == expectedAccessCount;
+                        }
+                    });
+                } catch (InterruptedException e) {
+                }
+            }
+        });
+    }
+
     private void deletePage(BookmarkId bookmarkId, final int expectedResult)
             throws InterruptedException {
         final Semaphore semaphore = new Semaphore(0);