Bug 191853 – Remove @no-unify of InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp

• Status: REOPENED
• Component: WebKit2

[Fujii Hironori]

Remove @no-unify of InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp 

In r235845, I excluded InjectedBundleRangeHandle.cpp and
InjectedBundleNodeHandle.cpp from unify source builds in order to
work around a MSVC bug.

  <https://trac.webkit.org/changeset/235845>

I commited a different workaround for the MSVC bug in r238386. Now, we
can include InjectedBundleRangeHandle.cpp and
InjectedBundleNodeHandle.cpp in unified source builds.

  <https://trac.webkit.org/changeset/238386>

[Fujii Hironori]

Created attachment 355316 [details]
Patch

[EWS Watchlist]

Comment on attachment 355316 [details]
Patch

Attachment 355316 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10085543

New failing tests:
media/no-fullscreen-when-hidden.html

[EWS Watchlist]

Created attachment 355327 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Tim Horton]

Nice

[Fujii Hironori]

Comment on attachment 355316 [details]
Patch

Clearing flags on attachment: 355316

Committed r238432: <https://trac.webkit.org/changeset/238432>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46205899>

[Mark Lam]

r238432 was rolled out in r238446: <http://trac.webkit.org/r238446>.

[Mark Lam]

I'm not sure we actually need to land this patch (since it doesn't add much value).  We rolled it out because it introduced a build breakage in internal Mac builds.

[zalan]

(In reply to Mark Lam from comment #9)
> I'm not sure we actually need to land this patch (since it doesn't add much
> value).  We rolled it out because it introduced a build breakage in internal
> Mac builds.
Thanks Mark, I was about to roll this out.

[Michael Catanzaro]

Can you post a build log?

[Michael Catanzaro]

Maybe we need an EWS for the internal builds; otherwise it's not really possible to avoid breaking it. Especially with unified builds. If you have different unified source bundles in the internal build, all bets are off....

[Tim Horton]

(In reply to Michael Catanzaro from comment #12)
> Maybe we need an EWS for the internal builds; otherwise it's not really
> possible to avoid breaking it. Especially with unified builds. If you have
> different unified source bundles in the internal build, all bets are off....

That's a bit tricky to do safely.

Anyway, in this particular case it looks like the usual problem of "someone's got a 'using namespace WebCore' nearby the changed file".

[Tim Horton]

Could be WKBundlePageOverlay

[Fujii Hironori]

Created attachment 355829 [details]
Patch for landing

Bug 191995 has addressed. Can I try again?

[Tim Horton]

(In reply to Fujii Hironori from comment #15)
> Created attachment 355829 [details]
> Patch for landing
> 
> Bug 191995 has addressed. Can I try again?

I don’t see why not! (Though I will note that morning PST would probably be a better bet if you want someone on #webkit to check on the internal builds).

[Fujii Hironori]

Hmm, it doesn't sound good to me (I'll be sleeping in the time). I'd like to ask Apple folks to land my patch in morning.

[Mark Lam]

Landing it now.

[WebKit Commit Bot]

Comment on attachment 355829 [details]
Patch for landing

Clearing flags on attachment: 355829

Committed r238620: <https://trac.webkit.org/changeset/238620>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

Reverted r238620 for reason:

Broke internal builds again.

Committed r238625: <https://trac.webkit.org/changeset/238625>

[Ryan Haddad]

Sorry for the churn here. Someone will work with you in the next few days to get this sorted out.