Bug 116312 – [WK2][WTR]Adding Layout test framework for MHTML page content

• Status: UNCONFIRMED
• Component: Tools / Tests

[Santosh Mahto]

Currently There is no support for layout test for page  content in MHTML format.
Currently  WKPageGetContentsAsMHTMLData has been exposed in WK2 which is used to get the
content in MHTML format.

This bug add  layout test framework  for testing page content in MHTMLArchive format.
it exposed testcase javascript API  testRunner.dumpAsMHTMLArchive. and compare with -expectd.mht file

Currently the Work is in process

[Santosh Mahto]

Created attachment 202079 [details]
Patch

[Santosh Mahto]

@ Benjamin p

Hi
This is initial set of modification wk2.

I am facing problem in modifying webkitpy/ related python code
I followed the webarchive  and added .mht in .webarvhive place
but test still seems to not working.

I want it to be a tex diff but as I see log 
---------------------------------------------------------------------

./Tools/Scripts/run-webkit-tests --efl --debug -v LayoutTests/mhtml/simple.html  -2 --details

[1/1] mhtml/simple.html
  txt: <none>
  png: <none>
  wav: <none>
  webarchive: <none>
  mht: mhtml/simple-expected.mht  ---> its recognising my expected file 
  exp: PASS
  got: IMAGE     ----> WHY IMAGE?? it seems it is going for image difftest
 took: 3.922
-----------------------------------------------------------------------

can you give me more clue(or some guy name) in webkitpy side modification

[Santosh Mahto]

@Benjamin 
It seems
On above patch it is making a Image Diff as I could see it is making image diff
in test result in bowser screen
Instead It should make text diff 
Could you help me why it is going for Image diff

[Zan Dobersek]

The issue here is that the same list of extensions is used to find both all the test cases and the reference files for the reftests.

In this case, the layout test that dumps the MHTML archive gets the corresponding .mht baseline recognized as a reference file. Hence the image mismatch.

The lists should be split. I'll upload the patch soon, in a separate bug.

[Santosh Mahto]

