[translate] Fix shutdown race for translate ranker model loader.

[translate] Fix shutdown race for translate ranker model loader.

The RankerModelLoader was using a background thread to coordinate
its efforts to load the model first from a file, then from a URL.
In some circumstances, browser shutdown could begin while the model
file is being read from disk. If the loader further proceeds to
download the model file from its URL on the background thread, it
performs a racy attempt to grab a scoped_refptr to the Translate
Download Manager's URL Request Context Getter. This access should
not have been made on that thread.

This CL refactors the coordination of the ranker model loader to be
performed on the main browser thread, with I/O operations performed
on the background and I/O threads.

The TranslateURLFetcher now access the TranslateDownloadManager from
the correct thread. It further validates that is was able obtain a
valid URL request context getter before attempting to use it.

BUG=709905
Review-Url: https://codereview.chromium.org/2839433002
Cr-Commit-Position: refs/heads/master@{#467373}
Committed: https://chromium.googlesource.com/chromium/src/+/691c75601912d7b1dad012265a72bfaaea79bd4c

[Roger McFarlane (Chromium), 2017-04-21 23:17:23]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 23:17:49]

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-04-21 23:40:13]

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

[commit-bot: I haz the power, 2017-04-21 23:40:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Roger McFarlane (Chromium), 2017-04-24 14:44:39]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 14:45:23]

Dry run: CQ is trying da patch.

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

[Roger McFarlane (Chromium), 2017-04-24 14:47:33]

The CQ bit was unchecked by rogerm@chromium.org

[Roger McFarlane (Chromium), 2017-04-24 14:47:51]

Patchset #4 (id:60001) has been deleted

[Roger McFarlane (Chromium), 2017-04-24 14:50:25]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 14:50:53]

Dry run: CQ is trying da patch.

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

[Roger McFarlane (Chromium), 2017-04-24 15:27:32]

Patchset #5 (id:100001) has been deleted

[Roger McFarlane (Chromium), 2017-04-24 17:02:06]

rogerm@chromium.org changed reviewers:
+ groby@chromium.org, hamelphi@chromium.org

[Roger McFarlane (Chromium), 2017-04-24 17:02:07]

This CL restructures the loader code such that the coordination is performed on
the main thread and only the IO operations are performed on a background thread.

PTAL?

[Roger McFarlane (Chromium), 2017-04-24 17:21:54]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 17:22:25]

Dry run: CQ is trying da patch.

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

[groby-ooo-7-16, 2017-04-24 19:05:46]

On 2017/04/24 17:02:07, Roger McFarlane (Chromium) wrote:
> This CL restructures the loader code such that the coordination is performed
on
> the main thread and only the IO operations are performed on a background
thread.
> 
> PTAL?

Why are you moving everything from Backend to RankerModelLoader?

[groby-ooo-7-16, 2017-04-24 19:12:46]

FWIW - the changes outside of the ModelLoader L-G-T-M. And they look like they
possibly could be moved to a separate CL, in case ModelLoader takes longer?

https://codereview.chromium.org/2839433002/diff/140001/chrome/browser/transla...
File chrome/browser/translate/translate_manager_render_view_host_unittest.cc
(right):

https://codereview.chromium.org/2839433002/diff/140001/chrome/browser/transla...
chrome/browser/translate/translate_manager_render_view_host_unittest.cc:458:
ChromeRenderViewHostTestHarness::SetUp();
Why is the order of RVHTest setup and TranslateService test setup changed?
Possibly worth commenting?

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/ranker_model_loader_unittest.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/ranker_model_loader_unittest.cc:15: #include
"base/memory/ref_counted.h"
Why does this need ref_counted.h?

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/translate_script_unittest.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/translate_script_unittest.cc:10: #include
"base/memory/ref_counted.h"
Why ref_counted.h?

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/translate_url_fetcher.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/translate_url_fetcher.cc:53:
TranslateDownloadManager::GetInstance()->request_context();
So, request_context is RefCountedThreadSafe. Should we maybe return a
scoped_refptr? Or at least take the parameter into a scoped_refptr?

[Roger McFarlane (Chromium), 2017-04-24 20:31:33]

rogerm@chromium.org changed reviewers:
+ fdoray@chromium.org, jwd@chromium.org

