1431846 - Remove nsIDOMNavigator

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Boris Zbarsky [:bzbarsky]]

It's not needed.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Kill off unused attributes from nsIDOMNavigator

MozReview-Commit-ID: HUUYPLSjWpG

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Rename nsPIDOMWindowInner::GetNavigator() to Navigator(), since it never returns null

Also have it return a dom::Navigator.

MozReview-Commit-ID: 5thGopGWIk9

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Make nsPIDOMWindowOuter::GetNavigator return dom::Navigator*

MozReview-Commit-ID: DgZWMcSg0PT

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Kill off unused attributes from nsIDOMNavigator

MozReview-Commit-ID: HUUYPLSjWpG

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Remove nsIDOMNavigator and nsIMozNavigatorNetwork

MozReview-Commit-ID: LSgeY2dM6gv

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8944120 [details] [diff] [review]
part 2.  Rename nsPIDOMWindowInner::GetNavigator() to Navigator(), since it never returns null

Review of attachment 8944120 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsMimeTypeArray.cpp
@@ +176,2 @@
>  
> +  IgnoredErrorResult rv;

Should we explicitly handle this rv value somehow? I would think we should .SuppressException() on it.

::: dom/base/nsPIDOMWindow.h
@@ +589,5 @@
>      return mMarkedCCGeneration;
>    }
>  
>    virtual nsIDOMScreen* GetScreen() = 0;
> +  virtual mozilla::dom::Navigator* Navigator() = 0;

Why not move Navigator() onto the nsPIDOMWindowInner, and avoid the virtual function call alltogether?

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8944121 [details] [diff] [review]
part 3.  Make nsPIDOMWindowOuter::GetNavigator return dom::Navigator*

Review of attachment 8944121 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpHandler.cpp
@@ +2654,5 @@
>      if (!piWindow)
>          return;
>  
> +    RefPtr<dom::Navigator> navigator = piWindow->GetNavigator();
> +    if (!navigator)

maybe add { } blocks around the return statements while you're in here?

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8944146 [details] [diff] [review]
part 4.  Remove nsIDOMNavigator and nsIMozNavigatorNetwork

Review of attachment 8944146 [details] [diff] [review]:
-----------------------------------------------------------------

beautiful

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Should we explicitly handle this rv value somehow? I would think we should .SuppressException() on it.

IgnoredErrorResult does just that in its destructor.  That's why I'm using it here.

> Why not move Navigator() onto the nsPIDOMWindowInner, and avoid the virtual function call alltogether?

Just didn't think about it.  Done.  ;)

> maybe add { } blocks around the return statements while you're in here?

Necko style doesn't have them...

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9436dd9a1265
part 1.  Kill off unused attributes from nsIDOMNavigator.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fdeaf0b28ae
part 2.  Rename nsPIDOMWindowInner::GetNavigator() to Navigator(), since it never returns null.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/1df5dfc75ebf
part 3.  Make nsPIDOMWindowOuter::GetNavigator return dom::Navigator*.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ba341846e22
part 4.  Remove nsIDOMNavigator and nsIMozNavigatorNetwork.  r=mystor

Comment 11

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/9436dd9a1265
https://hg.mozilla.org/mozilla-central/rev/0fdeaf0b28ae
https://hg.mozilla.org/mozilla-central/rev/1df5dfc75ebf
https://hg.mozilla.org/mozilla-central/rev/4ba341846e22