(In reply to comment #4)
> The issue here is that the same list of extensions is used to find both all the test cases and the reference files for the reftests.
> 
> In this case, the layout test that dumps the MHTML archive gets the corresponding .mht baseline recognized as a reference file. Hence the image mismatch.
> 
> The lists should be split. I'll upload the patch soon, in a separate bug.

Thanks Zan Dobersek for taking this issue forward.
The naming extesnion of baseline expected mhtml file depends on
decision on this bug https://bugs.webkit.org/show_bug.cgi?id=116333

[Santosh Mahto]

Created attachment 202228 [details]
Patch

[Santosh Mahto]

simple.html --> test page
simple-expected.txt --> basedline expected

[Santosh Mahto]

Created attachment 202253 [details]
Patch

[Benjamin Poulain]

Comment on attachment 202253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202253&action=review

This is a start, but that is not the proper way to test code with an API on the UIProcess.

Instead, what should happen is the InjectedBundle sends a message to the UIProcess asking for the data. The UIProcess uses the public API (WKPageGetContentsAsMHTMLData) and return the value to the injected bundle asynchronously.

On a more generic standpoint. Why wouldn't the test always dump 2 expected results? One binary, one text?

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:325
> +    bool m_useBinaryEncoding;

Instead of this, I would just have two WhatToDump:
    MHTMLArchive && MHTMLBinaryArchive

[Santosh Mahto]

(In reply to comment #9)
> (From update of attachment 202253 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202253&action=review
> 
> This is a start, but that is not the proper way to test code with an API on the UIProcess.

 I thought this is last :):)

> Instead, what should happen is the InjectedBundle sends a message to the UIProcess asking for the data. The UIProcess uses the public API (WKPageGetContentsAsMHTMLData) and return the value to the injected bundle asynchronously.

 you seems right.
> On a more generic standpoint. Why wouldn't the test always dump 2 expected results? One binary, one text?

 we don't need to dump two when layout test will just compare with one base line expected text file.
and we are doing content test, to do encoding test(binary) we dont need to duplicate everything and adding differnt archive format

binary specifies only that content(main resource  & subresources) to be encoded in binary or not , MHTML archive format(headers) will still be in same format . MHTMLBinaryArchive is  not any archive format.
So there is one Archive format MHTML where  data content(e.g image) are encoded with ascii(default) or Binary(if sspecified m_useBinaryEncoding = true)

its like in webarchive  <data>  </data> encoding is decided by binary but archive format is always XML 

> > Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:325
> > +    bool m_useBinaryEncoding;

         This doesnot interfere with MHTML archive format, just changes the encoding on inside content resources

> Instead of this, I would just have two WhatToDump:
>     MHTMLArchive && MHTMLBinaryArchive

 MHTMLArchive && MHTMLBinaryArchive  are not different archive as I said above.
specifiying two archive format name looks confusing to me and it will lead to code duplicate(redo everything again for MHTMLBinaryArchive.

For MHTML layout tests means we are testing for content of MHTML dump.So its not must seems to mix up encoding test 

So I think one javascript api  dumpAsMHTML(bool m_useBinaryEnCoding = false) is good enough

[Santosh Mahto]

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 202253 [details] [details])

> > Instead, what should happen is the InjectedBundle sends a message to the UIProcess asking for the data. The UIProcess uses the public API (WKPageGetContentsAsMHTMLData) and return the value to the injected bundle asynchronously.

it seems to me from code study that WebKitTestRunner injectedBundleClient (InjectedBundlePage.cpp)  is meant to talk to webprocess injectedBundle only .
It is not meant to access UIProcess directly

Although I am not 100% sure. So please update your view

[Santosh Mahto]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (From update of attachment 202253 [details] [details] [details])
> 
> > > Instead, what should happen is the InjectedBundle sends a message to the UIProcess asking for the data. The UIProcess uses the public API (WKPageGetContentsAsMHTMLData) and return the value to the injected bundle asynchronously.
> 

InjectedBundlePage.cpp is meant to access only WebProcess not UI process.
The concept of injected bundle it to inject(use) the code of WebProcess without using UIprocess
If I got through UI process it will clear violation of InjectedBundle concept.
InjectedBundlePage.cpp  is meant to call InjectedBundle of WebProcess.

Thats why all kind of dump done by webkittestrunner (webarchive, text) diectly 
use WebProcess.
For WebArchive  There is wk2 api also  WKFrameGetWebArchive(WKFrameRef in WkFrame.cpp, but injectedBundle use directly webprocess as per rules.

Anyway UIprocess Api should be tested by unittest seperatly

So I am confident of using webprocess directly

[Santosh Mahto]

Created attachment 203169 [details]
Patch

[Benjamin Poulain]

Comment on attachment 203169 [details]
Patch

Same reasons as before.

[Benjamin Poulain]

> InjectedBundlePage.cpp is meant to access only WebProcess not UI process.
> The concept of injected bundle it to inject(use) the code of WebProcess without using UIprocess
> If I got through UI process it will clear violation of InjectedBundle concept.
> InjectedBundlePage.cpp  is meant to call InjectedBundle of WebProcess.
> 
> Thats why all kind of dump done by webkittestrunner (webarchive, text) diectly 
> use WebProcess.
> For WebArchive  There is wk2 api also  WKFrameGetWebArchive(WKFrameRef in WkFrame.cpp, but injectedBundle use directly webprocess as per rules.
> 
> Anyway UIprocess Api should be tested by unittest seperatly
> 
> So I am confident of using webprocess directly

See TestRunner::setPageVisibility(). That will give you an idea of how those stuff are run.

[Santosh Mahto]

Created attachment 203485 [details]
Patch

[Santosh Mahto]

> 
> Instead, what should happen is the InjectedBundle sends a message to the UIProcess asking for the data. The UIProcess uses the public API (WKPageGetContentsAsMHTMLData) and return the value to the injected bundle asynchronously.
@Benjamin :
The patch is uploaded according to above requirement.

[Early Warning System Bot]

Comment on attachment 203485 [details]
Patch

Attachment 203485 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/717305

[Santosh Mahto]

Created attachment 203486 [details]
Patch

[Early Warning System Bot]

Comment on attachment 203486 [details]
Patch

Attachment 203486 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/723522

[Santosh Mahto]

Created attachment 203488 [details]
Patch

[Santosh Mahto]

Created attachment 203914 [details]
Patch

[Brady Eidson]

Comment on attachment 203914 [details]
Patch

Assuming that patches for review since 2013 are stale, r-