1305352 - [Presentation API] Implement PresentationRequestUIGlue for Fennec.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chih-Hsuan Kuo [:tommykuo]]

Implement PresentationRequestUIGlue for Fennec to support XUL browser as the presentation receiver.

Comment 1

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review commit: https://reviewboard.mozilla.org/r/82072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82072/

Comment 2

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review commit: https://reviewboard.mozilla.org/r/82074/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82074/

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

https://reviewboard.mozilla.org/r/82074/#review80632

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review81730

::: docshell/base/nsDocShell.cpp:14206
(Diff revision 1)
> +  nsCOMPtr<nsPIDOMWindowOuter> win = GetWindow();
> +  if (!win) {
> +    return NS_OK;
> +  }
> +  nsCOMPtr<Element> frameElement = win->GetFrameElementInternal();
> +  *aIsXULElement = frameElement->IsXULElement();

Please null check frameElement
(just in case things have been unlinked or so, we might be in an odd state)

::: docshell/base/nsIDocShell.idl:840
(Diff revision 1)
>    [infallible] readonly attribute boolean isInMozBrowserOrApp;
>  
>    /**
> +   * Returns true if this docshel corresponds to an <xul:browser>.
> +   */
> +  [infallible] readonly attribute boolean isXULElement;

So this works only in non-e10s. Better to document that.
Also, could you call it isInXULElement.

And the documentation says this is only about 
xul:browser, but the property returns true also for xul:iframe and xul:editor cases, so, better to fix the comment.


But. Wouldn't it make more sense to have an attribute in docshell telling whether presentation receiver is supported in it, or something like that.

Comment 5

[Chih-Hsuan Kuo [:tommykuo]]

(In reply to Olli Pettay [:smaug] from comment #4)
> ::: docshell/base/nsIDocShell.idl:840
> (Diff revision 1)
> >    [infallible] readonly attribute boolean isInMozBrowserOrApp;
> >  
> >    /**
> > +   * Returns true if this docshel corresponds to an <xul:browser>.
> > +   */
> > +  [infallible] readonly attribute boolean isXULElement;
> 
> So this works only in non-e10s. Better to document that.
> Also, could you call it isInXULElement.
> 

Sorry, I have no idea about why it only works in non-e10s. Does [infallible] cause that?

> And the documentation says this is only about 
> xul:browser, but the property returns true also for xul:iframe and
> xul:editor cases, so, better to fix the comment.
> 
> 
> But. Wouldn't it make more sense to have an attribute in docshell telling
> whether presentation receiver is supported in it, or something like that.

I also agree that. It would make the functionality of attribute more clear.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Tommy Kuo [:KuoE0] from comment #5)
> > 
> 
> Sorry, I have no idea about why it only works in non-e10s. Does [infallible]
> cause that?
I mean the top level docshell in child process doesn't have access to its parent xul:browser element (since xul:browser lives in the parent process).
So the property doesn't exactly tell anything too useful in e10s.

Comment 7

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/1-2/

Comment 8

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/1-2/

Comment 9

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/2-3/

Comment 10

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/2-3/

Comment 11

[Chih-Hsuan Kuo [:tommykuo]]

Hi S.C., I think the part 2 also need your review. Thanks!

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review84564

::: docshell/base/nsDocShell.h:1059
(Diff revision 3)
>  
>    // Notify consumers of a search being loaded through the observer service:
>    void MaybeNotifyKeywordSearchLoading(const nsString& aProvider,
>                                         const nsString& aKeyword);
>  
> +  bool IsInXULElement();

Could you add a comment that this check doesn't cross process boundary.

::: docshell/base/nsDocShell.cpp:14279
(Diff revision 3)
> +
> +  return frameElement->IsXULElement();
> +}
> +
> +NS_IMETHODIMP
> +nsDocShell::IsAllowedPresentation(nsIURI* aDocURI, bool *aIsAllowedPresentation)

nit, 
bool* aIsAllowedPresentation

