Introduce mock test system for navigator content utils

Introduce mock test system for navigator content utils

This cl introduces setNavigatorContentUtilsClientMock() and NavigatorContentUtilsClientMock class in order to simulate registerProtocolHandler(), unregisterProtocolHandler(), and isProtocolHandlerRegistered(). Thus, navigator content utils can test if defined functions can work correctly.

Besides this cl updates all navigator content util's tests so that the functions can work correctly.

BUG=none

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

[gyuyoung-inactive, 2014-05-29 12:56:02]

Add Kentaro to reviewer. I would like to get review after finishing Issue
298273008.

https://codereview.chromium.org/298273008/

[haraken, 2014-05-29 13:01:25]

https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h
(right):

https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h:29:
*/

Use the 4-line copyright. The same comment for other files in this CL.

[gyuyoung-inactive, 2014-05-29 13:22:34]

https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h
(right):

https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h:29:
*/
On 2014/05/29 13:01:26, haraken wrote:
> 
> Use the 4-line copyright. The same comment for other files in this CL.
> 

Ok, I will fix it in next patch.

[gyuyoung-inactive, 2014-06-02 02:21:26]

On 2014/05/29 13:22:34, gyuyoung wrote:
>
https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
> File
> Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h
> (right):
> 
>
https://codereview.chromium.org/293403002/diff/20001/Source/modules/navigator...
>
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h:29:
> */
> On 2014/05/29 13:01:26, haraken wrote:
> > 
> > Use the 4-line copyright. The same comment for other files in this CL.
> > 
> 
> Ok, I will fix it in next patch.

kentaro, patch is updated. Could you take a look again ?

[haraken, 2014-06-02 03:43:29]

Just help me understand: Why do you need to expose a Mock object instead of
exposing register/unregister/isProtocolHandler APIs to window.internals?

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h
(right):

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h:8:
#include "wtf/text/WTFString.h"

Remove this.

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp
(right):

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp:29:
// "declined" state was checked by
NavigatorContentUtils::isProtocolHandlerRegistered() before calling this
function.

I don't fully understand this. Who calls
NavigatorContentUtils::isProtocolHandlerRegistered() before
NavigatorContentUtilsClientMock::isProtocolHandlerRegistered is called?

Also, NavigatorContentUtils::isProtocolHandlerRegistered() returns
"declined"/"new"/"registered" whereas
NavigatorContentUtilsClientMock::isProtocolHandlerRegistered returns an enum
value. Why do we need the difference?

[gyuyoung-inactive, 2014-06-02 05:14:37]

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h
(right):

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/InternalsNavigatorContentUtils.h:8:
#include "wtf/text/WTFString.h"
On 2014/06/02 03:43:29, haraken wrote:
> 
> Remove this.

Ok, I will do.

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
File
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp
(right):

https://codereview.chromium.org/293403002/diff/40001/Source/modules/navigator...
Source/modules/navigatorcontentutils/testing/NavigatorContentUtilsClientMock.cpp:29:
// "declined" state was checked by
NavigatorContentUtils::isProtocolHandlerRegistered() before calling this
function.
On 2014/06/02 03:43:29, haraken wrote:
> 
> I don't fully understand this. Who calls
> NavigatorContentUtils::isProtocolHandlerRegistered() before
> NavigatorContentUtilsClientMock::isProtocolHandlerRegistered is called?

isProtocolHandlerRegistered() is one of NavigatorContentUtils interfaces. So, it
will be called by javascript.(e.g.
<script>navigator.isProtocolHandlerRegistered()</script>)



> Also, NavigatorContentUtils::isProtocolHandlerRegistered() returns
> "declined"/"new"/"registered" whereas
> NavigatorContentUtilsClientMock::isProtocolHandlerRegistered returns an enum
> value. Why do we need the difference?

According to specification, "declined" is defined as "rejected".

 - declined : Indicates that the given handler has been offered but was
rejected. Trying to register the handler again may prompt the user again.
   (http://www.whatwg.org/specs/web-apps/current-work/#custom-handlers)

Current navigator content utils implementation is that "NavigatorContentUtils"
checks if given scheme is valid or not in
"NavigatorContentUtils::isProtocolHandlerRegistered()". It returns "declined"
the given scheme is invalid. Then, it calls "isProtocolHandlerRegistered() of
client" in order to check if given scheme is "registered" or "new". 
However, though I think client's isProtocolHandlerRegistered() can return
"declined", I don't find such situation yet. So, I don't handle "declined"
status in client's isProtocolHandlerRegistered() in this cl. If I will find any
situation that client needs to return "declined" status, I will add it as soon
as possible.



FYI, NavigatorContentUtils::isProtocolHandlerRegistered() returns "declined"
status when given scheme or url is not valid.

String NavigatorContentUtils::isProtocolHandlerRegistered(Navigator& navigator,
const String& scheme, const String& url, ExceptionState& exceptionState)
{
    DEFINE_STATIC_LOCAL(const String, declined, ("declined"));

    if (!navigator.frame())
        return declined;

    Document* document = navigator.frame()->document();
    ASSERT(document);
    if (document->activeDOMObjectsAreStopped())
        return declined;

    KURL baseURL = document->baseURL();

    if (!verifyCustomHandlerURL(baseURL, url, exceptionState))
        return declined;

    if (!verifyProtocolHandlerScheme(scheme, "isProtocolHandlerRegistered",
exceptionState))
        return declined;

    ASSERT(navigator.frame()->page());
    return
customHandlersStateString(NavigatorContentUtils::from(*navigator.frame()->page())->client()->isProtocolHandlerRegistered(scheme,
baseURL, KURL(ParsedURLString, url)));
}

[haraken, 2014-06-04 03:40:01]

Sorry about the delay. Chatted in IRC and I understand it's worth creating a
mock object for our testing purpose.

LGTM.

[gyuyoung-inactive, 2014-06-04 03:42:19]

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

[commit-bot: I haz the power, 2014-06-04 03:42:57]

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

[commit-bot: I haz the power, 2014-06-04 04:57:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6883)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10194)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10518)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

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

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

[commit-bot: I haz the power, 2014-06-04 04:57:59]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[gyuyoung-inactive, 2014-06-04 07:00:55]

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

[commit-bot: I haz the power, 2014-06-04 07:01:16]

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

[commit-bot: I haz the power, 2014-06-04 08:31:36]

Change committed as 175454