Plugins: Add UKM Metrics to Flash loads

Plugins: Add UKM Metrics to Flash loads

Tracks the main frame's URL for loads of Flash plugin instances.

BUG=704688
Review-Url: https://codereview.chromium.org/2768263003
Cr-Commit-Position: refs/heads/master@{#461458}
Committed: https://chromium.googlesource.com/chromium/src/+/910b408e9d1ba5c36b5663a682b70cfc43e58a53

[tommycli, 2017-03-23 21:01:53]

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

[tommycli, 2017-03-23 21:02:56]

tommycli@chromium.org changed reviewers:
+ rkaplow@chromium.org

[tommycli, 2017-03-23 21:02:56]

rkaplow: PTAL. How does this look from the UKM perspective?

[commit-bot: I haz the power, 2017-03-23 21:03: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, 2017-03-23 21:40:00]

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

[commit-bot: I haz the power, 2017-03-23 21:40:01]

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

[rkaplow, 2017-03-23 22:57:41]

rkaplow@chromium.org changed reviewers:
+ holte@chromium.org

[rkaplow, 2017-03-23 22:57:42]

lgtm

lg, adding holte@ if he has any comments

https://codereview.chromium.org/2768263003/diff/1/chrome/browser/plugins/plug...
File chrome/browser/plugins/plugin_info_message_filter.h (right):

https://codereview.chromium.org/2768263003/diff/1/chrome/browser/plugins/plug...
chrome/browser/plugins/plugin_info_message_filter.h:155: static void
ReportMetrics(const std::string& mime_type,
may want to use base::StringPiece

[Steven Holte, 2017-03-23 23:29:47]

UKM parts lgtm.

Looks like you've got a crash in ChromeContentRendererClient though.

[tommycli, 2017-03-24 00:39:34]

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

[tommycli, 2017-03-24 00:39:48]

thanks UKM guys! I'll seek some help to resolve the OOPIF crash.

https://codereview.chromium.org/2768263003/diff/1/chrome/browser/plugins/plug...
File chrome/browser/plugins/plugin_info_message_filter.h (right):

https://codereview.chromium.org/2768263003/diff/1/chrome/browser/plugins/plug...
chrome/browser/plugins/plugin_info_message_filter.h:155: static void
ReportMetrics(const std::string& mime_type,
On 2017/03/23 22:57:41, rkaplow (slow) wrote:
> may want to use base::StringPiece

Done.

[commit-bot: I haz the power, 2017-03-24 00:39:55]

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-03-24 01:11:56]

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

[commit-bot: I haz the power, 2017-03-24 01:11:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tommycli, 2017-03-24 01:33:55]

tommycli@chromium.org changed reviewers:
+ alexmos@chromium.org

[tommycli, 2017-03-24 01:33:56]

alexmos: Hey, I'm getting some OOPIF crashes. I have some thoughts below, but
could use an expert consult...

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:242: main_frame_url,
mime_type};
alexmos:

It seems that we can't use top()->document().url() anymore in the OOPIF world.
Is the right approach to look up the top-level URL on the browser side?

This code is pretty close, but still needs to bounce the UI thread to retrieve
the URL (it crashes currently).

Am I on the right track? Or is there a way to get the top-level URL reliably on
the render process from RenderFrame?

Thanks,

Tommy

[alexmos, 2017-03-24 02:08:47]

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:242: main_frame_url,
mime_type};
On 2017/03/24 01:33:56, tommycli wrote:
> alexmos:
> 
> It seems that we can't use top()->document().url() anymore in the OOPIF world.
> Is the right approach to look up the top-level URL on the browser side?
> 
> This code is pretty close, but still needs to bounce the UI thread to retrieve
> the URL (it crashes currently).
> 
> Am I on the right track? Or is there a way to get the top-level URL reliably
on
> the render process from RenderFrame?

Yes, top()->document().url() is not available with OOPIFs.  We don't replicate
the URLs for remote frames, just the origins.  So if all you need is the top
frame's origin, you could still get it in Blink via
top()->securityContext()->getSecurityOrigin(), but if you need the URL, you'll
have to get it on the browser side.

And yes, the approach here is a good start except that you'll need to hop onto
the UI thread.  One concern with that is that it opens the door to race
conditions (e.g., the frame might go away or change the last committed URL
during the hop, so you could end up looking up the wrong URL), so you'd need to
think through whether that matters for your use case.  (Maybe you could detect
the last committed URL change if you check whether the subframe that sent this
still exists in the FrameTree, because if the top frame commits a new URL,
subframes that were part of the previous document would get destroyed.)

