[Offline pages] Use the new policy bits

components/offline_pages/offline_page_model_impl.h

Index: components/offline_pages/offline_page_model_impl.h
diff --git a/components/offline_pages/offline_page_model_impl.h b/components/offline_pages/offline_page_model_impl.h
index 521893111dadb3ccd1694dfcfdf25f215b43179c..ea36b27a06f97fbe80f70cada98c8fefc9f3cff5 100644
--- a/components/offline_pages/offline_page_model_impl.h
+++ b/components/offline_pages/offline_page_model_impl.h
@@ -259,7 +259,7 @@ class OfflinePageModelImpl : public OfflinePageModel, public KeyedService {
   void PostClearStorageIfNeededTask();
 
   // Check if |offline_page| is user-requested.

[romax, 2016/09/19 23:00:24]

I think the function name change would also affect the comment :)
And would 'WillBe*' be better here? I'm bad at naming and I'm fine if it's okay
to others :)

[chili, 2016/09/20 00:02:33]

Updated comment.

I'm inclined to keep the name as is, to be parallel with the naming in the
ClientPolicyController... 

-  bool IsUserRequestedPage(const OfflinePageItem& offline_page) const;
+  bool IsRemovedOnCacheReset(const OfflinePageItem& offline_page) const;

[Pete Williamson, 2016/09/20 00:01:59]

Drive-by:

Why rename the function?  While the concept of whether a page is user-requested
or not is not that important at the offline page model level, it is an important
concept to the background offliner, which treats user-requested pages
differently (with higher priority).  If we rename it, I'm a bit concerned that
somewhere we will be passing the value (is_removed_on_cache_reset) to a function
taking (is_user_requested).

I do understand that on the policy level, the only place that offline page model
interacts with user requested pages is about whether they should be removed on
cache reset.

[chili, 2016/09/20 00:06:21]

This came from comment on the original policy-bit CL
(https://codereview.chromium.org/2289143005/diff/1/components/offline_pages/cl...)
But perhaps the solution is to add another bit for user requested...

[romax, 2016/09/20 00:19:01]

It seems that we can get rid of this method and use the policy check (the new
implementation) where this method is called? The intention was not to write the
namespace check at multiple places. Would it be better if we remove this helper
function and use the policy_controller_->* here?

 
   void RunWhenLoaded(const base::Closure& job);