Make ModelLoader to release URLFeatcher once fetching is done.

Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

BUG=685235
Review-Url: https://codereview.chromium.org/2665653002
Cr-Commit-Position: refs/heads/master@{#447457}
Committed: https://chromium.googlesource.com/chromium/src/+/95948d4ee56a43c688ad9bc2975b1eff1376e2c0

[maksims (do not use this acc), 2017-01-30 11:38:30]

The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 11:38:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[maksims (do not use this acc), 2017-01-30 11:40:05]

Description was changed from

==========
Make ModelLoader to realease URLFeatcher once fetching is done.

This change makes URLFetcher to be realsed once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

BUG=685235
==========

to

==========
Make ModelLoader to realease URLFeatcher once fetching is done.

This change makes URLFetcher to be realsed once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

And the last - if auto update is disabled, |fetcher_| will
be destroyed and weak ptrs invalidated.

BUG=685235
==========

[maksims (do not use this acc), 2017-01-30 12:21:02]

maksim.sisov@intel.com changed reviewers:
+ mattm@chromium.org

[maksims (do not use this acc), 2017-01-30 12:21:02]

please review

[commit-bot: I haz the power, 2017-01-30 12:31:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 12:31:03]

Dry run: This issue passed the CQ dry run.

[mattm, 2017-01-30 20:23:13]

Also, description typos:
realease -> release
realsed -> released

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/client_side_model_loader.cc (right):

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:192: // Reset
|fetcher_| as long as it will be re-created on next fetch.
Wording is a little confusing, I'd remove the "as long", eg: "Reset |fetcher_|
as it will be re-created on next fetch."

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:202: CancelFetcher();
This seems unnecessary, can you explain when this would matter?

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:215: if (fetcher_)
unnecessary

[maksims (do not use this acc), 2017-01-31 05:49:52]

Description was changed from

==========
Make ModelLoader to realease URLFeatcher once fetching is done.

This change makes URLFetcher to be realsed once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

And the last - if auto update is disabled, |fetcher_| will
be destroyed and weak ptrs invalidated.

BUG=685235
==========

to

==========
Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

And the last - if auto update is disabled, |fetcher_| will
be destroyed and weak ptrs invalidated.

BUG=685235
==========

[maksims (do not use this acc), 2017-01-31 05:50:22]

Description was changed from

==========
Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

And the last - if auto update is disabled, |fetcher_| will
be destroyed and weak ptrs invalidated.

BUG=685235
==========

to

==========
Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

BUG=685235
==========

[maksims (do not use this acc), 2017-01-31 05:50:28]

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/client_side_model_loader.cc (right):

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:192: // Reset
|fetcher_| as long as it will be re-created on next fetch.
On 2017/01/30 20:23:13, mattm wrote:
> Wording is a little confusing, I'd remove the "as long", eg: "Reset |fetcher_|
> as it will be re-created on next fetch."

Done.

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:202: CancelFetcher();
On 2017/01/30 20:23:13, mattm wrote:
> This seems unnecessary, can you explain when this would matter?

I thought it would be necessary if fetch had been scheduled before switch became
disabled. But it requires restart of the browser.

https://codereview.chromium.org/2665653002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/client_side_model_loader.cc:215: if (fetcher_)
On 2017/01/30 20:23:13, mattm wrote:
> unnecessary

Done.

[maksims (do not use this acc), 2017-01-31 05:50:37]

The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 05:50:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 06:47:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 06:47:15]

Dry run: This issue passed the CQ dry run.

[mattm, 2017-02-01 01:47:18]

lgtm

[maksims (do not use this acc), 2017-02-01 05:27:55]

The CQ bit was checked by maksim.sisov@intel.com

[commit-bot: I haz the power, 2017-02-01 05:28:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 05:43:59]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485926875430340,
"parent_rev": "a1c31e95c84908154745bcf70becc43243a13f11", "commit_rev":
"95948d4ee56a43c688ad9bc2975b1eff1376e2c0"}

[commit-bot: I haz the power, 2017-02-01 05:47:50]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485926875430340,
"parent_rev": "a1c31e95c84908154745bcf70becc43243a13f11", "commit_rev":
"95948d4ee56a43c688ad9bc2975b1eff1376e2c0"}

[commit-bot: I haz the power, 2017-02-01 05:47:56]

Description was changed from

==========
Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

BUG=685235
==========

to

==========
Make ModelLoader to release URLFeatcher once fetching is done.

This change makes URLFetcher to be released once fetching is done.
It is not needed because new fetcher is created each new fetch.
What is more, old fetcher can be kept for 1 hour (kClientModelFetchIntervalMs)
and just waste resources.

BUG=685235

Review-Url: https://codereview.chromium.org/2665653002
Cr-Commit-Position: refs/heads/master@{#447457}
Committed:
https://chromium.googlesource.com/chromium/src/+/95948d4ee56a43c688ad9bc2975b...
==========

[commit-bot: I haz the power, 2017-02-01 05:47:57]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/95948d4ee56a43c688ad9bc2975b...