[page_load_metrics] Add support for chrome:// urls and add extensions observer

[page_load_metrics] Add support for chrome:// urls and add extensions observer

BUG=647764

[Charlie Harrison, 2016-09-16 21:29:35]

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

[commit-bot: I haz the power, 2016-09-16 21:30:03]

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

[Charlie Harrison, 2016-09-16 21:52:13]

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

[commit-bot: I haz the power, 2016-09-16 21:52:53]

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, 2016-09-16 22:33:09]

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

[commit-bot: I haz the power, 2016-09-16 22:33:10]

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_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Charlie Harrison, 2016-09-19 19:01:40]

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

[commit-bot: I haz the power, 2016-09-19 19:02:21]

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, 2016-09-19 19:52:29]

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

[commit-bot: I haz the power, 2016-09-19 19:52:30]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Charlie Harrison, 2016-09-20 17:29:35]

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

[commit-bot: I haz the power, 2016-09-20 17:29:58]

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

[Charlie Harrison, 2016-09-20 18:08:14]

csharrison@chromium.org changed reviewers:
+ bmcquade@chromium.org

[Charlie Harrison, 2016-09-20 18:08:14]

bmcquade, ptal?

[commit-bot: I haz the power, 2016-09-20 18:15:31]

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

[commit-bot: I haz the power, 2016-09-20 18:15:33]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Bryan McQuade, 2016-09-20 18:40:39]

Thanks for putting this together! Overall I think the structure is coming
together nicely.

My main feedback would be that it's potentially confusing that we've split the
policy about whether we should track a load across two places: the decider,
which is used to make a global decision for everything but http/https URLs, and
the new observer ShouldTrackLoad method, which is used to make observer-local
decisions currently for URL schemes only.

I think that for now it's probably OK do to it as you have it, but if we get one
more observer that wants to customize policy in a different way (perhaps they
want to track NTP URLs for example) I think we'll need to generalize the
implementation by moving all of the checking policy into the observer class,
perhaps providing a PageLoadMetricsObserver::ShouldTrackLoad base class
implementation that looks like this:

bool PageLoadMetricsObserver::ShouldTrackLoad(...) {
  return BrowserPageTrackDecider(embedder_interface_.get(), web_contents(),
                                 navigation_handle).ShouldTrack();
}

This has the drawback of essentially getting rid of the render decider class
since we now can't know which pages may be ignored on the browser side, but I
think centralizing the policy in the browser process and paying the slight
additional cost of sending timing data for page loads that aren't tracked is
probably worthwhile. That also means the shared PageLoadTrackDecider class can
go away, which is a nice simplification. I'd be open to making that change now
if you want but I think we should hash it out on a whiteboard first. I'm also
totally fine proceeding with what you've got here for now.

