Allow fragments to resolve relatively against any scheme

[WIP] Allow fragments to resolve relatively against any scheme

This is required for KURL backward compatibility for WebView - it
would allow a #foo fragment to apply against about:blank, for
example.

More background in b/10459904

still TODO
- agree if SetKURLCompatMode() is a sane way to go...
- tidy up the special casing in DoResolveRelative (extract to a helper?)
- raise crbug

[mkosiba (inactive), 2013-08-27 20:55:56]

https://codereview.chromium.org/23468003/diff/4001/url/url_util_unittest.cc
File url/url_util_unittest.cc (right):

https://codereview.chromium.org/23468003/diff/4001/url/url_util_unittest.cc#n...
url/url_util_unittest.cc:279: "javascript:alert('foo#badfrag" },
out of curiousity - what would KURL do in this case?

[joth, 2013-08-28 04:09:29]

brettw, please have  a look at let me know your thoughts on the direction I'm
going here.

The story-so-far is in https://b.corp.google.com/issue?id=10459904

[joth, 2013-08-28 06:12:45]

....what I forgot to ask specifically is, would introducing this resolution
rule into GURL unconditionally be bad for the rest of Chrome?

As if we were do it unconditionally, as per PS2, it's good for
maintainability by keeping  the configuration complexity much lower.
FWIW existing tests all seem to pass fine with it done this way
https://codereview.chromium.org/23468003/diff/4001/url/url_util.cc



On 27 August 2013 21:09, <joth@chromium.org> wrote:

> Reviewers: mkosiba, brettw,
>
> Message:
> brettw, please have  a look at let me know your thoughts on the direction
> I'm
> going here.
>
> The story-so-far is in
https://b.corp.google.com/**issue?id=10459904<https://b.corp.google.com/issue...
>
>
>
> Description:
> [WIP] Allow fragments to resolve relatively against any scheme
>
> This is required for KURL backward compatibility for WebView - it
> would allow a #foo fragment to apply against about:blank, for
> example.
>
> More background in b/10459904
>
> still TODO
> - agree if SetKURLCompatMode() is a sane way to go...
> - tidy up the special casing in DoResolveRelative (extract to a helper?)
> - raise crbug
>
>
>
> Please review this at
https://codereview.chromium.**org/23468003/<https://codereview.chromium.org/2...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M android_webview/common/aw_**switches.h
>   M android_webview/common/aw_**switches.cc
>   M android_webview/lib/main/aw_**main_delegate.cc
>   M url/url_util.h
>   M url/url_util.cc
>   M url/url_util_unittest.cc
>
>
> Index: android_webview/common/aw_**switches.cc
> diff --git a/android_webview/common/aw_**switches.cc
> b/android_webview/common/aw_**switches.cc
> index a87e93adc71208305a94cc04873059**0fd37b5f67..**
> 83f9bbc9ea3cb1c481c61ca467dcfb**0ebb8b5f09 100644
> --- a/android_webview/common/aw_**switches.cc
> +++ b/android_webview/common/aw_**switches.cc
> @@ -10,4 +10,6 @@ const char kDisableSimpleCache[] =
> "disable-simple-cache";
>
>  const char kDisableWebViewGLMode[] = "disable-webview-gl-mode";
>
> +const char kKURLCompatModep[] = "kurl-compat-mode";
> +
>  }  // namespace switches
> Index: android_webview/common/aw_**switches.h
> diff --git a/android_webview/common/aw_**switches.h
> b/android_webview/common/aw_**switches.h
> index 2c7db281c81523e175c046007a0c24**1a64d94156..**
> 38bc233d41c35f3dd67298fb1c0755**c003f2a612 100644
> --- a/android_webview/common/aw_**switches.h
> +++ b/android_webview/common/aw_**switches.h
> @@ -13,6 +13,9 @@ extern const char kDisableSimpleCache[];
>  // When set, forces use of fallback SW path even on HW canvas.
>  extern const char kDisableWebViewGLMode[];
>
> +// Specifying this flag will run the application in KURL compat mode
> +extern const char kKURLCompatMode[];
> +
>  }  // namespace switches
>
>  #endif  // ANDROID_WEBVIEW_COMMON_AW_**SWITCHES_H_
> Index: android_webview/lib/main/aw_**main_delegate.cc
> diff --git a/android_webview/lib/main/aw_**main_delegate.cc
> b/android_webview/lib/main/aw_**main_delegate.cc
> index 6843a748ea81445c345df649141ed2**627071fc32..**
> a5089e8b89f65842f521023d7818c6**f7ca2fda29 100644
> --- a/android_webview/lib/main/aw_**main_delegate.cc
> +++ b/android_webview/lib/main/aw_**main_delegate.cc
> @@ -23,6 +23,7 @@
>  #include "content/public/common/**content_switches.h"
>  #include "gpu/command_buffer/client/gl_**in_process_context.h"
>  #include "gpu/command_buffer/service/**in_process_command_buffer.h"
> +#include "url/url_util.h"
>  #include "webkit/common/gpu/**webgraphicscontext3d_in_**
> process_command_buffer_impl.h"
>
>  namespace android_webview {
> @@ -65,6 +66,8 @@ bool AwMainDelegate::**BasicStartupComplete(int*
> exit_code) {
>    // Ganesh backed 2D-Canvas is not yet working and causes crashes.
>    cl->AppendSwitch(switches::**kDisableAccelerated2dCanvas);
>
> +  url_util::SetKURLCompatMode(**cl->HasSwitch(iswitches::**
> kKURLCompatMode);
> +
>    return false;
>  }
>
> Index: url/url_util.cc
> diff --git a/url/url_util.cc b/url/url_util.cc
> index f16af98db85cc986a50af25630efa2**9da227d741..**
> 434b7c8a8d73d1cb137afb43641997**32baf14e2b 100644
> --- a/url/url_util.cc
> +++ b/url/url_util.cc
> @@ -36,6 +36,8 @@ inline bool DoLowerCaseEqualsASCII(Iter a_begin, Iter
> a_end, const char* b) {
>    return *b == 0;
>  }
>
> +bool g_kurl_compat_mode = false;
> +
>  const int kNumStandardURLSchemes = 8;
>  const char* kStandardURLSchemes[**kNumStandardURLSchemes] = {
>    "http",
> @@ -233,6 +235,30 @@ bool DoResolveRelative(const char* base_spec,
>                                  (base_is_hierarchical ||
> standard_base_scheme),
>                                  &is_relative,
>                                  &relative_component)) {
> +    if (g_kurl_compat_mode && relative_length && *relative == '#') {
> +      // Allow a fragemnt on its own to resolve against any base URL
> (even non-
> +      // hierarchical). First attempt to re-parse the path portion of the
> base
> +      // URL using a more relaxed algorithm for matching fragment
> sections.
> +      url_parse::Parsed base_reparsed = base_parsed;
> +      if (base_reparsed.path.is_valid() &&
> +          !base_reparsed.query.is_valid(**) &&
> +          !base_reparsed.ref.is_valid()) {
> +        const int path_end =  base_reparsed.path.end();
> +        for (int i = base_reparsed.path.begin; i < path_end; ++i) {
> +          if (base_spec[i] == '#') {
> +            base_reparsed.path.len = i - base_reparsed.path.begin;
> +            base_reparsed.ref = url_parse::MakeRange(i, path_end);
> +            break;
> +          }
> +        }
> +      }
> +      return url_canon::ResolveRelativeURL(**base_spec, base_reparsed,
> +                                           false, relative,
> +                                           url_parse::MakeRange(
> +                                               0, relative_length),
> +                                           charset_converter,
> +                                           output, output_parsed);
> +    }
>      // Error resolving.
>      return false;
>    }
> @@ -376,6 +402,10 @@ void Shutdown() {
>    }
>  }
>
> +void SetKURLCompatMode(bool enable) {
> +  g_kurl_compat_mode = enable;
> +}
> +
>  void AddStandardScheme(const char* new_scheme) {
>    // If this assert triggers, it means you've called AddStandardScheme
> after
>    // LockStandardSchemes have been called (see the header file for
> Index: url/url_util.h
> diff --git a/url/url_util.h b/url/url_util.h
> index 44948190e3c49e834622bd94dc368f**58eac7f232..**
> 9f17dcf06f95ddaba69b0252326ab8**eb693477ee 100644
> --- a/url/url_util.h
> +++ b/url/url_util.h
> @@ -35,6 +35,13 @@ URL_EXPORT void Initialize();
>  // library.
>  URL_EXPORT void Shutdown();
>
> +// Enables certain URL processing to work in a manner more like how KURL
> would
> +// have worked in classic (pre-blink) WebKit. This is only required for
> Android
> +// WebView compatibility.
> +// This should only be configured during initialization; subsequent calls
> are
> +// only permitted for testing.
> +URL_EXPORT void SetKURLCompatMode(bool enable);
> +
>  // Schemes ------------------------------**------------------------------
> **--------
>
>  // Adds an application-defined scheme to the internal list of "standard"
> URL
> Index: url/url_util_unittest.cc
> diff --git a/url/url_util_unittest.cc b/url/url_util_unittest.cc
> index 8b16d796cf1c027e10386318b5d742**aa40fb8a67..**
> 378d8f2854e6c916b51a10dde3d5fb**560d2974e9 100644
> --- a/url/url_util_unittest.cc
> +++ b/url/url_util_unittest.cc
> @@ -293,3 +293,48 @@ TEST(URLUtilTest,
TestResolveRelativeWithNonStan**dardBase)
> {
>        EXPECT_EQ(test_data.out, resolved) << i;
>    }
>  }
> +
> +TEST(URLUtilTest, TestKURLCompatMode) {
> +  // Tests the KURL compatibility mode in ResolveRelative
> +  struct ResolveRelativeCase {
> +    const char* base;
> +    const char* rel;
> +    const char* out;
> +  } kurl_compat_cases[] = {
> +    // These were all taken from running tests on KURL in WebKit 534.30.
> +    {"about:blank", "#id", "about:blank#id"},
> +    {"about:blank#oldfrag", "#id", "about:blank#id"},
> +    {"javascript:alert('ab#fg')", "#id", "javascript:alert('ab#id"},
> +    {"myscheme:foo?bar#baz?xxx#**yyy?zzz", "#id", "myscheme:foo?bar#id"},
> +    {"data:foo#bar?xxx#yyy?zzz", "#id", "data:foo#id"},
> +    {"mailto:joth@example.com#123"**, "#id", "mailto:joth@example.com
> #id"},
> +  };
> +
> +  for (int compat_mode = 0; compat_mode < 2; ++compat_mode) {
> +    url_util::SetKURLCompatMode(**compat_mode);
> +    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kurl_compat_**cases); i++) {
> +      const ResolveRelativeCase& test_data = kurl_compat_cases[i];
> +      url_parse::Parsed base_parsed;
> +      url_parse::ParsePathURL(test_**data.base, strlen(test_data.base),
> +                              &base_parsed);
> +
> +      std::string resolved;
> +      url_canon::**StdStringCanonOutput output(&resolved);
> +      url_parse::Parsed resolved_parsed;
> +      bool valid =
> +          url_util::ResolveRelative(**test_data.base,
> strlen(test_data.base),
> +                                    base_parsed,
> +                                    test_data.rel, strlen(test_data.rel),
> +                                    NULL, &output, &resolved_parsed);
> +      output.Complete();
> +
> +      if (!compat_mode) {
> +        EXPECT_FALSE(valid);
> +      } else {
> +        EXPECT_TRUE(valid);
> +        EXPECT_EQ(test_data.out, resolved) << i;
> +      }
> +    }
> +  }
> +  url_util::SetKURLCompatMode(**false);
> +}
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.