1187757 - [Stingray][Contribution] Customize UA String per App context

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Yadani]

+++ This bug was initially created as a clone of Bug #1181919 +++

Stingray contribution - A function to customize User-Agent string per app context

Comment 1

[Yadani]

In the internal discussion in bug 1181919, the following feature is proposed as stingray contribution.

- Adding an API on the <iframe mozbrowser> API to set the full User-Agent string.
- The API changes the User-Agent string in HTTP header of network connection in the <iframe mozbrowser>.
- The API changes the 'window.navigator.userAgent' value in the <iframe mozbrowser>'s JS context.

Comment 2

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

Attachment: [WIP] modify-ua-string-browser-api.patch

WIP of introducing BrowserElement.setFullUAString().

The usage will be like:

var browserFrame = document.createElement("iframe");
browserFrame.mozbrowser = true;
browserFrame.setFullUAString("some-customized-ua-string");
browserFrame.src = "url-to-server-required-customized-ua-string";
document.appendChild(browserFrame);

Comment 3

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

Attachment: [WIP] modify-ua-string-browser-api.patch

This is a more comprehensive WIP patch for @yadani to follow-up.

[Architecture]
We can leverage existing UserAgentOverrides.jsm to handle the US String modification while composing HTTP header. BrowserElementParent.js will provide the mapping table for any browser element that has customized UA string. BrowserElementChild.js will cache the customized UA string for |navigator.userAgent|. UserAgentOverrides.jsm can use aHttpChannel.owner to generate the key for querying the mapping table.

[TODO]
1. cache the customized UA string and retrieve the cached value in Navigator.
2. clean up the entry for dead browser element in UserAgentOverrides.jsm
3. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element.

Comment 4

[Yadani]

Attachment: [WIP] 1187757-wip-20151112.patch

Customize UserAgent string in HTTP header by setFullUAString.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element. (Not checked for case 2)
3. fix that the UserAgent in http connection for getting file_browserElement_SetFullUAString.js is not changed.
4. customize 'naviagator.userAgent' value.

Comment 5

[Yadani]

Attachment: 20151112_Atchitecture.PNG

Supplementary information　for comment 4.

[Architecture]
- Introduce serial number in nsFrameLoader.
  - The serial number differs by each nsFrameLoader instance.