::: docshell/base/nsDocShell.cpp:14281
(Diff revision 3)
> +}
> +
> +NS_IMETHODIMP
> +nsDocShell::IsAllowedPresentation(nsIURI* aDocURI, bool *aIsAllowedPresentation)
> +{
> +  // Grant access to browser receiving pages and their same-origin iframes. (App

I don't understand how this works in e10s. Shouldn't you add also some check whether the docshell is the top level docshell under TabChild?

Note, nsContentUtils::GetPresentationURL does deal with e10s already, so I don't know why we'd break e10s support here.

::: dom/presentation/PresentationSessionInfo.cpp:1431
(Diff revision 3)
> +    return NS_OK;
> +  }
> +
> +  // try to detect xul browser element
> +  nsXULElement* xulbrowser;
> +  rv = UNWRAP_OBJECT(XULElement, obj, xulbrowser);

Hmm, couldn't you above just UNWRAP to Element, and then QueryInterface to nsIFrameLoaderOwner.

Comment 13

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

https://reviewboard.mozilla.org/r/82074/#review84950

The lifecycle of PresentationView is the major issue to fix.

::: mobile/android/chrome/content/PresentationView.js:30
(Diff revision 3)
> +      DEBUG && log("Got observe: aTopic=" + aTopic);
> +
> +      let requestData = JSON.parse(aData);
> +      switch (requestData.type) {
> +        case "presentation-launch-receiver":
> +          if (this._id == requestData.windowId) {

Use early-return style to reduce indentation.

```
if (this._id !== requestData.windowId) {
  return;
}
```

::: mobile/android/chrome/content/PresentationView.js:45
(Diff revision 3)
> +                                          JSON.stringify(responseData));
> +          }
> +          break;
> +      }
> +    };
> +    Services.obs.addObserver(handleMozPresentationChromeEvent,

I'll expect the PresentationView.js have its own lifecycle, remove the observer while presentation terminated.

::: mobile/android/components/PresentationRequestUIGlue.js:9
(Diff revision 3)
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +"use strict"
> +
> +var DEBUG = true;

change to false before check-in

::: mobile/android/components/PresentationRequestUIGlue.js:32
(Diff revision 3)
> +
> +    let localDevice;
> +    try {
> +      localDevice = aDevice.QueryInterface(Ci.nsIPresentationLocalDevice);
> +    } catch (e) {
> +      DEBUG && log("Not an 1-UA device.")

Add a comment that describe this restriction is temporary and should be removed once we decide to support 2-UAs receiver in Fennec.

::: mobile/android/components/PresentationRequestUIGlue.js:40
(Diff revision 3)
> +
> +    return new Promise((aResolve, aReject) => {
> +
> +      let handleMozPresentationContentEvent = (aSubject, aTopic, aData) => {
> +        let data = JSON.parse(aData);
> +        if (data.id != aSessionId) {

I'll prefer creating a local, unique "requestId" for identifying each request instead of using sessionId. In this case you can avoid depending on the assumption of "no two sendRequest is called with the same sessionId", which might not hold true in very complicated fail/retry scenario.

::: mobile/android/components/PresentationRequestUIGlue.js:47
(Diff revision 3)
> +        }
> +
> +        Services.obs.removeObserver(handleMozPresentationContentEvent,
> +                                    "mozPresentationContentEvent");
> +
> +        switch(data.type) {

`data.result` should be more clear and you can use simpler value like "success"/"error".

::: mobile/android/components/PresentationRequestUIGlue.js:61
(Diff revision 3)
> +      Services.obs.addObserver(handleMozPresentationContentEvent,
> +                               "mozPresentationContentEvent",
> +                               false);
> +
> +      let data = {
> +        type: "presentation-launch-receiver",

no need to provide extra type here since we don't multiplex this notification topic with other events.

::: mobile/android/components/PresentationRequestUIGlue.js:62
(Diff revision 3)
> +          url: aURL,
> +          id: aSessionId,
> +          windowId: localDevice.windowId

nit: correct alignment

::: mobile/android/components/PresentationRequestUIGlue.js:66
(Diff revision 3)
> +        type: "presentation-launch-receiver",
> +          url: aURL,
> +          id: aSessionId,
> +          windowId: localDevice.windowId
> +      };
> +      Services.obs.notifyObservers(null, "mozPresentationChromeEvent", JSON.stringify(data));

ChromeEvent/ContentEvent are old naming convention for B2G. I'll suggest to rename it like:
- presentation-receiver:launch
- presentation-receiver:launch:response

Comment 14

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/3-4/

Comment 15

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/3-4/

Comment 16

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review84564

I decided to move the logic of isAllowedPresentation back to Presentation.cpp to prevent to increase too much LOC in DocShell.cpp.

> I don't understand how this works in e10s. Shouldn't you add also some check whether the docshell is the top level docshell under TabChild?
> 
> Note, nsContentUtils::GetPresentationURL does deal with e10s already, so I don't know why we'd break e10s support here.

I added the e10s support for that!

Comment 17

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/4-5/

Comment 18

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/4-5/

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review85542

Fix the issue in nsFrameLoader and explain why using GetSameTypeRootTreeItem example I added wouldn't be enough.
If GetSameTypeRootTreeItem approach would be enough, then this all would become a lot simpler. Just few lines of code in Presentation.cpp, and nothing else.

::: dom/base/nsFrameLoader.cpp:3391
(Diff revision 5)
>  
>    nsCOMPtr<nsIDocShell> docShell = mOwnerContent->OwnerDoc()->GetDocShell();
>    nsCOMPtr<nsILoadContext> parentContext = do_QueryInterface(docShell);
>    NS_ENSURE_STATE(parentContext);
>  
> +  bool isInXULElement = docShell->GetIsInXULElement();

This can't work. In Firefox ownerContent's docshell is the top level docshell which isn't inside any xul elements.
What you may want here is 
bool isInXULElement = mOwnerContent->IsXULElement();

::: dom/presentation/Presentation.cpp:61
(Diff revision 5)
>    nsCOMPtr<nsIDocShell> docshell = inner->GetDocShell();
>    if (!docshell) {
>      return false;
>    }
>  
> -  if (!docshell->GetIsInMozBrowserOrApp()) {
> +  if (!docshell->GetIsInMozBrowserOrApp() && !docshell->GetIsInXULElement()) {

I still wonder if "IsInXULElement" is the right question to ask.

Don't you want more like
"Is the top level content docshell"?
That would be way simpler to implement than all this IsInXULElement flag propagation.

Something like the following should work both in e10s and in non-e10s:

bool isTopLevelContentShell = false;
nsCOMPtr<nsILoadContent> lc = do_QueryInterface(docShell);
bool isContent = false;
lc->GetIsContent(&isContent);
if (isContent) {
  nsCOMPtr<nsIDocShellTreeItem> root;
  docShell->GetSameTypeRootTreeItem(getter_AddRefs(root));
  isTopLevelContentShell = root == docshell;
}


Would that be enough here?
(Sorry that I didn't think this before)

Comment 20

[Chih-Hsuan Kuo [:tommykuo]]

Actually, we don't care about whether it is the top level docshell. We just wanna know whether it is a XUL element. In our work, I launch the PresentationView.xul (see the patch on Bug 1285870) on Fennec. And use the <xul:browser> as the presentation receiver. We just need to let the <xul:browser> can pass the HasReceiverSupport check.

To handle the e10s and non-e10s, I think if we are in content process and can not get the frame element, we should query the IsInXULElement info from TabChild. Else, we query the IsXULElement from frame element directly.

Comment 21

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

https://reviewboard.mozilla.org/r/82074/#review85798

r+ with my comments addressed.

::: mobile/android/chrome/content/PresentationView.js:1
(Diff revision 5)
>  /* This Source Code Form is subject to the terms of the Mozilla Public

nit: add mode line

::: mobile/android/chrome/content/PresentationView.js:57
(Diff revision 5)
> +    Services.obs.notifyObservers(browser,
> +                                 PRESENTATION_RECEIVER_LAUNCH_RESPONSE_TOPIC,
> +                                 JSON.stringify({ result:    "success",
> +                                                  requestId: requestData.requestId }));

Move this into try block.

::: mobile/android/chrome/content/PresentationView.js:59
(Diff revision 5)
> +                                                    reason: e.message }));
> +    }
> +
> +    Services.obs.notifyObservers(browser,
> +                                 PRESENTATION_RECEIVER_LAUNCH_RESPONSE_TOPIC,
> +                                 JSON.stringify({ result:    "success",

nit: remove extra space before "success".

::: mobile/android/components/PresentationRequestUIGlue.js:1
(Diff revision 5)
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

nit: fix mode line, see https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Mode_Line

::: mobile/android/components/PresentationRequestUIGlue.js:65
(Diff revision 5)
> +      };
> +      Services.obs.addObserver(handleObserve,

nit: add extra line after `};`.

::: mobile/android/components/PresentationRequestUIGlue.js:70
(Diff revision 5)
> +      let data = { url:       aURL,
> +                   windowId:  localDevice.windowId,
> +                   requestId: requestId };

I'll prefer style like:

```
let data = {
  url: aURL,
  windowId: localDeivce.windowId,
  requestId: requestId,
};
```

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Tommy Kuo [:KuoE0] from comment #20)
> Actually, we don't care about whether it is the top level docshell. We just
> wanna know whether it is a XUL element. In our work, I launch the
> PresentationView.xul (see the patch on Bug 1285870) on Fennec. And use the
> <xul:browser> as the presentation receiver. We just need to let the
> <xul:browser> can pass the HasReceiverSupport check.
> 
> To handle the e10s and non-e10s, I think if we are in content process and
> can not get the frame element, we should query the IsInXULElement info from
> TabChild. Else, we query the IsXULElement from frame element directly.
Ok. But why top level content docshell isn't good enough for this?

Comment 23

[Chih-Hsuan Kuo [:tommykuo]]

+---+
| A | PresentationView.xul
+-+-+
  |
  |   +---+
  +---> B | <xul:browser>
      +-+-+
        |
 +---+  |  +---+
 | C <--+--> D |
 +---+     +---+

Actually, I didn't understand the method of top-level content docshell. I think I should explain what I want to do in detail. In this figure, A is the top-level window (PresentationView.xul) and B is the <xul:browser> element in PresentationView.xul. C & D may be some other <iframe> in the page we opened by B.

If some page uses Presentation API as the receiver in B, it should be passed the check. And if it uses Presentation API as the receiver in C or D, it should not be passed the check.

So, if the method of top-level content docshell can totally do that, I think that would be a good way!

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

I don't understand that picture. Is B (1) some web page under <xul:browser> or (2) is B xul:browser?

If (1) and xul:browser has type="content...", top level content docshell is same as B.

Comment 25

[Chih-Hsuan Kuo [:tommykuo]]

B is the <xul:browser> and I set the "src" attribute to make it load some page as the presentation receiver.

Comment 26

[Olli Pettay [:smaug][[e-mail]]]

So nothing represent the docshell underneath xul:browser? And C and D are iframes... so where do those iframes live.

But I think we're talking about the same thing. xul:browser owns an nsFrameLoader which owns
in non-e10s a DocShell which contains the top level web page, and in e10s nsFrameLoader owns TabParent which then has connection to TabChild in child process and TabChild owns the docshell for the top level web page there.

Comment 27

[Chih-Hsuan Kuo [:tommykuo]]

Hi smaug, the top level content docshell works well. Please review again, thanks.

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

hmm, did you update the patch? I'm still seeing Presentation::HasReceiverSupport using && !docshell->GetIsInXULElement()).

What does the spec say about Presentation.receiver? Should it be hidden in non-top-level receiving web pages or just null?
Looks like "In any other browsing context, it MUST return null". So adding the check to 
HasReceiverSupport isn't right, but it should be in Presentation::GetReceiver().
Could you add a test for it - perhaps test could have an iframe in the receiver side and inside iframe 'receiver' property should still be there in Presentation object, but just returning null.

Comment 29

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Olli Pettay [:smaug] (reviewing overload, queue closed for a day or two) from comment #28)
> hmm, did you update the patch? I'm still seeing
> Presentation::HasReceiverSupport using && !docshell->GetIsInXULElement()).
> 
> What does the spec say about Presentation.receiver? Should it be hidden in
> non-top-level receiving web pages or just null?
> Looks like "In any other browsing context, it MUST return null". So adding
> the check to 
> HasReceiverSupport isn't right, but it should be in
> Presentation::GetReceiver().
> Could you add a test for it - perhaps test could have an iframe in the
> receiver side and inside iframe 'receiver' property should still be there in
> Presentation object, but just returning null.

There is already a test case for that: see https://dxr.mozilla.org/mozilla-central/source/dom/presentation/tests/mochitest/file_presentation_receiver.html#72-90

Comment 30

[Olli Pettay [:smaug][[e-mail]]]

ahaa, but wouldn't the current patch break it?

And, per spec, shouldn't .receiver be null (but not undefined!) in iframes, since inside iframe the browsing context isn't "receiving browsing context"?

Comment 31

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/5-6/

Comment 32

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/5-6/

Comment 33

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/6-7/

Comment 34

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/6-7/

Comment 35

[Chih-Hsuan Kuo [:tommykuo]]

I forgot to publish the changes, and the latest patch is there, now. I also ran the test and no test case is broken.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review88482

::: dom/base/nsContentUtils.cpp:9371
(Diff revision 7)
>  
>    topFrameElement->GetAttribute(NS_LITERAL_STRING("mozpresentation"), aPresentationUrl);
>  }
>  
> +/* static */ bool
> +nsContentUtils::IsTopLevelContentDocShell(nsIDocShell* aDocShell)

hmm, this is ok too, but would be even simpler to add
[infallible] attribute boolean IsTopLevelContentDocShell() 
to nsIDocShell. And there the implementation could be something like

NS_IMETHODIMP
nsDocShell::IsTopLevelContentDocShell(bool* aRetVal)
{
  if (mItemType == typeContent) {
    nsCOMPtr<nsIDocShellTreeItem> root;
    GetSameTypeRootTreeItem(getter_AddRefs(root));
    *aRetVal = root.get() == static_cast<nsIDocShellTreeItem*>(this)
  } else {
    *aRetVal = false;
  }
  return NS_OK;
}

::: dom/presentation/Presentation.cpp:62
(Diff revision 7)
>    if (!docshell) {
>      return false;
>    }
>  
> -  if (!docshell->GetIsInMozBrowserOrApp()) {
> +  if (!docshell->GetIsInMozBrowserOrApp() &&
> +      !nsContentUtils::IsTopLevelContentDocShell(docshell)) {

So, this is still wrong, as far as I see.
iframes under the top level receiver page should have presentation.receiver == null;, not
presentation.receiver == undefined.

Comment 37

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review88482

> So, this is still wrong, as far as I see.
> iframes under the top level receiver page should have presentation.receiver == null;, not
> presentation.receiver == undefined.

I filed bug 1314229 to do that and schien will fix it.

Comment 38

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/7-8/

Comment 39

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/7-8/

Comment 40

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

https://reviewboard.mozilla.org/r/82072/#review89264

Comment 41

[Chih-Hsuan Kuo [:tommykuo]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d871bdab43f23f1faac6e838b460ceac2254b3e

Comment 42

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796113 [details]
Bug 1305352 - (Part 1) Make Presentation API support XUL browser element.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82072/diff/8-9/

Comment 43

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8796114 [details]
Bug 1305352 - (Part 2) Implement PresentationRequestUIGlue on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82074/diff/8-9/

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a64e70dc94f7
(Part 1) Make Presentation API support XUL browser element. r=smaug
https://hg.mozilla.org/integration/autoland/rev/47f86091dedd
(Part 2) Implement PresentationRequestUIGlue on Fennec. r=schien,snorp

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a64e70dc94f7
https://hg.mozilla.org/mozilla-central/rev/47f86091dedd