[Roger McFarlane (Chromium), 2017-04-24 20:31:34]

re: Why remove backend.

Backend only existed to mark the scope of stateful operations performed on the
background thread, as opposed to the operations in RankerModelLoader which were
performed on the main thread. This was based on feedback/guidance I received
from the task scheduler folks about patterns and idiom for doing background work
in Chrome.

After moving the bits that should run on the main thread over to the
RankerModelLoader, there's no real "backend" abstraction left. So the class went
away.

https://codereview.chromium.org/2839433002/diff/140001/chrome/browser/transla...
File chrome/browser/translate/translate_manager_render_view_host_unittest.cc
(right):

https://codereview.chromium.org/2839433002/diff/140001/chrome/browser/transla...
chrome/browser/translate/translate_manager_render_view_host_unittest.cc:458:
ChromeRenderViewHostTestHarness::SetUp();
On 2017/04/24 19:12:46, groby wrote:
> Why is the order of RVHTest setup and TranslateService test setup changed?
> Possibly worth commenting?

RVHTest setup sets up the threading and message loop infrastructure. This needs
to be done before attempting to get the ThreadTaskRunnerHandle for the current
thread, in order to setup the TestURLRequestContextGetter.

Added a comment.

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/ranker_model_loader_unittest.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/ranker_model_loader_unittest.cc:15: #include
"base/memory/ref_counted.h"
On 2017/04/24 19:12:46, groby wrote:
> Why does this need ref_counted.h?

Just something I noticed is used without being explicitly included.

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/translate_script_unittest.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/translate_script_unittest.cc:10: #include
"base/memory/ref_counted.h"
On 2017/04/24 19:12:46, groby wrote:
> Why ref_counted.h?

I had originally added a scoped_refptr<net::URLRequestContextGetter> member to
the test fixture, but, since no-other code was referencing it, I just pass the
raw pointer into the download manager... and forgot to remove the inlcude.

Thanks.

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
File components/translate/core/browser/translate_url_fetcher.cc (right):

https://codereview.chromium.org/2839433002/diff/140001/components/translate/c...
components/translate/core/browser/translate_url_fetcher.cc:53:
TranslateDownloadManager::GetInstance()->request_context();
On 2017/04/24 19:12:46, groby wrote:
> So, request_context is RefCountedThreadSafe. Should we maybe return a
> scoped_refptr? Or at least take the parameter into a scoped_refptr?

I put the returned value in to a scoped_refptr.

It's a bit unfortunate that the getter and setter take raw pointer values. But I
suspect it might be because taking and returning scoped_refptr<T> by value
requires that the full definition of T be available, which means you can't just
forward declare T.

I'm not averse to changing it to accept/return scoped_refptr instead, but that's
probably be better done in its own CL.

[Roger McFarlane (Chromium), 2017-04-24 21:54:28]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 21:55:33]

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-04-24 23:20:20]

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

[commit-bot: I haz the power, 2017-04-24 23:20:21]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-04-25 14:50:44]

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.cc (right):

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:48: if (counter)
DCHECK(counter);

base::Histogram::FactoryTimeGet should always return an histogram if you provide
valid arguments.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:71: const std::string&
uma_prefix,
not used

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:156:
background_task_runner_->PostTask(
Return an std::string in LoadFromFile and do this here:

base::PostTaskAndReplyWithResult(
    background_task_runner_.get(),
    FROM_HERE,
    base::BindOnce(&LoadFromFile, model_path_, uma_prefix_),
    base::BindOnce(&RankerModelLoader::OnFileLoaded,
                   weak_ptr_factory_.GetWeakPtr());
        
The return value of LoadFromFile() will be passed as argument to
RankerModelLoader::OnFileLoaded.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:171: // Empty data
means model_path wasn't successfully read. Otherwise,
|model_path|

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:173:
std::unique_ptr<RankerModel> model = nullptr;
"= nullptr" not needed.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.h (right):

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:11: #include
"base/bind.h"
Not used

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:56: // callback may be
invoked after the RankerModelLoader has been destroyed and
"may be invoked after the RankerModelLoader has been destroyed" no longer true

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:63: // constructed. 
This callback may be invoked after the RankerModelLoader has
"may be invoked after the RankerModelLoader has been destroyed" no longer true

[Roger McFarlane (Chromium), 2017-04-25 21:16:25]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 21:17:05]

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-04-25 21:38:59]

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