- Use the serial number as |context|, the key of gBrowserElementUAs.
  - Because nsFrameLoader connects to both nsBrowserElement and nsHttpChannel as attached chart.
    (Actually I think it is complicated architecture. I'm happy if someone kindly informs me better idea.)

Comment 6

[Yadani]

Hello SC-san,
I'm grad if you could check the architecture of comment 5.
> UserAgentOverrides.jsm can use aHttpChannel.owner to generate the key for querying the mapping table.
Actually, I cannot find how to share information between nsBrowserElement and nsHttpChannel by aHttpChannel.owner.

Comment 7

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

(In reply to Yadani from comment #6)
> Hello SC-san,
> I'm grad if you could check the architecture of comment 5.
> > UserAgentOverrides.jsm can use aHttpChannel.owner to generate the key for querying the mapping table.
> Actually, I cannot find how to share information between nsBrowserElement
> and nsHttpChannel by aHttpChannel.owner.

Hi Yadani-san, the reason you add the UAContext is for identifying browser element in UserAgentOerrides.jsm, right? It's kind scary to see how many components are touched by this modification. I'll need take sometime to evaluate if there is any other way to do that.

Comment 8

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

ni? myself to remember this task.

Comment 9

[Yadani]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #7)
> Hi Yadani-san, the reason you add the UAContext is for identifying browser
> element in UserAgentOerrides.jsm, right?
Hello SC-san,
Yes. Thank you very much for you help.

Comment 10

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

Maybe @kanru can also take a look at comment #5 first and we can discuss it this week.

Comment 11

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

Here is the suggestion:

1. Use TabId as the mapping identifier. This is the unique/sync ID in both parent and child process. We can also allocate an TabId for nsFrameLoader that has no TabParent.
https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentProcessManager.h#80
@kanru filed bug 1226535 for this part of change. You can split your patch and can parallelize the review process.

2. Store and pass the TabId via nsILoadInfo because it is already passed between processes during channel initialization. Only need to add some code for nsILoadInfo serialization.
https://dxr.mozilla.org/mozilla-central/source/netwerk/ipc/NeckoChannelParams.ipdlh#27

3. UserAgentOverrides.jsm can get the TabId from nsIChannel.loadInfo, and use it for table lookup.

Comment 12

[Yadani]

Attachment: 20151126_Atchitecture.PNG

Thank you for your suggestion. For my understanding, I updated architecture chart.
I'll try to revise patch with the architecture.

Comment 13

[Yadani]

Attachment: [WIP] 1187757-wip-20151130.patch

Update based on new architecture.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element. (Not checked for case 2)
3. fix that the UserAgent in http connection for getting file_browserElement_SetFullUAString.js is not changed.
4. customize 'naviagator.userAgent' value.
5. split patch for bug 1226535.

Comment 14

[Yadani]

Attachment: [WIP] 1187757-wip-20151209.patch

Customize both UserAgent string in HTTP header and 'navigator.userAgent' value.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element. (Not checked for case 2)
3. fix that the UserAgent in http connection for getting file_browserElement_SetFullUAString.js is not changed.
4. split patch for bug 1226535.

Comment 15

[Yadani]

Supplementary information　for comment 14.

[Architecture]
To customize 'navigator.userAgent' value, we introduce a new attribute 'uaString' in docShell. 'uaString' stores the updated user agent string.
From BrowserElementChildPreload.js, we can access uaString via docShell.
From Navigator.cpp, we can access uaString via aWindow->GetDocShell().

Comment 16

[Yadani]

Attachment: [WIP] 1187757-wip-20151215.patch

Split patch for bug 1226535.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element. (Not checked for case 2)
3. fix that the UserAgent in http connection for getting file_browserElement_SetFullUAString.js is not changed.

Comment 17

[Yadani]

Attachment: [WIP] 1187757-wip-20151221.patch

Fix that the UserAgent in http connection for getting file_browserElement_SetFullUAString.js is not changed.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. define the |context| as a key to identify browser element in UserAgentOerrides.jsm. The representative object of a browser element in b2g process can be 1) window object for an in-process browser element, or 2) TabParent object for an out-of-process browser element. (Not checked for case 2)

Comment 18

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

Hi Yadani-san, thanks for keeping update the patch. Feel free to use f? if you want @kanru or me to take a look at your current work.

Comment 19

[Yadani]

Hello SC-san, thank you. Next time I'll try f? flag. I've not noticed the flag...

Comment 20

[Yadani]