[tommycli, 2017-03-24 18:03:00]

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

[tommycli, 2017-03-24 18:03:02]

alexmos: Thanks. PS5 has an update.

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:242: main_frame_url,
mime_type};
On 2017/03/24 02:08:47, alexmos wrote:
> On 2017/03/24 01:33:56, tommycli wrote:
> > alexmos:
> > 
> > It seems that we can't use top()->document().url() anymore in the OOPIF
world.
> > Is the right approach to look up the top-level URL on the browser side?
> > 
> > This code is pretty close, but still needs to bounce the UI thread to
retrieve
> > the URL (it crashes currently).
> > 
> > Am I on the right track? Or is there a way to get the top-level URL reliably
> on
> > the render process from RenderFrame?
> 
> Yes, top()->document().url() is not available with OOPIFs.  We don't replicate
> the URLs for remote frames, just the origins.  So if all you need is the top
> frame's origin, you could still get it in Blink via
> top()->securityContext()->getSecurityOrigin(), but if you need the URL, you'll
> have to get it on the browser side.
> 
> And yes, the approach here is a good start except that you'll need to hop onto
> the UI thread.  One concern with that is that it opens the door to race
> conditions (e.g., the frame might go away or change the last committed URL
> during the hop, so you could end up looking up the wrong URL), so you'd need
to
> think through whether that matters for your use case.  (Maybe you could detect
> the last committed URL change if you check whether the subframe that sent this
> still exists in the FrameTree, because if the top frame commits a new URL,
> subframes that were part of the previous document would get destroyed.)

Awesome, thank you for the detailed info.

