New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 715525 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 699079



Sign in to add a comment

Glowplug: Refactor resource_prefetch_predictor to align with the expanded scope

Project Member Reported by lizeb@chromium.org, Apr 26 2017

Issue description

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8

commit 6175b0a9f10d288f663f88c871c88ca3b5f0b9e8
Author: alexilin <alexilin@chromium.org>
Date: Fri Apr 28 17:34:40 2017

predictors: Extract sql key-value tables into separate class.

This is the first step of Glowplug Database refactoring. As a result we should
get rid of boilerplate related to adding/removing tables from Glowplug.
The next step will be providing users the direct access to tables instead of
using Update{Resource,Redirect,etc.}Data wrappers.
The code uses templates instead of google::protobuf::MessageLite for the
type-safety.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2851473002
Cr-Commit-Position: refs/heads/master@{#468046}

[modify] https://crrev.com/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8/chrome/browser/BUILD.gn
[add] https://crrev.com/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8/chrome/browser/predictors/glowplug_key_value_table.cc
[add] https://crrev.com/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8/chrome/browser/predictors/glowplug_key_value_table.h
[modify] https://crrev.com/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8/chrome/browser/predictors/resource_prefetch_predictor_tables.cc
[modify] https://crrev.com/6175b0a9f10d288f663f88c871c88ca3b5f0b9e8/chrome/browser/predictors/resource_prefetch_predictor_tables.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d999f0f298065398ec9bbc156c177892d74d17f6

commit d999f0f298065398ec9bbc156c177892d74d17f6
Author: lizeb <lizeb@chromium.org>
Date: Mon May 15 10:23:55 2017

predictors: Introduce LoadingPredictor.

This is the first step of the Glowplug refactoring. It extracts the
external entry points of Glowplug, and starts to separate the resource
prefetch specific parts from the rest.

This CL mostly adds the top-level Glowplug classes, and there is no
behavior change. This will make further refactoring easier. The Android
side client code will be updated in a forthcoming CL.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2847183002
Cr-Commit-Position: refs/heads/master@{#471718}

[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/BUILD.gn
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/precache/precache_manager_factory.cc
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor.cc
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor.h
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor_config.cc
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor_config.h
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor_factory.cc
[add] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/loading_predictor_factory.h
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/predictor_database.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_common.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_common.h
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_common_unittest.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor_android.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[delete] https://crrev.com/6e6b3251a1e8291b81058d51d168f8ce377b8eb3/chrome/browser/predictors/resource_prefetch_predictor_factory.cc
[delete] https://crrev.com/6e6b3251a1e8291b81058d51d168f8ce377b8eb3/chrome/browser/predictors/resource_prefetch_predictor_factory.h
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetcher_manager.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/predictors/resource_prefetcher_manager.h
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/ui/webui/predictors/predictors_handler.cc
[modify] https://crrev.com/d999f0f298065398ec9bbc156c177892d74d17f6/chrome/browser/ui/webui/predictors/predictors_handler.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfe486d5448e3b37f38c7f854c0f284cff8850fa

commit cfe486d5448e3b37f38c7f854c0f284cff8850fa
Author: lizeb <lizeb@chromium.org>
Date: Mon May 15 15:53:33 2017

android: move ResourcePrefetchPredictor usage to LoadingPredictor.

Following the native side refactoring (crrev.com/2847183002), update the
android code entry points to LoadingPredictor.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2879103003
Cr-Commit-Position: refs/heads/master@{#471776}

[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
[add] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/android/java/src/org/chromium/chrome/browser/customtabs/LoadingPredictor.java
[delete] https://crrev.com/fbaa170f914ca9eb0c75f9bd9d589d0a1d6f548c/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ResourcePrefetchPredictor.java
[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/android/java_sources.gni
[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/BUILD.gn
[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/predictors/loading_predictor.h
[rename] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/predictors/loading_predictor_android.cc
[add] https://crrev.com/cfe486d5448e3b37f38c7f854c0f284cff8850fa/chrome/browser/predictors/loading_predictor_android.h
[delete] https://crrev.com/fbaa170f914ca9eb0c75f9bd9d589d0a1d6f548c/chrome/browser/predictors/resource_prefetch_predictor_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb0f08a8b0768450acac29a5230cc2202fc89077

commit eb0f08a8b0768450acac29a5230cc2202fc89077
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue May 30 12:32:55 2017

predictors: Refactor Glowplug Database.

All Glowplug tables have the same structure. A table has two columns
representing string as a key and protobuf message as a value.

The goal of this refactor is to ease adding new tables in Glowplug and reduce
amount of the code. This CL also makes easier a replacement of database backend.

Bug:  715525 
Change-Id: Idc16a81b91b3d9576454839bceaa602db4b3f354
Reviewed-on: https://chromium-review.googlesource.com/506087
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475500}
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/BUILD.gn
[add] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/glowplug_key_value_data.h
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/glowplug_key_value_table.h
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor_tables.cc
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor_tables.h
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/ui/webui/predictors/predictors_handler.cc
[modify] https://crrev.com/eb0f08a8b0768450acac29a5230cc2202fc89077/chrome/browser/ui/webui/predictors/predictors_handler.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b

commit 46c85c0e2c60c88ed671ea707bebd3d35bc4f28b
Author: lizeb <lizeb@chromium.org>
Date: Thu Jun 01 16:02:31 2017

predictors: Refactor resource_prefetch_predictor triggering.

Triggering and policy move to LoadingPredictor, instead of being in
ResourcePrefetchPredictor. As a consequence, rename
ResourcePrefetchPredictorObserver.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2887133003
Cr-Commit-Position: refs/heads/master@{#476313}

[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/BUILD.gn
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[rename] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/net/loading_predictor_observer.cc
[rename] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/net/loading_predictor_observer.h
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/loading_predictor.h
[add] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetch_predictor_test_util.h
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/predictors/resource_prefetcher_manager.h
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/46c85c0e2c60c88ed671ea707bebd3d35bc4f28b/chrome/test/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/961bcec3f6219054b3032cec64c4958f3e43aca2

commit 961bcec3f6219054b3032cec64c4958f3e43aca2
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Jun 02 15:52:15 2017

predictors: Refactor resource_prefetch_predictor stats collection.

Extract a new class that collects data from different LoadingPredictor
components and reports histograms.

Bug:  715525 
Change-Id: I821897b4e5092b5f14f9e350dac063a1607ff534
Reviewed-on: https://chromium-review.googlesource.com/521146
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476676}
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/BUILD.gn
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_predictor.h
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_predictor_unittest.cc
[add] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_stats_collector.cc
[add] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_stats_collector.h
[add] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/loading_stats_collector_unittest.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor_test_util.h
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/961bcec3f6219054b3032cec64c4958f3e43aca2/chrome/test/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3eb67a8404b818203d0498faf86438681c4a6c97

commit 3eb67a8404b818203d0498faf86438681c4a6c97
Author: trevordixon <trevordixon@chromium.org>
Date: Fri Jun 09 12:21:32 2017

predictors: Create LoadingDataCollector class and have observers rely on it instead of ResourcePrefetchPredictor.

Part of a refactoring effort to split ResourcePrefetchPredictor.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2896713003
Cr-Commit-Position: refs/heads/master@{#478254}

[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/BUILD.gn
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/net/loading_predictor_observer.cc
[rename] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.cc
[rename] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer.h
[rename] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[add] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_data_collector.cc
[add] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_data_collector.h
[add] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_data_collector_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_predictor.h
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_stats_collector_unittest.cc
[rename] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_test_util.cc
[rename] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/loading_test_util.h
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor_tables_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/browser/predictors/resource_prefetcher_unittest.cc
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/chrome/test/BUILD.gn
[modify] https://crrev.com/3eb67a8404b818203d0498faf86438681c4a6c97/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2565454f397846a83afc020c3fbd1d31766e03a

commit b2565454f397846a83afc020c3fbd1d31766e03a
Author: trevordixon <trevordixon@chromium.org>
Date: Tue Jun 13 14:09:20 2017

predictors: Improve comment clarity and remove unecessary namespace specifiers.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2933133003
Cr-Commit-Position: refs/heads/master@{#479004}

[modify] https://crrev.com/b2565454f397846a83afc020c3fbd1d31766e03a/chrome/browser/predictors/loading_data_collector.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce400eae248cb5725d8ae44672a19b5371b8d49f

commit ce400eae248cb5725d8ae44672a19b5371b8d49f
Author: lizeb <lizeb@chromium.org>
Date: Tue Jul 04 08:12:07 2017

predictors: move ResourcePrefetcher handling to LoadingPredictor.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2928033003
Cr-Commit-Position: refs/heads/master@{#484064}

[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/BUILD.gn
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/loading_predictor.h
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetcher.cc
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetcher.h
[delete] https://crrev.com/08ff97980714bbfb42d9b0d50ac01feabce3e500/chrome/browser/predictors/resource_prefetcher_manager.cc
[delete] https://crrev.com/08ff97980714bbfb42d9b0d50ac01feabce3e500/chrome/browser/predictors/resource_prefetcher_manager.h
[modify] https://crrev.com/ce400eae248cb5725d8ae44672a19b5371b8d49f/chrome/browser/predictors/resource_prefetcher_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d87657cd9251e9c60a1c8df464df6d032e91530c

commit d87657cd9251e9c60a1c8df464df6d032e91530c
Author: trevordixon <trevordixon@chromium.org>
Date: Thu Jul 13 09:26:48 2017

predictors: Move more methods from ResourcePrefetchPredictor into LoadingDataCollector.

BUG= 715525 

Review-Url: https://codereview.chromium.org/2937623007
Cr-Commit-Position: refs/heads/master@{#486325}

[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/net/loading_predictor_observer.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/net/loading_predictor_observer.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/page_load_metrics/observers/loading_predictor_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_data_collector.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_data_collector.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_data_collector_unittest.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_predictor.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_stats_collector.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_stats_collector.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_stats_collector_unittest.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_test_util.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/loading_test_util.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[modify] https://crrev.com/d87657cd9251e9c60a1c8df464df6d032e91530c/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc

Comment 13 by pasko@chromium.org, Jul 17 2017

Summary: Glowplug: Refactor resource_prefetch_predictor to align with the expanded scope (was: Glowplug: Refactor resource_prefetch_predictor to align with the expended scope)
can the doc be made public?
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c36f566d215f2bf8d4ee29567dfa91a85c583d7e

commit c36f566d215f2bf8d4ee29567dfa91a85c583d7e
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Mon Jul 17 18:26:50 2017

predictors: Remove unused pointer from ResourcePrefetchPredictor

ResourcePrefetchPredictor doesn't use LoadingStatsCollector since the CL
https://crrev.com/2937623007 was landed.

Bug:  715525 
Change-Id: Ic16227b20fe2f3dfbd2b28e662a28f9339cc816e
Reviewed-on: https://chromium-review.googlesource.com/574606
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487167}
[modify] https://crrev.com/c36f566d215f2bf8d4ee29567dfa91a85c583d7e/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/c36f566d215f2bf8d4ee29567dfa91a85c583d7e/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/c36f566d215f2bf8d4ee29567dfa91a85c583d7e/chrome/browser/predictors/resource_prefetch_predictor.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b15c2fb0d128f0e7f384763275d9a0e55b67c29f

commit b15c2fb0d128f0e7f384763275d9a0e55b67c29f
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue Jul 18 14:26:29 2017

predictors: Fix WebUI crash due to the lazy initialization.

Navigation to chrome://predictors cause a crash if it's the first
navigation after profile creation. The reason is that ResourcePrefetchPredictor
is initialized only after the first commited navigation excluding a navigation
to NTP. On the other hand, WebUI predictors handler doesn't check that the
predictor was initialized before accessing the field that is nullptr in
uninitialized state.

This CL turns on the initialization of ResourcePrefetchPredictor after the
navigation to NTP and adds a check for a uninitialized state in WebUI handler.

Bug:  715525 
Change-Id: I9382e892ce713d2b0beb406be344df64e2d5dd3d
Reviewed-on: https://chromium-review.googlesource.com/575989
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487465}
[modify] https://crrev.com/b15c2fb0d128f0e7f384763275d9a0e55b67c29f/chrome/browser/predictors/loading_data_collector.cc
[modify] https://crrev.com/b15c2fb0d128f0e7f384763275d9a0e55b67c29f/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/b15c2fb0d128f0e7f384763275d9a0e55b67c29f/chrome/browser/predictors/loading_test_util.h
[modify] https://crrev.com/b15c2fb0d128f0e7f384763275d9a0e55b67c29f/chrome/browser/ui/webui/predictors/predictors_handler.cc

Comment 16 by lizeb@chromium.org, Aug 18 2017

Status: Fixed (was: Assigned)

Sign in to add a comment