Avoid starting a distillation process if the URL is not distillable.

Avoid starting a distillation process if the URL is not distillable.

The distillation does not need to start in this cases, the data
source is going to fallback to the error webpage.

Committed: https://crrev.com/4788d01b26d8b2c24967436cc5f9b95189b5f172
Cr-Commit-Position: refs/heads/master@{#321328}

[alexg__, 2015-03-13 18:40:20]

alex@igalia.com changed reviewers:
+ nyquist@chromium.org

[nyquist, 2015-03-13 21:21:20]

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
chrome/browser/dom_distiller/tab_utils.cc:115: if
(!dom_distiller::url_utils::IsUrlDistillable(last_committed_url)) {
If the distillation is not started here, wouldn't it then just be started from
the navigation that has already started?

[alexg__, 2015-03-13 21:34:22]

On 2015/03/13 21:21:20, nyquist wrote:
>
https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
> File chrome/browser/dom_distiller/tab_utils.cc (right):
> 
>
https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
> chrome/browser/dom_distiller/tab_utils.cc:115: if
> (!dom_distiller::url_utils::IsUrlDistillable(last_committed_url)) {
> If the distillation is not started here, wouldn't it then just be started from
> the navigation that has already started?

Yep, and in this case it would basically load a the failed webpage which is an
static resource, so it does not seem and advantage to launch this process before
and it wastes some resources.

[nyquist, 2015-03-13 23:46:08]

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
chrome/browser/dom_distiller/tab_utils.cc:115: if
(!dom_distiller::url_utils::IsUrlDistillable(last_committed_url)) {
On 2015/03/13 at 21:21:20, nyquist wrote:
> If the distillation is not started here, wouldn't it then just be started from
the navigation that has already started?

Please remove the curly braces.

[nyquist, 2015-03-14 00:49:58]

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/1003153003/diff/1/chrome/browser/dom_distille...
chrome/browser/dom_distiller/tab_utils.cc:115: if
(!dom_distiller::url_utils::IsUrlDistillable(last_committed_url)) {
On 2015/03/13 at 23:46:08, nyquist wrote:
> On 2015/03/13 at 21:21:20, nyquist wrote:
> > If the distillation is not started here, wouldn't it then just be started
from the navigation that has already started?
> 
> Please remove the curly braces.

Also; I guess at that point we would have to rename the method to
MaybeStartDistillation.

[alexg__, 2015-03-16 16:37:14]

On 2015/03/14 00:49:58, nyquist wrote:
> Also; I guess at that point we would have to rename the method to
> MaybeStartDistillation.

Thanks for the comments, just uploaded a new version with your proposals and
releasing the web_contents created in the previous function in this early
return.

[nyquist, 2015-03-19 00:42:51]

lgtm

[alexg__, 2015-03-19 09:20:04]

The CQ bit was checked by alex@igalia.com

[commit-bot: I haz the power, 2015-03-19 09:21:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1003153003/20001

[commit-bot: I haz the power, 2015-03-19 10:22:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-19 10:24:08]

Patchset 2 (id:??) landed as
https://crrev.com/4788d01b26d8b2c24967436cc5f9b95189b5f172
Cr-Commit-Position: refs/heads/master@{#321328}