I changed the code in PS5 so that the origin is still passed through as normal.
Then once we bounce to the UI thread for the ReportMetrics call, we fetch the
WebContents URL and compare origins (to check if it's changed).

Hopefully that seems adequete. My theory was that RenderFrameHost::FromId would
return nullptr if the subframe has been destroyed. -- And the origin wouldn't
match if we navigated to a different origin. Let me know if that logic seems
sound.

[commit-bot: I haz the power, 2017-03-24 18:03:42]

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

[tommycli, 2017-03-24 18:12:12]

tommycli@chromium.org changed reviewers:
+ bauerb@chromium.org
- alexmos@chromium.org

[tommycli, 2017-03-24 18:12:12]

bauerb: PTAL plugins code changes in general, thanks!

[tommycli, 2017-03-24 19:04:16]

Description was changed from

==========
Plugins: Add UKM Metrics to Flash loads

BUG=704688
==========

to

==========
Plugins: Add UKM Metrics to Flash loads

Tracks the main frame's URL for loads of Flash plugin instances.

BUG=704688
==========

[commit-bot: I haz the power, 2017-03-24 19:43:25]

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

[commit-bot: I haz the power, 2017-03-24 19:43:26]

Dry run: This issue passed the CQ dry run.

[alexmos, 2017-03-25 02:20:45]

alexmos@chromium.org changed reviewers:
+ alexmos@chromium.org

[alexmos, 2017-03-25 02:20:45]

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/40001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:242: main_frame_url,
mime_type};
On 2017/03/24 18:03:02, tommycli wrote:
> On 2017/03/24 02:08:47, alexmos wrote:
> > On 2017/03/24 01:33:56, tommycli wrote:
> > > alexmos:
> > > 
> > > It seems that we can't use top()->document().url() anymore in the OOPIF
> world.
> > > Is the right approach to look up the top-level URL on the browser side?
> > > 
> > > This code is pretty close, but still needs to bounce the UI thread to
> retrieve
> > > the URL (it crashes currently).
> > > 
> > > Am I on the right track? Or is there a way to get the top-level URL
reliably
> > on
> > > the render process from RenderFrame?
> > 
> > Yes, top()->document().url() is not available with OOPIFs.  We don't
replicate
> > the URLs for remote frames, just the origins.  So if all you need is the top
> > frame's origin, you could still get it in Blink via
> > top()->securityContext()->getSecurityOrigin(), but if you need the URL,
you'll
> > have to get it on the browser side.
> > 
> > And yes, the approach here is a good start except that you'll need to hop
onto
> > the UI thread.  One concern with that is that it opens the door to race
> > conditions (e.g., the frame might go away or change the last committed URL
> > during the hop, so you could end up looking up the wrong URL), so you'd need
> to
> > think through whether that matters for your use case.  (Maybe you could
detect
> > the last committed URL change if you check whether the subframe that sent
this
> > still exists in the FrameTree, because if the top frame commits a new URL,
> > subframes that were part of the previous document would get destroyed.)
> 
> Awesome, thank you for the detailed info.
> 
> I changed the code in PS5 so that the origin is still passed through as
normal.
> Then once we bounce to the UI thread for the ReportMetrics call, we fetch the
> WebContents URL and compare origins (to check if it's changed).
> 
> Hopefully that seems adequete. My theory was that RenderFrameHost::FromId
would
> return nullptr if the subframe has been destroyed. -- And the origin wouldn't
> match if we navigated to a different origin. Let me know if that logic seems
> sound.

Looks reasonable.  I was thinking through whether this works when the main frame
origin doesn't change, but the last committed URL does.  I.e., you're on
a.com/1.html, which embeds b.com/plugin.html, and as you send your IPC, the top
frame navigates to a.com/2.html, which will reuse the top-level RenderFrameHost.
 The origin won't change, but the URL would be a.com/2.html instead of
a.com/1.html.  However, I don't think that's actually a problem, since the
subframe RFH for b.com should still get destroyed in that case as part of
navigating to a new document, so you should never get to the origin check due to
RenderFrameHost::FromId returning null.  So you may not actually need to check
origins.

[Bernhard Bauer, 2017-03-27 11:04:43]

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:549: if
(chrome::IsIncognitoSessionActive())
Can we check whether the profile corresponding to the WebContents is incognito
rather than whether any incognito session is active?

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:558: if (mime_type ==
content::kFlashPluginSwfMimeType ||
Return early if they mismatch? (Also for the other checks below)

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:582:
ukm_service->GetEntryBuilder(ukm_source_id,
Wait, we create a new entry builder and then immediately destroy it?

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.h (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.h:157: // report on the main
thread, and cross-thread weak pointers are problematic.
"Problematic" sounds like a Haunted Graveyard
(https://www.usenix.org/sites/default/files/conference/protected-files/srecon1...)
:) In general there are some operations that aren't allowed with cross-thread
weak pointers, but in this particular instance the filter is refcounted _and_
uses weak pointers, and invalidates the weak pointers if the refcount drops to
zero, so either using refcounting or weak pointers would work. The profile might
not exist anymore when the callback runs, but in that case we would get a null
RenderFrameHost and WebContents, which we already check for, and anything else
is coming from the browser process.

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.h:158: static void
ReportMetrics(int render_process_id,
Can you add a comment to explain why this needs to be in the class?

https://codereview.chromium.org/2768263003/diff/80001/components/ukm/ukm_serv...
File components/ukm/ukm_service.h (right):

https://codereview.chromium.org/2768263003/diff/80001/components/ukm/ukm_serv...
components/ukm/ukm_service.h:49: class UkmService : public
base::SupportsWeakPtr<UkmService> {
Yak-shaving: Is this actually used? For internal callbacks there is already a
WeakPtrFactory.

[Steven Holte, 2017-03-27 19:27:18]

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:582:
ukm_service->GetEntryBuilder(ukm_source_id,
On 2017/03/27 11:04:43, Bernhard (OOO until Mar 27) wrote:
> Wait, we create a new entry builder and then immediately destroy it?

That seems right, since UkmEntryBuilder records the built Entry when it falls
out of scope.  This just creates an Entry with a name and source_id, but no
Metrics.

[tommycli, 2017-03-28 17:20:40]

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

[tommycli, 2017-03-28 17:20:50]

thanks! I've addresed the comment below.

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.cc (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:549: if
(chrome::IsIncognitoSessionActive())
On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> Can we check whether the profile corresponding to the WebContents is incognito
> rather than whether any incognito session is active?

Done.

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.cc:558: if (mime_type ==
content::kFlashPluginSwfMimeType ||
On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> Return early if they mismatch? (Also for the other checks below)

Done.

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.h (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.h:157: // report on the main
thread, and cross-thread weak pointers are problematic.
On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> "Problematic" sounds like a Haunted Graveyard
>
(https://www.usenix.org/sites/default/files/conference/protected-files/srecon1...)
> :) In general there are some operations that aren't allowed with cross-thread
> weak pointers, but in this particular instance the filter is refcounted _and_
> uses weak pointers, and invalidates the weak pointers if the refcount drops to
> zero, so either using refcounting or weak pointers would work. The profile
might
> not exist anymore when the callback runs, but in that case we would get a null
> RenderFrameHost and WebContents, which we already check for, and anything else
> is coming from the browser process.

Done. I couldn't use weak pointers because the weak ptr factory is only for the
IO thread. I ended up using ref counting though. thanks!

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.h:158: static void
ReportMetrics(int render_process_id,
On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> Can you add a comment to explain why this needs to be in the class?

Done.

https://codereview.chromium.org/2768263003/diff/80001/components/ukm/ukm_serv...
File components/ukm/ukm_service.h (right):

https://codereview.chromium.org/2768263003/diff/80001/components/ukm/ukm_serv...
components/ukm/ukm_service.h:49: class UkmService : public
base::SupportsWeakPtr<UkmService> {
On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> Yak-shaving: Is this actually used? For internal callbacks there is already a
> WeakPtrFactory.

Done.

[commit-bot: I haz the power, 2017-03-28 17:21:13]

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-03-28 19:29:22]

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

[commit-bot: I haz the power, 2017-03-28 19:29:23]

Dry run: This issue passed the CQ dry run.

[tommycli, 2017-03-29 17:19:46]

bauerb: friendly ping! thanks!

[tommycli, 2017-03-30 20:52:04]

On 2017/03/29 17:19:46, tommycli wrote:
> bauerb: friendly ping! thanks!

bauerb: ping! do you mind taking another look? Thanks!

[Bernhard Bauer, 2017-03-31 09:47:26]

LGTM!

Sorry, somehow missed this.

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
File chrome/browser/plugins/plugin_info_message_filter.h (right):

https://codereview.chromium.org/2768263003/diff/80001/chrome/browser/plugins/...
chrome/browser/plugins/plugin_info_message_filter.h:157: // report on the main
thread, and cross-thread weak pointers are problematic.
On 2017/03/28 17:20:49, tommycli wrote:
> On 2017/03/27 11:04:43, Bernhard Bauer wrote:
> > "Problematic" sounds like a Haunted Graveyard
> >
>
(https://www.usenix.org/sites/default/files/conference/protected-files/srecon1...)
> > :) In general there are some operations that aren't allowed with
cross-thread
> > weak pointers, but in this particular instance the filter is refcounted
_and_
> > uses weak pointers, and invalidates the weak pointers if the refcount drops
to
> > zero, so either using refcounting or weak pointers would work. The profile
> might
> > not exist anymore when the callback runs, but in that case we would get a
null
> > RenderFrameHost and WebContents, which we already check for, and anything
else
> > is coming from the browser process.
> 
> Done. I couldn't use weak pointers because the weak ptr factory is only for
the
> IO thread. I ended up using ref counting though. thanks!

Hm, I just realized that the WeakPtrFactory could be invalidated on the UI
thread if that is where the last reference is dropped, which would DCHECK (as
invalidation needs to happen on the sequence it's bound to). I'll need to fix
that separately though. Thanks for the rubber-duck debugging! :)

[tommycli, 2017-03-31 16:08:42]

thanks all, sending it in!

[tommycli, 2017-03-31 16:08:52]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2017-03-31 16:08:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2768263003/#ps100001
(title: "fix")

[commit-bot: I haz the power, 2017-03-31 16:09:28]

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-03-31 16:37:59]

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

[commit-bot: I haz the power, 2017-03-31 16:38:00]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[tommycli, 2017-04-03 15:22:54]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2017-04-03 15:23:19]

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-03 15:26:20]

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

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

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tommycli, 2017-04-03 15:30:53]

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

[tommycli, 2017-04-03 15:31:04]

The CQ bit was unchecked by tommycli@chromium.org

[tommycli, 2017-04-03 15:31:05]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2017-04-03 15:31:06]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rkaplow@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2768263003/#ps120001
(title: "merge")

[commit-bot: I haz the power, 2017-04-03 15:31:18]

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-03 17:04:20]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491233465642460,
"parent_rev": "b445984eba859a4e3febed02d619776e4afd3dff", "commit_rev":
"910b408e9d1ba5c36b5663a682b70cfc43e58a53"}

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

Description was changed from

==========
Plugins: Add UKM Metrics to Flash loads

Tracks the main frame's URL for loads of Flash plugin instances.

BUG=704688
==========

to

==========
Plugins: Add UKM Metrics to Flash loads

Tracks the main frame's URL for loads of Flash plugin instances.

BUG=704688

Review-Url: https://codereview.chromium.org/2768263003
Cr-Commit-Position: refs/heads/master@{#461458}
Committed:
https://chromium.googlesource.com/chromium/src/+/910b408e9d1ba5c36b5663a682b7...
==========

[commit-bot: I haz the power, 2017-04-03 17:05:28]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/910b408e9d1ba5c36b5663a682b7...