[commit-bot: I haz the power, 2017-04-25 21:39:00]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Roger McFarlane (Chromium), 2017-04-25 21:49:21]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 21:49:50]

Dry run: CQ is trying da patch.

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

[Roger McFarlane (Chromium), 2017-04-25 21:50:40]

Thanks. Another look

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.cc (right):

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:48: if (counter)
On 2017/04/25 14:50:43, fdoray wrote:
> DCHECK(counter);
> 
> base::Histogram::FactoryTimeGet should always return an histogram if you
provide
> valid arguments.

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:71: const std::string&
uma_prefix,
On 2017/04/25 14:50:43, fdoray wrote:
> not used

Done and switched to PostTaskAndReplyWithResult

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:156:
background_task_runner_->PostTask(
On 2017/04/25 14:50:43, fdoray wrote:
> Return an std::string in LoadFromFile and do this here:
> 
> base::PostTaskAndReplyWithResult(
>     background_task_runner_.get(),
>     FROM_HERE,
>     base::BindOnce(&LoadFromFile, model_path_, uma_prefix_),
>     base::BindOnce(&RankerModelLoader::OnFileLoaded,
>                    weak_ptr_factory_.GetWeakPtr());
>         
> The return value of LoadFromFile() will be passed as argument to
> RankerModelLoader::OnFileLoaded.

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:171: // Empty data
means model_path wasn't successfully read. Otherwise,
On 2017/04/25 14:50:43, fdoray wrote:
> |model_path|

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:173:
std::unique_ptr<RankerModel> model = nullptr;
On 2017/04/25 14:50:43, fdoray wrote:
> "= nullptr" not needed.

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.h (right):

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:11: #include
"base/bind.h"
On 2017/04/25 14:50:44, fdoray wrote:
> Not used

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:56: // callback may be
invoked after the RankerModelLoader has been destroyed and
On 2017/04/25 14:50:44, fdoray wrote:
> "may be invoked after the RankerModelLoader has been destroyed" no longer true

Done.

https://codereview.chromium.org/2839433002/diff/180001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:63: // constructed. 
This callback may be invoked after the RankerModelLoader has
On 2017/04/25 14:50:43, fdoray wrote:
> "may be invoked after the RankerModelLoader has been destroyed" no longer true

Done.

[fdoray, 2017-04-25 22:09:09]

lgtm

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.cc (right):

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:95: const
ValidateModelCallback& validate_model_cb,
It is more efficient to take callbacks by value and std::move them into the
member variable.

Details: base::Callback is a wrapper around a ref-counted state object.
std::move into a member variable avoids a ref-count bump and doesn't really copy
data (just copy a pointer).

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:152:
base::Bind(&LoadFromFile, model_path_),
base::BindOnce

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.h (right):

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:101: // empty, to
LOADING_FROM_URL. If both model_path_ and model_url_ are
|model_path_| and |model_url_|

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:104: // The loader is
busy loading the model from model_path_ in the background.
|model_path_|

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:113: // The loader is
busy loading the model from model_url_ in the background.
|model_url_|

[commit-bot: I haz the power, 2017-04-25 22:44:26]

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

[commit-bot: I haz the power, 2017-04-25 22:44:27]

Dry run: This issue passed the CQ dry run.

[Roger McFarlane (Chromium), 2017-04-26 02:07:11]

Patchset #10 (id:220001) has been deleted

[Roger McFarlane (Chromium), 2017-04-26 02:18:50]

Thanks.

groby@, jwd@?

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.cc (right):

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:95: const
ValidateModelCallback& validate_model_cb,
On 2017/04/25 22:09:08, fdoray wrote:
> It is more efficient to take callbacks by value and std::move them into the
> member variable.
> 
> Details: base::Callback is a wrapper around a ref-counted state object.
> std::move into a member variable avoids a ref-count bump and doesn't really
copy
> data (just copy a pointer).

Done.

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.cc:152:
base::Bind(&LoadFromFile, model_path_),
On 2017/04/25 22:09:08, fdoray wrote:
> base::BindOnce

Alas, this fails to compile (on Linux at least) if either or both of these is
BindOnce.

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
File components/translate/core/browser/ranker_model_loader.h (right):

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:101: // empty, to
LOADING_FROM_URL. If both model_path_ and model_url_ are
On 2017/04/25 22:09:08, fdoray wrote:
> |model_path_| and |model_url_|

Done.

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:104: // The loader is
busy loading the model from model_path_ in the background.
On 2017/04/25 22:09:08, fdoray wrote:
> |model_path_|

Done.

https://codereview.chromium.org/2839433002/diff/200001/components/translate/c...
components/translate/core/browser/ranker_model_loader.h:113: // The loader is
busy loading the model from model_url_ in the background.
On 2017/04/25 22:09:08, fdoray wrote:
> |model_url_|

Done.

[Roger McFarlane (Chromium), 2017-04-26 02:19:01]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 02:19:38]

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-04-26 03:27:26]

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