Attachment: [WIP1187757-wip-20160107.patch

Comment 21

[Yadani]

Attachment: [WIP] 1187757-wip-20160107.patch

Rebase to master. (Sorry, this patch is basically same as comment 17)

Comment 22

[Yadani]

Attachment: [WIP] 1187757-wip-20160204.patch

Add oop test.

[TODO]
1. clean up the entry for dead browser element in UserAgentOverrides.jsm
2. fix that if browser element is out-of-process, the UA in http header is not changed

Comment 23

[Yadani]

Attachment: 20160204_Atchitecture.png

Hello SC-san,
I'm trying to solve TODO 2 in Comment 22.
After examination, I found that setting TabId to gBrowserElementUAs map is processed in b2g process (red box in attached image), but setting TabId to LoadInfoArgs is processed in plugin-container process (blue box in attached image). Currently TabIds are different between b2g and plugin-container, it leads to TabId mismatch. So UA overriding mechanism doesn't work.

To solve this problem, I think that we need some mechanism to share a identifier in nsFrameLoader between b2g process and plugin-container process. And we may use the identifier as TabId. Is my understanding correct?

Comment 24

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

Sorry for not being responsive last few weeks. I'll provide my feedback in these two days. (keep ni? flag for notification)

Comment 25

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

Attachment: tab-id-allocation.jpg

TabId allocation and passing flow for three condition:
1. in-process <iframe> in b2g process
2. oop <iframe> in b2g process
3. <iframe> on content process

Comment 26

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

(In reply to Yadani from comment #23)
> Created attachment 8715772 [details]
> 20160204_Atchitecture.png
> 
> Hello SC-san,
> I'm trying to solve TODO 2 in Comment 22.
> After examination, I found that setting TabId to gBrowserElementUAs map is
> processed in b2g process (red box in attached image), but setting TabId to
> LoadInfoArgs is processed in plugin-container process (blue box in attached
> image). Currently TabIds are different between b2g and plugin-container, it
> leads to TabId mismatch. So UA overriding mechanism doesn't work.
> 
> To solve this problem, I think that we need some mechanism to share a
> identifier in nsFrameLoader between b2g process and plugin-container
> process. And we may use the identifier as TabId. Is my understanding correct?

Hi Yadani-san, nsHttpChannel is always on b2g process and the tabId you get from "ContentParent::AllocateTabId" in bug 1226535 is actually generated by b2g process. Is the scenario you described matched with the condition #3 in comment #25?

Comment 27

[Yadani]

Hello SC-san, I found the problem while testing of test_browserElement_oop_SetFullUAString.html (mochitest-oop.ini). If my understanding of mochitest-oop.ini is correct, I think the scenario in comment 23 matches with the condition #2 in comment #25 (test_browserElement_oop_SetFullUAString.html is parent process, and file_browserElement_SetFullUAString.htm (called by browserElement_SetFullUAString.js) is child process).

tab-id-allocation.jpg means that
- On condition #2, 'Id' (==TabId) is shared among TabParent, TabChilld, HTTPChannelParent, HTTPChannelChild and nsHttpChannel, and the 'Id' can be used as the |context| as a key to identify browser element.
- On condition #3, 'Id' (==TabId) is shared among nsFrameLoader, HTTPChannelParent, HTTPChannelChild, nsHTTPChannel, and the 'Id' can be used as the |context| as a key to identify browser element.
Is it right?

Comment 28

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

(In reply to Yadani from comment #27)
> Hello SC-san, I found the problem while testing of
> test_browserElement_oop_SetFullUAString.html (mochitest-oop.ini). If my
> understanding of mochitest-oop.ini is correct, I think the scenario in
> comment 23 matches with the condition #2 in comment #25
> (test_browserElement_oop_SetFullUAString.html is parent process, and
> file_browserElement_SetFullUAString.htm (called by
> browserElement_SetFullUAString.js) is child process).
> 
> tab-id-allocation.jpg means that
> - On condition #2, 'Id' (==TabId) is shared among TabParent, TabChilld,
> HTTPChannelParent, HTTPChannelChild and nsHttpChannel, and the 'Id' can be
> used as the |context| as a key to identify browser element.
> - On condition #3, 'Id' (==TabId) is shared among nsFrameLoader,
> HTTPChannelParent, HTTPChannelChild, nsHTTPChannel, and the 'Id' can be used
> as the |context| as a key to identify browser element.
> Is it right?

Yes that's right. I double check your patch and I think the error you encountered is because you get TabId from frameloader but not TabParent in dom/html/nsBrowserElement.cpp .

Comment 29

[Yadani]

Hello SC-san, Thank you for your advice. I'll try to connect nsBrowserElement and TabParent to pass tabId information from TabParent to nsBrowserElement.

Comment 30

[Yadani]

Attachment: [WIP] 1187757-wip-20160323.patch

Now works on oop <iframe> in b2g process.
[TODO]
1. Add test.
2. Remove temporal code.

Comment 31

[Yadani]

Attachment: 20160323_Architecture.png

Update architecture.

Comment 32

[Yadani]

Attachment: [WIP] 1187757-wip-20160329.patch

Remove temporal code.
[TODO]
1. Add test.

Comment 33

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

Comment on attachment 8735796 [details] [diff] [review]
[WIP] 1187757-wip-20160329.patch

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

@kanru can you also take a look at this patch? it contains more code then I expected but I couldn't tell if it is correct or not.

::: netwerk/base/LoadInfo.cpp
@@ +116,5 @@
> +        sOuterWindowIDToTabID[mOuterWindowID] = tabId;
> +        mTabID = tabId;
> +      } else if (sOuterWindowIDToTabID.count(mOuterWindowID) > 0) {
> +        mTabID = sOuterWindowIDToTabID[mOuterWindowID];
> +      }

Can you explain more about why this mapping is necessary?

::: netwerk/ipc/NeckoChannelParams.ipdlh
@@ +122,4 @@
>    bool                        suspendAfterSynthesizeResponse;
>    bool                        allowStaleCacheContent;
>    nsCString                   contentTypeHint;
> +  uint32_t                    tabId;

why you need carry extra "tabId" here since we already have the information in LoadInfoArgs?

::: netwerk/protocol/http/HttpBaseChannel.cpp
@@ +2544,5 @@
>      nsAutoCString ua;
> +    uint64_t tabID = 0;
> +    mLoadInfo->GetTabID(&tabID);
> +    bool isUAOverridedTabId = nsBrowserElement::IsUAOverridedTabId(tabID);
> +    if (nsContentUtils::IsNonSubresourceRequest(this) || isUAOverridedTabId) {

Can you explain the purpose of this code?

::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +314,5 @@
> +  uint64_t tabID = 0;
> +  loadInfo->GetTabID(&tabID);
> +  if (tabID == 0) {
> +    loadInfo->SetTabID(aTabId);
> +  }

Why you need the "tabID == 0" check? You should always set to aTabId.

Comment 34

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

Hi Yadani-san, thanks for updating your patch. Can you check my comment in comment #33?
Thanks.

Comment 35

[Yadani]

Attachment: [WIP] 1187757-wip-20160421.patch

Comment 36

[Yadani]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #33)

> ::: netwerk/base/LoadInfo.cpp
> @@ +116,5 @@
> > +        sOuterWindowIDToTabID[mOuterWindowID] = tabId;
> > +        mTabID = tabId;
> > +      } else if (sOuterWindowIDToTabID.count(mOuterWindowID) > 0) {
> > +        mTabID = sOuterWindowIDToTabID[mOuterWindowID];
> > +      }
> 
> Can you explain more about why this mapping is necessary?

The mapping is needed to change the UA in HTTP header of files loaded by html document (e.g., js file).

> ::: netwerk/ipc/NeckoChannelParams.ipdlh
> @@ +122,4 @@
> >    bool                        suspendAfterSynthesizeResponse;
> >    bool                        allowStaleCacheContent;
> >    nsCString                   contentTypeHint;
> > +  uint32_t                    tabId;
> 
> why you need carry extra "tabId" here since we already have the information
> in LoadInfoArgs?

I removed tabId in the revised patch.

> ::: netwerk/protocol/http/HttpBaseChannel.cpp
> @@ +2544,5 @@
> >      nsAutoCString ua;
> > +    uint64_t tabID = 0;
> > +    mLoadInfo->GetTabID(&tabID);
> > +    bool isUAOverridedTabId = nsBrowserElement::IsUAOverridedTabId(tabID);
> > +    if (nsContentUtils::IsNonSubresourceRequest(this) || isUAOverridedTabId) {
> 
> Can you explain the purpose of this code?

We want to call |gHttpHandler->OnUserAgentRequest(this)| to utilize the UA modification architecture in UserAgentOverrides.jsm. But on 'in-process <iframe> in b2g process' case, |nsContentUtils::IsNonSubresourceRequest(this)| will be false. So I introduced |isUAOverridedTabId| to call |gHttpHandler->OnUserAgentRequest(this)| in 'in-process <iframe> in b2g process' case.

> ::: netwerk/protocol/http/HttpChannelParent.cpp
> @@ +314,5 @@
> > +  uint64_t tabID = 0;
> > +  loadInfo->GetTabID(&tabID);
> > +  if (tabID == 0) {
> > +    loadInfo->SetTabID(aTabId);
> > +  }
> 
> Why you need the "tabID == 0" check? You should always set to aTabId.

In the revised patch, similar code exists in |HttpChannelChild::ContinueAsyncOpen|.

On 'in-process <iframe> in b2g process' case, tabId is set at LoadInfo constructor from frameLoader. In this case, tabID != 0 in |HttpChannelChild::ContinueAsyncOpen|.
On 'oop <iframe> in b2g process' case, tabId is unknown at LoadInfo constructor so at first tabId is set to 0. When HttpChannelChild::ContinueAsyncOpen is called, we can get tabId from tabChild. In this case, tabID == 0 in |HttpChannelChild::ContinueAsyncOpen|.

Comment 37

[Yadani]

Attachment: [WIP] 1187757-wip-20160601.patch

Rebase to master.

Comment 38

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

Comment on attachment 8758589 [details] [diff] [review]
[WIP] 1187757-wip-20160601.patch

Based on current project status, it not likely that we'll take this patch into m-c.