One thing I'm a little unclear on: why do we need to have the IsAboutBlankUrl
check? won't the default policy restrict to http/https URLs, and the new
observer will only allow chrome schemes, which means about blank always gets
ignored and we don't need that extra method?

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File chrome/browser/page_load_metrics/page_load_metrics_observer.h (right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/page_load_metrics_observer.h:140: virtual
TrackingPolicy ShouldTrackLoad(
i think a boolean is ok here - the method name is really clear about what a
boolean return value would indicate. do you think we need an enum?

[Bryan McQuade, 2016-09-20 18:47:33]

A couple comments. Thanks for putting this together!

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
can this become
if (url.host() == chrome::kChromeUIExtensionsHost)
?

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:43:
internal::kHistogramExtensionsParseDuration,
is the concern for this page that it may spend a bunch of time loading script?
if so it may also be useful to track parse_blocked_on_script_load_duration as
well. if script exec time is a concern we can also track the exec variant.

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.h
(right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.h:18:
class WebUIPageLoadMetricsObserver
i'm not familiar with the name WebUI so this class would benefit from a comment
- does WebUI refer to pages with chrome URL scheme, or only a subset of those
pages?

https://codereview.chromium.org/2350473002/diff/80001/chrome/renderer/page_lo...
File chrome/renderer/page_load_metrics/renderer_page_track_decider.cc (right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/renderer/page_lo...
chrome/renderer/page_load_metrics/renderer_page_track_decider.cc:34: return
document_->url().string() == "about:blank";
can we use
https://cs.chromium.org/chromium/src/url/url_constants.cc?q=about:blank&l=9&dr=C
constant here?

[Charlie Harrison, 2016-09-20 22:46:22]

csharrison@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Charlie Harrison, 2016-09-20 22:46:23]

+rdevlin.cronin, please let me know if there's anything I missed here.

A few things:
 - I wanted to loosen the IPC policy without completely eliminating it. I could
potentially be convinced that completely eliminating them is the right move but
it would take a bit of convincing.

 - I included about:blank pages because they occur as the first navigation in
new tabs / renderers. I think it's one of the important non-http/s cases to
continue to gate.

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/20 18:47:33, Bryan McQuade wrote:
> can this become
> if (url.host() == chrome::kChromeUIExtensionsHost)
> ?

the chrome url gets rewritten to chrome://chrome/extensions. I saw this pattern
elsewhere for chrome urls...weird right?

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:43:
internal::kHistogramExtensionsParseDuration,
On 2016/09/20 18:47:33, Bryan McQuade wrote:
> is the concern for this page that it may spend a bunch of time loading script?
> if so it may also be useful to track parse_blocked_on_script_load_duration as
> well. if script exec time is a concern we can also track the exec variant.

I mainly added this because it seemed useful to track the "many small resources"
case to optimize loading for (and other md-settings pages).

+devlin is this useful to you?

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.h
(right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.h:18:
class WebUIPageLoadMetricsObserver
On 2016/09/20 18:47:33, Bryan McQuade wrote:
> i'm not familiar with the name WebUI so this class would benefit from a
comment
> - does WebUI refer to pages with chrome URL scheme, or only a subset of those
> pages?

AFAIK they are all chrome:// urls, and it is indicated as such in the bug.

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
File chrome/browser/page_load_metrics/page_load_metrics_observer.h (right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/page_load_metrics_observer.h:140: virtual
TrackingPolicy ShouldTrackLoad(
On 2016/09/20 18:40:38, Bryan McQuade wrote:
> i think a boolean is ok here - the method name is really clear about what a
> boolean return value would indicate. do you think we need an enum?

rdevlin recommended this on the doc. Fine to nix it though I don't feel strongly
either way.

https://codereview.chromium.org/2350473002/diff/80001/chrome/renderer/page_lo...
File chrome/renderer/page_load_metrics/renderer_page_track_decider.cc (right):

https://codereview.chromium.org/2350473002/diff/80001/chrome/renderer/page_lo...
chrome/renderer/page_load_metrics/renderer_page_track_decider.cc:34: return
document_->url().string() == "about:blank";
On 2016/09/20 18:47:33, Bryan McQuade wrote:
> can we use
>
https://cs.chromium.org/chromium/src/url/url_constants.cc?q=about:blank&l=9&dr=C
> constant here?

Done.

[Charlie Harrison, 2016-09-20 22:51:56]

Also, browsertests look flaky I'll investigate tomorrow.

[Devlin, 2016-09-20 22:59:16]

Thanks for doing this!  I just looked at webui_page_load_metrics_observer -
lemme know if there's anything else I should see.

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:23:
if (url.SchemeIs("chrome")) {
kChromeUIScheme

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
more information than you ever wanted:
In the current settings page, we have two frames - the top frame
(chrome://chrome/extensions) and the content frame (chrome://extensions-frame). 
The former isn't very exciting; the latter has all the interesting stuff.  If we
can limit ourselves to tracking the extensions-frame, that's probably better.
In the new (MD) world, we enter a more sane state, where there is a single frame
- chrome://extensions.

Can we change this to track those two instances, rather than the top frame of
the settings page?

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:43:
internal::kHistogramExtensionsParseDuration,
It would be useful to have this broken apart by MD vs non-MD* - see e.g. how we
do this in extensions_ui.cc today:
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/extensions/exten...

*In theory, Finch grants us this breakdown, but that's only useful when we have
equivalent populations, and I'd love to be able to look at this for performance
data prior to rollout.

[Charlie Harrison, 2016-09-21 00:32:49]

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/20 22:59:15, Devlin wrote:
> more information than you ever wanted:
> In the current settings page, we have two frames - the top frame
> (chrome://chrome/extensions) and the content frame
(chrome://extensions-frame). 
> The former isn't very exciting; the latter has all the interesting stuff.  If
we
> can limit ourselves to tracking the extensions-frame, that's probably better.
> In the new (MD) world, we enter a more sane state, where there is a single
frame
> - chrome://extensions.
> 
> Can we change this to track those two instances, rather than the top frame of
> the settings page?

Hmm this is tricky with the current architecture, which only tracks metrics for
a main frame navigation. FCP tracks the min of {first text paint, first image
paint}. If necessary we can probably log first meaningful paint which tracks the
most significant layout.

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:43:
internal::kHistogramExtensionsParseDuration,
On 2016/09/20 22:59:15, Devlin wrote:
> It would be useful to have this broken apart by MD vs non-MD* - see e.g. how
we
> do this in extensions_ui.cc today:
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/extensions/exten...
> 
> *In theory, Finch grants us this breakdown, but that's only useful when we
have
> equivalent populations, and I'd love to be able to look at this for
performance
> data prior to rollout.

ACK. This is a good idea.

[Devlin, 2016-09-21 00:47:22]

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/21 00:32:48, Charlie Harrison wrote:
> On 2016/09/20 22:59:15, Devlin wrote:
> > more information than you ever wanted:
> > In the current settings page, we have two frames - the top frame
> > (chrome://chrome/extensions) and the content frame
> (chrome://extensions-frame). 
> > The former isn't very exciting; the latter has all the interesting stuff. 
If
> we
> > can limit ourselves to tracking the extensions-frame, that's probably
better.
> > In the new (MD) world, we enter a more sane state, where there is a single
> frame
> > - chrome://extensions.
> > 
> > Can we change this to track those two instances, rather than the top frame
of
> > the settings page?
> 
> Hmm this is tricky with the current architecture, which only tracks metrics
for
> a main frame navigation. FCP tracks the min of {first text paint, first image
> paint}. If necessary we can probably log first meaningful paint which tracks
the
> most significant layout.

This would still be useful for MD extensions, then, but I'd be a little dubious
of relying on it for the current world.  You can see for yourself how little
content there is when you remove the big iframe in chrome://extensions through
devtools - it's basically just the sidebar.

FirstMeaningfulPaint sounds interesting, and might be worth trying out.  I don't
mind being the guinea pig. :)

[Charlie Harrison, 2016-09-26 20:05:57]

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/21 00:47:22, Devlin wrote:
> On 2016/09/21 00:32:48, Charlie Harrison wrote:
> > On 2016/09/20 22:59:15, Devlin wrote:
> > > more information than you ever wanted:
> > > In the current settings page, we have two frames - the top frame
> > > (chrome://chrome/extensions) and the content frame
> > (chrome://extensions-frame). 
> > > The former isn't very exciting; the latter has all the interesting stuff. 
> If
> > we
> > > can limit ourselves to tracking the extensions-frame, that's probably
> better.
> > > In the new (MD) world, we enter a more sane state, where there is a single
> > frame
> > > - chrome://extensions.
> > > 
> > > Can we change this to track those two instances, rather than the top frame
> of
> > > the settings page?
> > 
> > Hmm this is tricky with the current architecture, which only tracks metrics
> for
> > a main frame navigation. FCP tracks the min of {first text paint, first
image
> > paint}. If necessary we can probably log first meaningful paint which tracks
> the
> > most significant layout.
> 
> This would still be useful for MD extensions, then, but I'd be a little
dubious
> of relying on it for the current world.  You can see for yourself how little
> content there is when you remove the big iframe in chrome://extensions through
> devtools - it's basically just the sidebar.
> 
> FirstMeaningfulPaint sounds interesting, and might be worth trying out.  I
don't
> mind being the guinea pig. :)
I did an experiment and it still seems like (most of the time) FMP is triggered
at first paint of the shell, even when I add ~7 extensions which makes the frame
fill my entire screen.

This seems like an error in FMP, which I thought would take this into account.
How would you like to proceed with this patch? I can cc ksakamoto@ to the bug
and explain this failure.

[Devlin, 2016-09-26 20:39:54]

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/26 20:05:57, Charlie Harrison wrote:
> On 2016/09/21 00:47:22, Devlin wrote:
> > On 2016/09/21 00:32:48, Charlie Harrison wrote:
> > > On 2016/09/20 22:59:15, Devlin wrote:
> > > > more information than you ever wanted:
> > > > In the current settings page, we have two frames - the top frame
> > > > (chrome://chrome/extensions) and the content frame
> > > (chrome://extensions-frame). 
> > > > The former isn't very exciting; the latter has all the interesting
stuff. 
> > If
> > > we
> > > > can limit ourselves to tracking the extensions-frame, that's probably
> > better.
> > > > In the new (MD) world, we enter a more sane state, where there is a
single
> > > frame
> > > > - chrome://extensions.
> > > > 
> > > > Can we change this to track those two instances, rather than the top
frame
> > of
> > > > the settings page?
> > > 
> > > Hmm this is tricky with the current architecture, which only tracks
metrics
> > for
> > > a main frame navigation. FCP tracks the min of {first text paint, first
> image
> > > paint}. If necessary we can probably log first meaningful paint which
tracks
> > the
> > > most significant layout.
> > 
> > This would still be useful for MD extensions, then, but I'd be a little
> dubious
> > of relying on it for the current world.  You can see for yourself how little
> > content there is when you remove the big iframe in chrome://extensions
through
> > devtools - it's basically just the sidebar.
> > 
> > FirstMeaningfulPaint sounds interesting, and might be worth trying out.  I
> don't
> > mind being the guinea pig. :)
> I did an experiment and it still seems like (most of the time) FMP is
triggered
> at first paint of the shell, even when I add ~7 extensions which makes the
frame
> fill my entire screen.
> 
> This seems like an error in FMP, which I thought would take this into account.
> How would you like to proceed with this patch? I can cc ksakamoto@ to the bug
> and explain this failure.

If this turns out to be very complicated, could we do this to only record for MD
extensions for now, and then pursue metrics for more complicated cases (like the
uber page) separately?  It's a shame it wouldn't land in parallel since it means
we'd be lacking comparison data, but it would be a good starting point.

[Charlie Harrison, 2016-09-26 21:38:19]

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
File
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2350473002/diff/100001/chrome/browser/page_lo...
chrome/browser/page_load_metrics/observers/webui_page_load_metrics_observer.cc:24:
if (url.path_piece().find(chrome::kChromeUIExtensionsHost) == 1) {
On 2016/09/26 20:39:54, Devlin (catching up) wrote:
> On 2016/09/26 20:05:57, Charlie Harrison wrote:
> > On 2016/09/21 00:47:22, Devlin wrote:
> > > On 2016/09/21 00:32:48, Charlie Harrison wrote:
> > > > On 2016/09/20 22:59:15, Devlin wrote:
> > > > > more information than you ever wanted:
> > > > > In the current settings page, we have two frames - the top frame
> > > > > (chrome://chrome/extensions) and the content frame
> > > > (chrome://extensions-frame). 
> > > > > The former isn't very exciting; the latter has all the interesting
> stuff. 
> > > If
> > > > we
> > > > > can limit ourselves to tracking the extensions-frame, that's probably
> > > better.
> > > > > In the new (MD) world, we enter a more sane state, where there is a
> single
> > > > frame
> > > > > - chrome://extensions.
> > > > > 
> > > > > Can we change this to track those two instances, rather than the top
> frame
> > > of
> > > > > the settings page?
> > > > 
> > > > Hmm this is tricky with the current architecture, which only tracks
> metrics
> > > for
> > > > a main frame navigation. FCP tracks the min of {first text paint, first
> > image
> > > > paint}. If necessary we can probably log first meaningful paint which
> tracks
> > > the
> > > > most significant layout.
> > > 
> > > This would still be useful for MD extensions, then, but I'd be a little
> > dubious
> > > of relying on it for the current world.  You can see for yourself how
little
> > > content there is when you remove the big iframe in chrome://extensions
> through
> > > devtools - it's basically just the sidebar.
> > > 
> > > FirstMeaningfulPaint sounds interesting, and might be worth trying out.  I
> > don't
> > > mind being the guinea pig. :)
> > I did an experiment and it still seems like (most of the time) FMP is
> triggered
> > at first paint of the shell, even when I add ~7 extensions which makes the
> frame
> > fill my entire screen.
> > 
> > This seems like an error in FMP, which I thought would take this into
account.
> > How would you like to proceed with this patch? I can cc ksakamoto@ to the
bug
> > and explain this failure.
> 
> If this turns out to be very complicated, could we do this to only record for
MD
> extensions for now, and then pursue metrics for more complicated cases (like
the
> uber page) separately?  It's a shame it wouldn't land in parallel since it
means
> we'd be lacking comparison data, but it would be a good starting point.

SG. I will amend the patch to only log for MD.

[Bryan McQuade, 2017-06-19 12:39:26]

should we close this old issue out?

[Charlie Harrison, 2017-06-19 13:22:37]

Yeah,
Devlin sorry I dropped the ball with this one, let's chat again if the priority
of this work increases.

[Charlie Harrison, 2017-06-19 13:22:53]

Description was changed from

==========
[page_load_metrics] Add support for chrome:// urls and add extensions observer

BUG=647764
==========

to

==========
[page_load_metrics] Add support for chrome:// urls and add extensions observer

BUG=647764
==========