[commit-bot: I haz the power, 2017-04-26 03:27:27]

Dry run: This issue passed the CQ dry run.

[hamelphi, 2017-04-26 15:56:30]

lgtm

[groby-ooo-7-16, 2017-04-26 16:33:13]

On 2017/04/26 15:56:30, hamelphi wrote:
> lgtm

lgtm

[Roger McFarlane (Chromium), 2017-04-26 17:02:32]

The CQ bit was checked by rogerm@chromium.org

[Roger McFarlane (Chromium), 2017-04-26 17:02:33]

The patchset sent to the CQ was uploaded after l-g-t-m from fdoray@chromium.org
Link to the patchset: https://codereview.chromium.org/2839433002/#ps240001
(title: "fdoray part deux")

[commit-bot: I haz the power, 2017-04-26 17:03:35]

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-04-26 17:13:37]

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

[commit-bot: I haz the power, 2017-04-26 17:13:39]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Roger McFarlane (Chromium), 2017-04-26 17:15:29]

jwd@: ping?

[jwd, 2017-04-26 17:26:47]

lgtm

[Roger McFarlane (Chromium), 2017-04-26 17:37:58]

The CQ bit was checked by rogerm@chromium.org

[commit-bot: I haz the power, 2017-04-26 17:38:33]

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-04-26 17:51:31]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1493228278737680,
"parent_rev": "678a0bea8eb9989953ca780f6afcb382ae047aee", "commit_rev":
"691c75601912d7b1dad012265a72bfaaea79bd4c"}

[commit-bot: I haz the power, 2017-04-26 17:51:40]

Description was changed from

==========
[translate] Fix shutdown race for translate ranker model loader.

The RankerModelLoader was using a background thread to coordinate
its efforts to load the model first from a file, then from a URL.
In some circumstances, browser shutdown could begin while the model
file is being read from disk. If the loader further proceeds to
download the model file from its URL on the background thread, it
performs a racy attempt to grab a scoped_refptr to the Translate
Download Manager's URL Request Context Getter. This access should
not have been made on that thread.

This CL refactors the coordination of the ranker model loader to be
performed on the main browser thread, with I/O operations performed
on the background and I/O threads.

The TranslateURLFetcher now access the TranslateDownloadManager from
the correct thread. It further validates that is was able obtain a
valid URL request context getter before attempting to use it.

BUG=709905
==========

to

==========
[translate] Fix shutdown race for translate ranker model loader.

The RankerModelLoader was using a background thread to coordinate
its efforts to load the model first from a file, then from a URL.
In some circumstances, browser shutdown could begin while the model
file is being read from disk. If the loader further proceeds to
download the model file from its URL on the background thread, it
performs a racy attempt to grab a scoped_refptr to the Translate
Download Manager's URL Request Context Getter. This access should
not have been made on that thread.

This CL refactors the coordination of the ranker model loader to be
performed on the main browser thread, with I/O operations performed
on the background and I/O threads.

The TranslateURLFetcher now access the TranslateDownloadManager from
the correct thread. It further validates that is was able obtain a
valid URL request context getter before attempting to use it.

BUG=709905

Review-Url: https://codereview.chromium.org/2839433002
Cr-Commit-Position: refs/heads/master@{#467373}
Committed:
https://chromium.googlesource.com/chromium/src/+/691c75601912d7b1dad012265a72...
==========

[commit-bot: I haz the power, 2017-04-26 17:51:41]

Committed patchset #10 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/691c75601912d7b1dad012265a72...