Mark Supplement instead of RefCountedSupplement in NavigatorContentUtils

Mark Supplement instead of RefCountedSupplement in NavigatorContentUtils

There is no big reason that NavigatorContentUtils should use RefCountedSupplement. This CL marks the NavigatorContentUtils to use Supplement instead of RefCountedSupplement. Additionally this cl adds a tracing method for supplement.

BUG=none

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171403

[haraken, 2014-04-11 13:06:04]

BTW, why does NavigatorUtils need to be RefCountedSupplement? I don't see any
particular reason why NavigatorUtils needs to be ref-counted.

[gyuyoung-inactive, 2014-04-11 16:26:22]

On 2014/04/11 13:06:04, haraken wrote:
> BTW, why does NavigatorUtils need to be RefCountedSupplement? I don't see any
> particular reason why NavigatorUtils needs to be ref-counted.

My original WebKit patch was to use Supplement<>. However, Hajime recommended to
use RefCountedSupplement<> instead of Supplement<> in previous my patch.
https://bugs.webkit.org/show_bug.cgi?id=90940#c10

There wasn't big reason we should use RefCountedSupplement<>. We just could get
rid of the controller if we use the RefCountedSupplement<>.

But, now I'm also not sure if RefCountedSupplement<> was correct way for the
NavigatorContentUtils. According to the his comment, Hajime thought that
RegisterProtocolHandlerController was marked as "Supplement<Navigator>". But,
original patch was "Supplement<Page>".

If there isn't any object to change to Supplement<>, I'd like to change it.

[haraken, 2014-04-11 16:28:19]

+morrita-san

[gyuyoung-inactive, 2014-04-11 18:48:47]

On 2014/04/11 16:28:19, haraken wrote:
> +morrita-san

If there is no reason we need to use RefCountedSupplement, I think Supplement is
proper for NavigatorContentUtils. I apply Supplement to new CL. If there is any
concern on that, please let me know.

[Hajime Morrita, 2014-04-12 00:23:34]

On 2014/04/11 18:48:47, gyuyoung wrote:
> On 2014/04/11 16:28:19, haraken wrote:
> > +morrita-san
> 
> If there is no reason we need to use RefCountedSupplement, I think Supplement
is
> proper for NavigatorContentUtils. I apply Supplement to new CL. If there is
any
> concern on that, please let me know.

Seems like I was confused. On the original patch. I should've said it should be
RefCountedSupplement<Navigator> (to make it sherable across navigator instances)
or Supplement<Page> (to refer the same instance through the Page object.)

anyway lgtm.

[gyuyoung-inactive, 2014-04-12 00:30:55]

On 2014/04/12 00:23:34, morrita1 wrote:
> On 2014/04/11 18:48:47, gyuyoung wrote:
> > On 2014/04/11 16:28:19, haraken wrote:
> > > +morrita-san
> > 
> > If there is no reason we need to use RefCountedSupplement, I think
Supplement
> is
> > proper for NavigatorContentUtils. I apply Supplement to new CL. If there is
> any
> > concern on that, please let me know.
> 
> Seems like I was confused. On the original patch. I should've said it should
be
> RefCountedSupplement<Navigator> (to make it sherable across navigator
instances)
> or Supplement<Page> (to refer the same instance through the Page object.)
> 
> anyway lgtm.

I didn't fully understand about it as well. Sorry about it.

[gyuyoung-inactive, 2014-04-12 10:47:35]

The CQ bit was checked by gyuyoung.kim@samsung.com

[commit-bot: I haz the power, 2014-04-12 10:47:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/235023003/40001

[commit-bot: I haz the power, 2014-04-12 10:58:57]

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

[commit-bot: I haz the power, 2014-04-12 10:58:57]

Try jobs failed on following builders:
  tryserver.blink on blink_presubmit

[haraken, 2014-04-12 15:19:58]

LGTM

https://codereview.chromium.org/235023003/diff/40001/Source/modules/navigator...
File Source/modules/navigatorcontentutils/NavigatorContentUtils.h (right):

https://codereview.chromium.org/235023003/diff/40001/Source/modules/navigator...
Source/modules/navigatorcontentutils/NavigatorContentUtils.h:57: // FIXME:
Oilpan: Move Page to the managed heap before using this trace method.

Remove this comment. Page is already on oilpan's heap.

[gyuyoung-inactive, 2014-04-12 15:24:36]

The CQ bit was checked by gyuyoung.kim@samsung.com

[gyuyoung-inactive, 2014-04-12 15:24:43]

On 2014/04/12 15:19:58, haraken wrote:
> LGTM
> 
>
https://codereview.chromium.org/235023003/diff/40001/Source/modules/navigator...
> File Source/modules/navigatorcontentutils/NavigatorContentUtils.h (right):
> 
>
https://codereview.chromium.org/235023003/diff/40001/Source/modules/navigator...
> Source/modules/navigatorcontentutils/NavigatorContentUtils.h:57: // FIXME:
> Oilpan: Move Page to the managed heap before using this trace method.
> 
> Remove this comment. Page is already on oilpan's heap.

Fixed.

[commit-bot: I haz the power, 2014-04-12 15:24:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/235023003/80001

[commit-bot: I haz the power, 2014-04-12 16:27:32]

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

[commit-bot: I haz the power, 2014-04-12 16:27:33]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[gyuyoung-inactive, 2014-04-12 22:20:28]

The CQ bit was checked by gyuyoung.kim@samsung.com

[commit-bot: I haz the power, 2014-04-12 22:20:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/235023003/80001

[commit-bot: I haz the power, 2014-04-12 23:22:14]

Change committed as 171403